| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d3ae3196-4e8a-4e73-bc36-f53541598ab2@microchip.com>
Date: Wed, 6 Mar 2024 14:35:14 +0000
From: <Dharma.B@...rochip.com>
To: <robh@...nel.org>
CC: <maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
<tzimmermann@...e.de>, <airlied@...il.com>, <daniel@...ll.ch>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<Nicolas.Ferre@...rochip.com>, <alexandre.belloni@...tlin.com>,
<claudiu.beznea@...on.dev>, <dri-devel@...ts.freedesktop.org>,
<devicetree@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] dt-bindings: display: atmel,lcdc: convert to dtschema
On 05/03/24 3:31 am, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, Mar 04, 2024 at 08:00:03PM +0530, Dharma Balasubiramani wrote:
>> Convert the atmel,lcdc bindings to DT schema.
>> Changes during conversion: add missing clocks and clock-names properties.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@...rochip.com>
>> ---
>> This patch converts the existing lcdc display text binding to JSON schema.
>> The binding is split into two namely
>> lcdc.yaml
>> - Holds the frame buffer properties
>> lcdc-display.yaml
>> - Holds the display panel properties which is a phandle to the display
>> property in lcdc fb node.
>>
>> These bindings are tested using the following command.
>> 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> ---
>> Changes in v3:
>> - Remove the generic property "bits-per-pixel"
>> - Link to v2: https://lore.kernel.org/r/20240304-lcdc-fb-v2-1-a14b463c157a@microchip.com
>>
>> Changes in v2:
>> - Run checkpatch and remove whitespace errors.
>> - Add the standard interrupt flags.
>> - Split the binding into two, namely lcdc.yaml and lcdc-display.yaml.
>> - Link to v1: https://lore.kernel.org/r/20240223-lcdc-fb-v1-1-4c64cb6277df@microchip.com
>> ---
>> .../bindings/display/atmel,lcdc-display.yaml | 97 ++++++++++++++++++++++
>> .../devicetree/bindings/display/atmel,lcdc.txt | 87 -------------------
>> .../devicetree/bindings/display/atmel,lcdc.yaml | 70 ++++++++++++++++
>> 3 files changed, 167 insertions(+), 87 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/atmel,lcdc-display.yaml b/Documentation/devicetree/bindings/display/atmel,lcdc-display.yaml
>> new file mode 100644
>> index 000000000000..5e0b706d695d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/atmel,lcdc-display.yaml
>> @@ -0,0 +1,97 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/atmel,lcdc-display.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip's LCDC Display
>> +
>> +maintainers:
>> + - Nicolas Ferre <nicolas.ferre@...rochip.com>
>> + - Dharma Balasubiramani <dharma.b@...rochip.com>
>> +
>> +description:
>> + The LCD Controller (LCDC) consists of logic for transferring LCD image data
>> + from an external display buffer to a TFT LCD panel. The LCDC has one display
>> + input buffer per layer that fetches pixels through the single bus host
>> + interface and a look-up table to allow palletized display configurations. The
>> + LCDC is programmable on a per layer basis, and supports different LCD
>> + resolutions, window sizes, image formats and pixel depths.
>> +
>> +# We need a select here since this schema is applicable only for nodes with the
>> +# following properties
>> +
>> +select:
>> + anyOf:
>> + - required: [ 'atmel,dmacon' ]
>> + - required: [ 'atmel,lcdcon2' ]
>> + - required: [ 'atmel,guard-time' ]
>> +
>> +properties:
>> + atmel,dmacon:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: dma controller configuration
>> +
>> + atmel,lcdcon2:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: lcd controller configuration
>> +
>> + atmel,guard-time:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: lcd guard time (Delay in frame periods)
>
> Is there a maximum?
The datasheet (https://www.keil.com/dd/chip/4210.htm) for both
AT91SAM9261 and AT91SAM9263 specifies the GUARD_TIME field within the
PWRCON register. This field is 7 bits wide (bits 1-7) and acts as a mask
to define the guard time delay.
Hence the maximum value for the guard time for both AT91SAM9261 and
AT91SAM9263 SoCs is 127 when using the GUARD_TIME field in the PWRCON
register.
The datasheet doesn't specify the duration of each "frame period." So,
this value only represents the maximum number of frame periods that can
be set for the guard time, not the actual time delay in seconds or
milliseconds.
>
>> +
>> + bits-per-pixel:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: lcd panel bit-depth.
>
> Constraints?
The bits-per-pixel value can be 1, 2, 4, 8, 16 or 24.
>
>> +
>> + atmel,lcdcon-backlight:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: enable backlight
>> +
>> + atmel,lcdcon-backlight-inverted:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: invert backlight PWM polarity
>> +
>> + atmel,lcd-wiring-mode:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>
> Isn't this just a single string rather than an array?
>
>> + description: lcd wiring mode "RGB" or "BRG"
>
> enum:
> - RGB
> - BRG
>
> No BGR?
In the current driver implementation, we have interpreted the wiring
mode represented by ATMEL_LCDC_WIRING_BGR as 'BRG' in the array
atmel_lcdfb_wiring_modes. Considering conventional color representation,
would it be appropriate to consider modifying the existing driver to use
the 'BGR' string instead of 'BRG' for better alignment with standard
naming conventions?
static const char *atmel_lcdfb_wiring_modes[] = {
[ATMEL_LCDC_WIRING_BGR] = "BRG",
[ATMEL_LCDC_WIRING_RGB] = "RGB",
};
>
> But wait, the example shows the value is '1'. That should fail testing.
> It didn't, but I've now fixed that.
It seems correctly configured in our dts files but didn't noticed the
same in the bindings example, thanks for letting me know, I will correct
it in the next revision.
>
>> +
>> + atmel,power-control-gpio:
>> + description: gpio to power on or off the LCD (as many as needed)
>
> maxItems: 1
Thanks, I will add it.
>
>> +
>> + display-timings:
>> + $ref: panel/display-timings.yaml#
>> +
>> +required:
>> + - atmel,dmacon
>> + - atmel,lcdcon2
>> + - atmel,guard-time
>> + - bits-per-pixel
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + display: panel {
>> + bits-per-pixel = <32>;
>> + atmel,lcdcon-backlight;
>> + atmel,dmacon = <0x1>;
>> + atmel,lcdcon2 = <0x80008002>;
>> + atmel,guard-time = <9>;
>> + atmel,lcd-wiring-mode = <1>;
>> +
>> + display-timings {
>> + native-mode = <&timing0>;
>> + timing0: timing0 {
>> + clock-frequency = <9000000>;
>> + hactive = <480>;
>> + vactive = <272>;
>> + hback-porch = <1>;
>> + hfront-porch = <1>;
>> + vback-porch = <40>;
>> + vfront-porch = <1>;
>> + hsync-len = <45>;
>> + vsync-len = <1>;
>> + };
>> + };
>> + };
--
With Best Regards,
Dharma B.
Powered by blists - more mailing lists