diff mbox series

[2/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format

Message ID 20240110102535.246177-3-dharma.b@microchip.com
State New
Headers
Series Convert Microchip's HLCDC Text based DT bindings to JSON schema |

Commit Message

Dharma Balasubiramani Jan. 10, 2024, 10:25 a.m. UTC 
  Convert the atmel,hlcdc binding to DT schema format.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
 .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
 2 files changed, 106 insertions(+), 56 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt

Comments

Rob Herring Jan. 10, 2024, 11:57 a.m. UTC | #1 
On Wed, 10 Jan 2024 15:55:34 +0530, Dharma Balasubiramani wrote:
> Convert the atmel,hlcdc binding to DT schema format.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
>  .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>  .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>  2 files changed, 106 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.example.dtb: hlcdc@f0030000: hlcdc-pwm: False schema does not allow {'compatible': ['atmel,hlcdc-pwm'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], '#pwm-cells': [[3]]}
	from schema $id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/atmel,hlcdc.example.dtb: hlcdc@f0030000: hlcdc-pwm: False schema does not allow {'compatible': ['atmel,hlcdc-pwm'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], '#pwm-cells': [[3]]}
	from schema $id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
Documentation/devicetree/bindings/mfd/atmel,hlcdc.example.dtb: /example-0/hlcdc@f0030000/hlcdc-pwm: failed to match any schema with compatible: ['atmel,hlcdc-pwm']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240110102535.246177-3-dharma.b@microchip.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Jan. 10, 2024, 6:01 p.m. UTC | #2 
On 10/01/2024 11:25, Dharma Balasubiramani wrote:
> Convert the atmel,hlcdc binding to DT schema format.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
>  .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>  .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>  2 files changed, 106 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> new file mode 100644
> index 000000000000..555d6faa9104
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml

This looks not tested, so limited review follows:

> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's HLCDC (High LCD Controller) MFD driver

Drop "MFD driver" and rather describe/name the hardware. MFD is Linux
term, so I really doubt that's how this was called.

> +
> +maintainers:
> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +description: |
> +  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.

Drop

> +  The HLCDC IP exposes two subdevices:
> +  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
> +  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml

Rephrase to describe hardware. Drop redundant paths.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9n12-hlcdc
> +      - atmel,at91sam9x5-hlcdc
> +      - atmel,sama5d2-hlcdc
> +      - atmel,sama5d3-hlcdc
> +      - atmel,sama5d4-hlcdc
> +      - microchip,sam9x60-hlcdc
> +      - microchip,sam9x75-xlcdc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    anyOf:
> +      - items:
> +          - enum:
> +              - sys_clk
> +              - lvds_pll_clk

Old binding was not mentioning this and you did not describe differences
against pure conversion. You have entire commit msg for this...

> +      - contains:
> +          const: periph_clk
> +      - contains:
> +          const: slow_clk
> +        maxItems: 3

Why it has to be so complicated? I doubt that same devices have
different inputs.

> +
> +  hlcdc-display-controller:

Does anything depend on the name? If not, then just display-controller

> +    $ref: /schemas/display/atmel/atmel,hlcdc-dc.yaml
> +
> +  hlcdc-pwm:

Same comment.

> +    $ref: /schemas/pwm/atmel,hlcdc-pwm.yaml

There is no such file.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/dma/at91.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    hlcdc: hlcdc@f0030000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +      compatible = "atmel,sama5d3-hlcdc";
> +      reg = <0xf0030000 0x2000>;
> +      clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> +      clock-names = "periph_clk","sys_clk", "slow_clk";

Missing space after ,

> +      interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
> +
> +      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>;
> +          };
> +        };
> +      };
> +


Best regards,
Krzysztof
Dharma Balasubiramani Jan. 17, 2024, 2:22 a.m. UTC | #3 
Hi Krzysztof,
On 10/01/24 11:31 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/01/2024 11:25, Dharma Balasubiramani wrote:
>> Convert the atmel,hlcdc binding to DT schema format.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>> ---
>>   .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>>   .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>>   2 files changed, 106 insertions(+), 56 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>> new file mode 100644
>> index 000000000000..555d6faa9104
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> 
> This looks not tested, so limited review follows:
I acknowledge that I didn't test the patches individually. I appreciate 
your understanding. Taken care in v2.
> 
>> @@ -0,0 +1,106 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel's HLCDC (High LCD Controller) MFD driver
> 
> Drop "MFD driver" and rather describe/name the hardware. MFD is Linux
> term, so I really doubt that's how this was called.
Done.
> 
>> +
>> +maintainers:
>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> +
>> +description: |
>> +  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.
> 
> Drop
Done.
> 
>> +  The HLCDC IP exposes two subdevices:
>> +  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
>> +  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml
> 
> Rephrase to describe hardware. Drop redundant paths.
Sure, I will truncate this to "subdevices: a PWM chip and a display 
controller." & drop the |.

