2024-01-30 12:19:54

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: pinctrl: cy8x95x0: Minor fix & update

On 30/01/2024 13:08, Naresh Solanki wrote:
> Update maxItems to 60 for gpio-reserved-ranges to allow multiple gpio
> reserved ranges.
> Add input-enable property to allow configuring a pin as input.
> Rearrange allOf

Why?

> Update example.
>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
> .../bindings/pinctrl/cypress,cy8c95x0.yaml | 28 +++++++++++++++----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml b/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> index 7f30ec2f1e54..89ce0cb68834 100644
> --- a/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> @@ -45,7 +45,8 @@ properties:
> maxItems: 1
>
> gpio-reserved-ranges:
> - maxItems: 1
> + minItems: 1
> + maxItems: 60
>
> vdd-supply:
> description:
> @@ -85,6 +86,8 @@ patternProperties:
>
> bias-disable: true
>
> + input-enable: true
> +
> output-high: true
>
> output-low: true
> @@ -101,6 +104,9 @@ patternProperties:
>
> additionalProperties: false
>
> +allOf:
> + - $ref: pinctrl.yaml#
> +

That's not a correct placement.

> required:
> - compatible
> - reg
> @@ -112,9 +118,6 @@ required:
>
> additionalProperties: false
>
> -allOf:
> - - $ref: pinctrl.yaml#
> -
> examples:
> - |
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -133,6 +136,21 @@ examples:
> interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-controller;
> vdd-supply = <&p3v3>;
> - gpio-reserved-ranges = <5 1>;
> + gpio-reserved-ranges = <1 2>, <6 1>, <10 1>, <15 1>;
> +
> + pinctrl-0 = <&U62160_pins>, <&U62160_ipins>;
> + pinctrl-names = "default";
> + U62160_pins: cfg-pins {
> + pins = "gp03", "gp16", "gp20", "gp50", "gp51";

Your indentation is totally broken.


Best regards,
Krzysztof



2024-01-30 12:27:13

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: pinctrl: cy8x95x0: Minor fix & update

Hi Krzysztof,

On Tue, 30 Jan 2024 at 17:49, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 30/01/2024 13:08, Naresh Solanki wrote:
> > Update maxItems to 60 for gpio-reserved-ranges to allow multiple gpio
> > reserved ranges.
> > Add input-enable property to allow configuring a pin as input.
> > Rearrange allOf
>
> Why?
allOf is aligned with other implementations.
>
> > Update example.
> >
> > Signed-off-by: Naresh Solanki <[email protected]>
> > ---
> > .../bindings/pinctrl/cypress,cy8c95x0.yaml | 28 +++++++++++++++----
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml b/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> > index 7f30ec2f1e54..89ce0cb68834 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/cypress,cy8c95x0.yaml
> > @@ -45,7 +45,8 @@ properties:
> > maxItems: 1
> >
> > gpio-reserved-ranges:
> > - maxItems: 1
> > + minItems: 1
> > + maxItems: 60
> >
> > vdd-supply:
> > description:
> > @@ -85,6 +86,8 @@ patternProperties:
> >
> > bias-disable: true
> >
> > + input-enable: true
> > +
> > output-high: true
> >
> > output-low: true
> > @@ -101,6 +104,9 @@ patternProperties:
> >
> > additionalProperties: false
> >
> > +allOf:
> > + - $ref: pinctrl.yaml#
> > +
>
> That's not a correct placement.
ok. Will remove this specific change.
>
> > required:
> > - compatible
> > - reg
> > @@ -112,9 +118,6 @@ required:
> >
> > additionalProperties: false
> >
> > -allOf:
> > - - $ref: pinctrl.yaml#
> > -
> > examples:
> > - |
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -133,6 +136,21 @@ examples:
> > interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> > interrupt-controller;
> > vdd-supply = <&p3v3>;
> > - gpio-reserved-ranges = <5 1>;
> > + gpio-reserved-ranges = <1 2>, <6 1>, <10 1>, <15 1>;
> > +
> > + pinctrl-0 = <&U62160_pins>, <&U62160_ipins>;
> > + pinctrl-names = "default";
> > + U62160_pins: cfg-pins {
> > + pins = "gp03", "gp16", "gp20", "gp50", "gp51";
>
> Your indentation is totally broken.
sorry I missed checking on this before sending the patch for review. will fix
it to use two space for indentation.

Regards,
Naresh
>
>
> Best regards,
> Krzysztof
>

2024-01-30 12:31:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: pinctrl: cy8x95x0: Minor fix & update

On 30/01/2024 13:26, Naresh Solanki wrote:
> Hi Krzysztof,
>
> On Tue, 30 Jan 2024 at 17:49, Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 30/01/2024 13:08, Naresh Solanki wrote:
>>> Update maxItems to 60 for gpio-reserved-ranges to allow multiple gpio
>>> reserved ranges.
>>> Add input-enable property to allow configuring a pin as input.
>>> Rearrange allOf
>>
>> Why?
> allOf is aligned with other implementations.

Which ones? example-schema and other implementations have it in
different place. If you align some parts of the code, please align to
match the correct code, not other incorrect. How to find it?
example-schema is the reference, recent commits are also sometimes good
indication.

Best regards,
Krzysztof


2024-01-30 12:36:03

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: pinctrl: cy8x95x0: Minor fix & update

Hi Krzysztof,


On Tue, 30 Jan 2024 at 18:01, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 30/01/2024 13:26, Naresh Solanki wrote:
> > Hi Krzysztof,
> >
> > On Tue, 30 Jan 2024 at 17:49, Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 30/01/2024 13:08, Naresh Solanki wrote:
> >>> Update maxItems to 60 for gpio-reserved-ranges to allow multiple gpio
> >>> reserved ranges.
> >>> Add input-enable property to allow configuring a pin as input.
> >>> Rearrange allOf
> >>
> >> Why?
> > allOf is aligned with other implementations.
>
> Which ones? example-schema and other implementations have it in
> different place. If you align some parts of the code, please align to
> match the correct code, not other incorrect. How to find it?
> example-schema is the reference, recent commits are also sometimes good
> indication.
Yes your right. Will align with example-schema. Thanks.

Regards,
Naresh
>
> Best regards,
> Krzysztof
>