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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a84946c-ba15-44ec-ae4a-3ad7a0673d4c@microchip.com>
Date: Wed, 17 Jan 2024 02:35:25 +0000
From: <Dharma.B@...rochip.com>
To: <conor@...nel.org>
CC: <Conor.Dooley@...rochip.com>, <sam@...nborg.org>, <bbrezillon@...nel.org>,
	<maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
	<tzimmermann@...e.de>, <airlied@...il.com>, <daniel@...ll.ch>,
	<robh+dt@...nel.org>, <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>,
	<lee@...nel.org>, <thierry.reding@...il.com>,
	<u.kleine-koenig@...gutronix.de>, <linux-pwm@...r.kernel.org>,
	<Linux4Microchip@...rochip.com>
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT
 schema

On 16/01/24 11:25 pm, Conor Dooley wrote:
> Yo,
> 
> On Tue, Jan 16, 2024 at 05:07:58PM +0530, Dharma Balasubiramani wrote:
>> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
>> controller.
>>
>> Signed-off-by: Dharma Balasubiramani<dharma.b@...rochip.com>
>> ---
>> changelog
>> v1 -> v2
>> - Remove the explicit copyrights.
>> - Modify filename like compatible.
>> - Modify title (drop words like binding/driver).
>> - Modify description actually describing the hardware and not the driver.
>> - Remove pinctrl properties which aren't required.
>> - Ref endpoint and not endpoint-base.
>> - Drop redundant info about bus-width description and add ref to video-interfaces.
>> - Move 'additionalProperties' after 'Required'.
>> - Drop parent node and it's other sub-device node which are not related here.
>> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
>> ---
>>   .../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
>>   .../bindings/display/atmel/hlcdc-dc.txt       |  75 ------------
>>   2 files changed, 110 insertions(+), 75 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> new file mode 100644
>> index 000000000000..f022c294cfbc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel's High LCD Controller (HLCDC)
>> +
>> +maintainers:
>> +  - Nicolas Ferre<nicolas.ferre@...rochip.com>
>> +  - Alexandre Belloni<alexandre.belloni@...tlin.com>
>> +  - Claudiu Beznea<claudiu.beznea@...on.dev>
>> +
>> +description: |
> This | is not needed as you have no formatting to preserve.
Sure, I will drop this '|'.
> 
>> +  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.
>> +
>> +properties:
>> +  compatible:
>> +    const: atmel,hlcdc-display-controller
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +  port@0:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    unevaluatedProperties: false
>> +    description:
>> +      Output endpoint of the controller, connecting the LCD panel signals.
>> +
>> +    properties:
>> +      '#address-cells':
>> +        const: 1
>> +
>> +      '#size-cells':
>> +        const: 0
>> +
>> +      reg:
>> +        maxItems: 1
>> +
>> +      endpoint:
>> +        $ref: /schemas/graph.yaml#/$defs/endpoint
> $ref: /schemas/media/video-interfaces.yaml#
I will replace this.
> 
> to match approximately all other endpoints?
I'm not sure; some of the referenced devices, like this one, were 
pointing to 'endpoint.' I will go with 'video-interfaces.'
> 
>> +        unevaluatedProperties: false
>> +        description:
>> +          Endpoint connecting the LCD panel signals.
>> +
>> +        properties:
>> +          bus-width:
>> +            description: Endpoint bus width.
>> +            $ref: /schemas/media/video-interfaces.yaml#
> and then bus-width's type is already defined for you, no?
I will remove this $ref here.
> 
>> +            enum: [ 12, 16, 18, 24 ]
>> +
>> +required:
>> +  - '#address-cells'
>> +  - '#size-cells'
>> +  - compatible
>> +  - port@0
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    //Example 1
>> +
>> +    display-controller {
>> +      compatible = "atmel,hlcdc-display-controller";
>> +      pinctrl-names = "default";
>> +      pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      port@0 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <0>;
>> +
>> +        hlcdc_panel_output: endpoint@0 {
>> +          reg = <0>;
>> +          remote-endpoint = <&panel_input>;
>> +        };
>> +      };
>> +    };
>> +
>> +  - |
>> +    //Example 2 With a video interface override to force rgb565, bus-width=16
>> +
>> +    display-controller {
>> +      compatible = "atmel,hlcdc-display-controller";
>> +      pinctrl-names = "default";
>> +      pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      port@0 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <0>;
> Should be a newline here before the child node.
Sure, I will take care of this in v3.

-- 
With Best Regards,
Dharma B.
> 
> Cheers,
> Conor.
> 
>> +        hlcdc_panel_output2: endpoint@0 {
>> +          reg = <0>;
>> +          remote-endpoint = <&panel_input>;
>> +          bus-width = <16>;
>> +        };
>> +      };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>> deleted file mode 100644
>> index 923aea25344c..000000000000
>> --- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>> +++ /dev/null
>> @@ -1,75 +0,0 @@
>> -Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
>> -
>> -The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
>> -See ../../mfd/atmel-hlcdc.txt for more details.
>> -
>> -Required properties:
>> - - compatible: value should be "atmel,hlcdc-display-controller"
>> - - pinctrl-names: the pin control state names. Should contain "default".
>> - - pinctrl-0: should contain the default pinctrl states.
>> - - #address-cells: should be set to 1.
>> - - #size-cells: should be set to 0.
>> -
>> -Required children nodes:
>> - Children nodes are encoding available output ports and their connections
>> - to external devices using the OF graph representation (see ../graph.txt).
>> - At least one port node is required.
>> -
>> -Optional properties in grandchild nodes:
>> - Any endpoint grandchild node may specify a desired video interface
>> - according to ../../media/video-interfaces.txt, specifically
>> - - bus-width: recognized values are <12>, <16>, <18> and <24>, and
>> -   override any output mode selection heuristic, forcing "rgb444",
>> -   "rgb565", "rgb666" and "rgb888" respectively.
>> -
>> -Example:
>> -
>> -	hlcdc: hlcdc@...30000 {
>> -		compatible = "atmel,sama5d3-hlcdc";
>> -		reg = <0xf0030000 0x2000>;
>> -		interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
>> -		clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
>> -		clock-names = "periph_clk","sys_clk", "slow_clk";
>> -
>> -		hlcdc-display-controller {
>> -			compatible = "atmel,hlcdc-display-controller";
>> -			pinctrl-names = "default";
>> -			pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
>> -			#address-cells = <1>;
>> -			#size-cells = <0>;
>> -
>> -			port@0 {
>> -				#address-cells = <1>;
>> -				#size-cells = <0>;
>> -				reg = <0>;
>> -
>> -				hlcdc_panel_output: endpoint@0 {
>> -					reg = <0>;
>> -					remote-endpoint = <&panel_input>;
>> -				};
>> -			};
>> -		};
>> -
>> -		hlcdc_pwm: hlcdc-pwm {
>> -			compatible = "atmel,hlcdc-pwm";
>> -			pinctrl-names = "default";
>> -			pinctrl-0 = <&pinctrl_lcd_pwm>;
>> -			#pwm-cells = <3>;
>> -		};
>> -	};
>> -
>> -Example 2: With a video interface override to force rgb565; as above
>> -but with these changes/additions:
>> -
>> -	&hlcdc {
>> -		hlcdc-display-controller {
>> -			pinctrl-names = "default";
>> -			pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
>> -
>> -			port@0 {
>> -				hlcdc_panel_output: endpoint@0 {
>> -					bus-width = <16>;
>> -				};
>> -			};
>> -		};
>> -	};
>> -- 
>> 2.25.1
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