I added description about those two subdevices below.
> 
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - atmel,at91sam9n12-hlcdc
>> +      - atmel,at91sam9x5-hlcdc
>> +      - atmel,sama5d2-hlcdc
>> +      - atmel,sama5d3-hlcdc
>> +      - atmel,sama5d4-hlcdc
>> +      - microchip,sam9x60-hlcdc
>> +      - microchip,sam9x75-xlcdc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 3
>> +
>> +  clock-names:
>> +    anyOf:
>> +      - items:
>> +          - enum:
>> +              - sys_clk
>> +              - lvds_pll_clk
> 
> Old binding was not mentioning this and you did not describe differences
> against pure conversion. You have entire commit msg for this...
Done, added commit message in v2.
> 
>> +      - contains:
>> +          const: periph_clk
>> +      - contains:
>> +          const: slow_clk
>> +        maxItems: 3
> 
> Why it has to be so complicated? I doubt that same devices have
> different inputs.
I will modify it to
   clock-names:
     items:
       - const: periph_clk
       - enum: [sys_clk, lvds_pll_clk]
       - const: slow_clk
in v3.
> 
>> +
>> +  hlcdc-display-controller:
> 
> Does anything depend on the name? If not, then just display-controller
Got an error "'hlcdc-display-controller' does not match any of the 
regexes: 'pinctrl-[0-9]+'" so I retained it in v2,but as conor advised 
to have node names generic and we can easily adopt, I will modify it in v3.
> 
>> +    $ref: /schemas/display/atmel/atmel,hlcdc-dc.yaml
>> +
>> +  hlcdc-pwm:
> 
> Same comment.
Sure, I will modify it in v3.
> 
>> +    $ref: /schemas/pwm/atmel,hlcdc-pwm.yaml
> 
> There is no such file.
This occurred because I added it before pwm patch. In v2, I introduced 
'display' and 'pwm' before 'mfd' so that I could reference them here.
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/at91.h>
>> +    #include <dt-bindings/dma/at91.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    hlcdc: hlcdc@f0030000 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Done.
> 
> 
>> +      compatible = "atmel,sama5d3-hlcdc";
>> +      reg = <0xf0030000 0x2000>;
>> +      clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
>> +      clock-names = "periph_clk","sys_clk", "slow_clk";
> 
> Missing space after ,
Sure, fixed in v2.

-- 
With Best Regards,
Dharma B.
> 
>> +      interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
>> +
>> +      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>;
>> +          };
>> +        };
>> +      };
>> +
> 
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 17, 2024, 7:37 a.m. UTC | #4 
On 17/01/2024 03:22, Dharma.B@microchip.com wrote:
> Hi Krzysztof,
> On 10/01/24 11:31 pm, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 10/01/2024 11:25, Dharma Balasubiramani wrote:
>>> Convert the atmel,hlcdc binding to DT schema format.
>>>
>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>> ---
>>>   .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>>>   .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>>>   2 files changed, 106 insertions(+), 56 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>   delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>> new file mode 100644
>>> index 000000000000..555d6faa9104
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>
>> This looks not tested, so limited review follows:
> I acknowledge that I didn't test the patches individually. I appreciate 
> your understanding. Taken care in v2.
>>
>>> @@ -0,0 +1,106 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Atmel's HLCDC (High LCD Controller) MFD driver
>>
>> Drop "MFD driver" and rather describe/name the hardware. MFD is Linux
>> term, so I really doubt that's how this was called.
> Done.
>>
>>> +
>>> +maintainers:
>>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>> +
>>> +description: |
>>> +  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.
>>
>> Drop
> Done.
>>
>>> +  The HLCDC IP exposes two subdevices:
>>> +  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
>>> +  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml
>>
>> Rephrase to describe hardware. Drop redundant paths.
> Sure, I will truncate this to "subdevices: a PWM chip and a display 
> controller." & drop the |.
> 
> I added description about those two subdevices below.

Then why do you still keep there comments? # is a comment.
..

> in v3.
>>
>>> +
>>> +  hlcdc-display-controller:
>>
>> Does anything depend on the name? If not, then just display-controller
> Got an error "'hlcdc-display-controller' does not match any of the 
> regexes: 'pinctrl-[0-9]+'" so I retained it in v2,but as conor advised 
> to have node names generic and we can easily adopt, I will modify it in v3.

That's not a dependency. I was talking about any users of the binding.
User of a binding is for example: Linux kernel driver, other open-source
projects, out-of-tree projects.


Best regards,
Krzysztof
Dharma Balasubiramani Jan. 18, 2024, 5:50 a.m. UTC | #5 
On 17/01/24 1:07 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 17/01/2024 03:22, Dharma.B@microchip.com wrote:
>> Hi Krzysztof,
>> On 10/01/24 11:31 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/01/2024 11:25, Dharma Balasubiramani wrote:
>>>> Convert the atmel,hlcdc binding to DT schema format.
>>>>
>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>> ---
>>>>    .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>>>>    .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>>>>    2 files changed, 106 insertions(+), 56 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>>    delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>> new file mode 100644
>>>> index 000000000000..555d6faa9104
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>
>>> This looks not tested, so limited review follows:
>> I acknowledge that I didn't test the patches individually. I appreciate
>> your understanding. Taken care in v2.
>>>
>>>> @@ -0,0 +1,106 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Atmel's HLCDC (High LCD Controller) MFD driver
>>>
>>> Drop "MFD driver" and rather describe/name the hardware. MFD is Linux
>>> term, so I really doubt that's how this was called.
>> Done.
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>>>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>> +
>>>> +description: |
>>>> +  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.
>>>
>>> Drop
>> Done.
>>>
>>>> +  The HLCDC IP exposes two subdevices:
>>>> +  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
>>>> +  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml
>>>
>>> Rephrase to describe hardware. Drop redundant paths.
>> Sure, I will truncate this to "subdevices: a PWM chip and a display
>> controller." & drop the |.
>>
>> I added description about those two subdevices below.
> 
> Then why do you still keep there comments? # is a comment.
Sure, I will drop the comments here.
> ...
> 
>> in v3.
>>>
>>>> +
>>>> +  hlcdc-display-controller:
>>>
>>> Does anything depend on the name? If not, then just display-controller
>> Got an error "'hlcdc-display-controller' does not match any of the
>> regexes: 'pinctrl-[0-9]+'" so I retained it in v2,but as conor advised
>> to have node names generic and we can easily adopt, I will modify it in v3.
> 
> That's not a dependency. I was talking about any users of the binding.
> User of a binding is for example: Linux kernel driver, other open-source
> projects, out-of-tree projects.
Sure, I will go with generic names 'display-controller' and 'pwm'.

-- 
With Best Regards,
Dharma B.
> 
> 
> Best regards,
> Krzysztof
>
Dharma Balasubiramani Jan. 18, 2024, 9:07 a.m. UTC | #6 
On 17/01/24 1:07 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 17/01/2024 03:22, Dharma.B@microchip.com wrote:
>> Hi Krzysztof,
>> On 10/01/24 11:31 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/01/2024 11:25, Dharma Balasubiramani wrote:
>>>> Convert the atmel,hlcdc binding to DT schema format.
>>>>
>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>> ---
>>>>    .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 106 ++++++++++++++++++
>>>>    .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ---------
>>>>    2 files changed, 106 insertions(+), 56 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>>    delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>> new file mode 100644
>>>> index 000000000000..555d6faa9104
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>
>>> This looks not tested, so limited review follows:
>> I acknowledge that I didn't test the patches individually. I appreciate
>> your understanding. Taken care in v2.
>>>
>>>> @@ -0,0 +1,106 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Atmel's HLCDC (High LCD Controller) MFD driver
>>>
>>> Drop "MFD driver" and rather describe/name the hardware. MFD is Linux
>>> term, so I really doubt that's how this was called.
>> Done.
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>>>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>> +
>>>> +description: |
>>>> +  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.
>>>
>>> Drop
>> Done.
>>>
>>>> +  The HLCDC IP exposes two subdevices:
>>>> +  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
>>>> +  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml
>>>
>>> Rephrase to describe hardware. Drop redundant paths.
>> Sure, I will truncate this to "subdevices: a PWM chip and a display
>> controller." & drop the |.
>>
>> I added description about those two subdevices below.
> 
> Then why do you still keep there comments? # is a comment.
My bad, I will remove the comments and rephrase it as a typical description.
> ...
> 
>> in v3.
>>>
>>>> +
>>>> +  hlcdc-display-controller:
>>>
>>> Does anything depend on the name? If not, then just display-controller
>> Got an error "'hlcdc-display-controller' does not match any of the
>> regexes: 'pinctrl-[0-9]+'" so I retained it in v2,but as conor advised
>> to have node names generic and we can easily adopt, I will modify it in v3.
> 
> That's not a dependency. I was talking about any users of the binding.
> User of a binding is for example: Linux kernel driver, other open-source
> projects, out-of-tree projects.
Got it, I will make them generic.
> 
> 
> Best regards,
> Krzysztof
> 

-- 
With Best Regards,
Dharma B.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
new file mode 100644
index 000000000000..555d6faa9104
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
@@ -0,0 +1,106 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel's HLCDC (High LCD Controller) MFD driver
+
+maintainers:
+  - Nicolas Ferre <nicolas.ferre@microchip.com>
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
+
+description: |
+  Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver.
+  The HLCDC IP exposes two subdevices:
+  # a PWM chip: see ../pwm/atmel,hlcdc-pwm.yaml
+  # a Display Controller: see ../display/atmel/atmel,hlcdc-dc.yaml
+
+properties:
+  compatible:
+    enum:
+      - atmel,at91sam9n12-hlcdc
+      - atmel,at91sam9x5-hlcdc
+      - atmel,sama5d2-hlcdc
+      - atmel,sama5d3-hlcdc
+      - atmel,sama5d4-hlcdc
+      - microchip,sam9x60-hlcdc
+      - microchip,sam9x75-xlcdc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    anyOf:
+      - items:
+          - enum:
+              - sys_clk
+              - lvds_pll_clk
+      - contains:
+          const: periph_clk
+      - contains:
+          const: slow_clk
+        maxItems: 3
+
+  hlcdc-display-controller:
+    $ref: /schemas/display/atmel/atmel,hlcdc-dc.yaml
+
+  hlcdc-pwm:
+    $ref: /schemas/pwm/atmel,hlcdc-pwm.yaml
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    hlcdc: hlcdc@f0030000 {
+      compatible = "atmel,sama5d3-hlcdc";
+      reg = <0xf0030000 0x2000>;
+      clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
+      clock-names = "periph_clk","sys_clk", "slow_clk";
+      interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+
+      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>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
deleted file mode 100644
index 7de696eefaed..000000000000
--- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
+++ /dev/null
@@ -1,56 +0,0 @@ 
-Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver
-
-Required properties:
- - compatible: value should be one of the following:
-   "atmel,at91sam9n12-hlcdc"
-   "atmel,at91sam9x5-hlcdc"
-   "atmel,sama5d2-hlcdc"
-   "atmel,sama5d3-hlcdc"
-   "atmel,sama5d4-hlcdc"
-   "microchip,sam9x60-hlcdc"
-   "microchip,sam9x75-xlcdc"
- - reg: base address and size of the HLCDC device registers.
- - clock-names: the name of the 3 clocks requested by the HLCDC device.
-   Should contain "periph_clk", "sys_clk" and "slow_clk".
- - clocks: should contain the 3 clocks requested by the HLCDC device.
- - interrupts: should contain the description of the HLCDC interrupt line
-
-The HLCDC IP exposes two subdevices:
- - a PWM chip: see ../pwm/atmel-hlcdc-pwm.txt
- - a Display Controller: see ../display/atmel/hlcdc-dc.txt
-
-Example:
-
-	hlcdc: hlcdc@f0030000 {
-		compatible = "atmel,sama5d3-hlcdc";
-		reg = <0xf0030000 0x2000>;
-		clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
-		clock-names = "periph_clk","sys_clk", "slow_clk";
-		interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
-
-		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>;
-		};
-	};