[PATCH 0/2] Add Apple macsmc GPIO support

[PATCH 0/2] Add Apple macsmc GPIO support

[Russell King (Oracle)]

Hi,

This series adds support for the Apple Mac SMC GPIO driver that was
previously posted before the last merge window. I believe all previous
review points have been addressed.

This series is smaller than the previous posting because I'm splitting
this up to allow progress to be made; this driver is dependent on the
Apple SMC driver in order to be buildable and usable. It is expected
that this Apple SMC driver will be merged via Lee's MFD tree. The
Kconfig dependencies this driver from causing build issues until that
driver is also merged, so I believe it is appropriate to send this
separately.

There is no issue if we wish to delay applying this until the core
driver has been accepted by Lee.

The series is made up of two patches - one patch for the bindings,
which now include two compatibles, one specific and one generic. The
second patch adds the actual driver itself.

 .../devicetree/bindings/gpio/gpio-macsmc.yaml      |  28 +++
 drivers/gpio/Kconfig                               |  11 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-macsmc.c                         | 245 +++++++++++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
 create mode 100644 drivers/gpio/gpio-macsmc.c

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

[PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Russell King (Oracle)]

Add the DT binding for the Apple Mac System Management Controller GPIOs.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml

diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
new file mode 100644
index 000000000000..a3883d62292d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Mac System Management Controller GPIO
+
+maintainers:
+  - Hector Martin <marcan@marcan.st>
+
+description:
+  This describes the binding for the Apple Mac System Management Controller
+  GPIO block.
+
+properties:
+  compatible:
+    allOf:
+      - enum:
+          - apple,t8103-smc
+      - const: apple,smc-gpio
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+additionalProperties: false
-- 
2.30.2

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Krzysztof Kozlowski]

On 27/10/2022 13:03, Russell King (Oracle) wrote:
> Add the DT binding for the Apple Mac System Management Controller GPIOs.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> new file mode 100644
> index 000000000000..a3883d62292d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml


Filename based on compatible, so "apple,smc-gpio.yaml"

> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Mac System Management Controller GPIO
> +
> +maintainers:
> +  - Hector Martin <marcan@marcan.st>
> +
> +description:
> +  This describes the binding for the Apple Mac System Management Controller

Drop "This describes the binding for"

> +  GPIO block.
> +
> +properties:
> +  compatible:
> +    allOf:

That's not proper syntax. Look at other examples (e.g. Apple bindings)
doing it. Probably you wanted items here.

> +      - enum:
> +          - apple,t8103-smc
> +      - const: apple,smc-gpio
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2

Missing required properties. Start from new bindings or example-schema.

> +
> +additionalProperties: false

Missing example, it's necessary to validate these.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Russell King (Oracle)]

On Thu, Oct 27, 2022 at 03:53:25PM -0400, Krzysztof Kozlowski wrote:
> On 27/10/2022 13:03, Russell King (Oracle) wrote:
> > Add the DT binding for the Apple Mac System Management Controller GPIOs.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> > new file mode 100644
> > index 000000000000..a3883d62292d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> 
> Filename based on compatible, so "apple,smc-gpio.yaml"

Many of the other yaml files in gpio/ are named as such.

> > @@ -0,0 +1,28 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple Mac System Management Controller GPIO
> > +
> > +maintainers:
> > +  - Hector Martin <marcan@marcan.st>
> > +
> > +description:
> > +  This describes the binding for the Apple Mac System Management Controller
> 
> Drop "This describes the binding for"
> 
> > +  GPIO block.
> > +
> > +properties:
> > +  compatible:
> > +    allOf:
> 
> That's not proper syntax. Look at other examples (e.g. Apple bindings)
> doing it. Probably you wanted items here.

Really? You're joking. I had sent an email to Rob to ask how this should
be done because my first guess spat out unhelpful error messages from
dt_bindings_check, and this is the best I could come up with based on
other "examples".

I tried "- items:" but that made no difference - dt_bindings_check spat
errors, so that's clearly incorrect. Specifically, I tried:

properties:
  compatible:
    - items:
        - enum:
	    - apple,t8103-smc
	- const: apple,smc-gpio

That doesn't work:

Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml: properties:compatible: [{'items': [{'const': 'apple,t8103-smc'}, {'const': 'apple,smc-gpio'}]}] is not of type 'object', 'boolean'  from schema $id: http://json-schema.org/draft-07/schema#

> > +      - enum:
> > +          - apple,t8103-smc
> > +      - const: apple,smc-gpio
> > +
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    const: 2
> 
> Missing required properties. Start from new bindings or example-schema.

What's missing? All the other bindings that I see follow this pattern.

> > +
> > +additionalProperties: false
> 
> Missing example, it's necessary to validate these.

Documentation states that examples are optional according to the
"writing-schema" documentation.

Honestly, I find this YAML stuff extremely difficult, especially given
the lack of documentation on how to write it and the cryptic error
messages from the tooling. It's impossible to get it right before
submitting it - and I suspect from what I see above, it's impossible
for reviewers to know what is correct either, since some of what you've
said above appears to be wrong!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Krzysztof Kozlowski]

On 27/10/2022 17:00, Russell King (Oracle) wrote:
> On Thu, Oct 27, 2022 at 03:53:25PM -0400, Krzysztof Kozlowski wrote:
>> On 27/10/2022 13:03, Russell King (Oracle) wrote:
>>> Add the DT binding for the Apple Mac System Management Controller GPIOs.
>>>
>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>> ---
>>>  .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>> new file mode 100644
>>> index 000000000000..a3883d62292d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>
>> Filename based on compatible, so "apple,smc-gpio.yaml"
> 
> Many of the other yaml files in gpio/ are named as such.

Poor patterns, inconsistencies or even bugs like to copy themselves and
it is never an argument.

The convention for all bindings is to use vendor,device.yaml, matching
the compatible when applicable.

> 
>>> @@ -0,0 +1,28 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Apple Mac System Management Controller GPIO
>>> +
>>> +maintainers:
>>> +  - Hector Martin <marcan@marcan.st>
>>> +
>>> +description:
>>> +  This describes the binding for the Apple Mac System Management Controller
>>
>> Drop "This describes the binding for"
>>
>>> +  GPIO block.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    allOf:
>>
>> That's not proper syntax. Look at other examples (e.g. Apple bindings)
>> doing it. Probably you wanted items here.
> 
> Really? You're joking. 

No. If you look at example-schema then answer should be obvious, so why
do you think I am joking?

> I had sent an email to Rob to ask how this should
> be done because my first guess spat out unhelpful error messages from
> dt_bindings_check, and this is the best I could come up with based on
> other "examples".
> 
> I tried "- items:" but that made no difference - dt_bindings_check spat
> errors, so that's clearly incorrect. Specifically, I tried:
> 
> properties:
>   compatible:
>     - items:
>         - enum:
> 	    - apple,t8103-smc
> 	- const: apple,smc-gpio
> 
> That doesn't work:

Of course, because "-" means list, so "- items" is not correct.

Where do you see such pattern? Anywhere following compatible? No. There
is no. You just invented something instead of using many, many existing
examples.


> 
> Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml: properties:compatible: [{'items': [{'const': 'apple,t8103-smc'}, {'const': 'apple,smc-gpio'}]}] is not of type 'object', 'boolean'  from schema $id: http://json-schema.org/draft-07/schema#
> 
>>> +      - enum:
>>> +          - apple,t8103-smc
>>> +      - const: apple,smc-gpio
>>> +
>>> +  gpio-controller: true
>>> +
>>> +  '#gpio-cells':
>>> +    const: 2
>>
>> Missing required properties. Start from new bindings or example-schema.
> 
> What's missing? All the other bindings that I see follow this pattern.

No. All other bindings have list of required properties. Only yours do
not have.

> 
>>> +
>>> +additionalProperties: false
>>
>> Missing example, it's necessary to validate these.
> 
> Documentation states that examples are optional according to the
> "writing-schema" documentation.

Yes, but without it we cannot validate the bindings.

> 
> Honestly, I find this YAML stuff extremely difficult, especially given
> the lack of documentation on how to write it and the cryptic error
> messages from the tooling. It's impossible to get it right before
> submitting it - and I suspect from what I see above, it's impossible
> for reviewers to know what is correct either, since some of what you've
> said above appears to be wrong!

I would say it is doable - copy example-schema or recent device specific
schema and customize it... But you started adding some weird stuff which
was never, never in other bindings.

I would also say that C stuff is extremely difficult... Impossible to
get it right. Wait...

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Russell King (Oracle)]

On Thu, Oct 27, 2022 at 05:31:49PM -0400, Krzysztof Kozlowski wrote:
> On 27/10/2022 17:00, Russell King (Oracle) wrote:
> > On Thu, Oct 27, 2022 at 03:53:25PM -0400, Krzysztof Kozlowski wrote:
> >> On 27/10/2022 13:03, Russell King (Oracle) wrote:
> >>> Add the DT binding for the Apple Mac System Management Controller GPIOs.
> >>>
> >>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> >>> ---
> >>>  .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
> >>>  1 file changed, 28 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> >>> new file mode 100644
> >>> index 000000000000..a3883d62292d
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
> >>
> >> Filename based on compatible, so "apple,smc-gpio.yaml"
> > 
> > Many of the other yaml files in gpio/ are named as such.
> 
> Poor patterns, inconsistencies or even bugs like to copy themselves and
> it is never an argument.
> 
> The convention for all bindings is to use vendor,device.yaml, matching
> the compatible when applicable.
> 
> > 
> >>> @@ -0,0 +1,28 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Apple Mac System Management Controller GPIO
> >>> +
> >>> +maintainers:
> >>> +  - Hector Martin <marcan@marcan.st>
> >>> +
> >>> +description:
> >>> +  This describes the binding for the Apple Mac System Management Controller
> >>
> >> Drop "This describes the binding for"
> >>
> >>> +  GPIO block.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    allOf:
> >>
> >> That's not proper syntax. Look at other examples (e.g. Apple bindings)
> >> doing it. Probably you wanted items here.
> > 
> > Really? You're joking. 
> 
> No. If you look at example-schema then answer should be obvious, so why
> do you think I am joking?
> 
> > I had sent an email to Rob to ask how this should
> > be done because my first guess spat out unhelpful error messages from
> > dt_bindings_check, and this is the best I could come up with based on
> > other "examples".
> > 
> > I tried "- items:" but that made no difference - dt_bindings_check spat
> > errors, so that's clearly incorrect. Specifically, I tried:
> > 
> > properties:
> >   compatible:
> >     - items:
> >         - enum:
> > 	    - apple,t8103-smc
> > 	- const: apple,smc-gpio
> > 
> > That doesn't work:
> 
> Of course, because "-" means list, so "- items" is not correct.
> 
> Where do you see such pattern? Anywhere following compatible? No. There
> is no. You just invented something instead of using many, many existing
> examples.

No, I did not "invent" something here. I tried to copy it from other
examples, but I couldn't find something that matched exactly.

In any case, relying on examples rather than a proper description of
how this should be done is utterly rediculous. There should be a formal
definition of the language used to describe this - but there doesn't
seem to be.

So, stuff like "-" means list is just not obvious, and the error
messages make it totally unobvious that's what the problem was.

> > Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml: properties:compatible: [{'items': [{'const': 'apple,t8103-smc'}, {'const': 'apple,smc-gpio'}]}] is not of type 'object', 'boolean'  from schema $id: http://json-schema.org/draft-07/schema#
> > 
> >>> +      - enum:
> >>> +          - apple,t8103-smc
> >>> +      - const: apple,smc-gpio
> >>> +
> >>> +  gpio-controller: true
> >>> +
> >>> +  '#gpio-cells':
> >>> +    const: 2
> >>
> >> Missing required properties. Start from new bindings or example-schema.
> > 
> > What's missing? All the other bindings that I see follow this pattern.
> 
> No. All other bindings have list of required properties. Only yours do
> not have.

Oh, you don't mean there are required properties missing from
#gpio-cells, you mean the list of required properties is missing,
which is something _entirely_ different. Your comment was utterly
ambiguous.

> >>> +
> >>> +additionalProperties: false
> >>
> >> Missing example, it's necessary to validate these.
> > 
> > Documentation states that examples are optional according to the
> > "writing-schema" documentation.
> 
> Yes, but without it we cannot validate the bindings.

Please update the writing-schema documentation to state that it's now
required to validate bindings, so that the documentation is no longer
stating something that's different from the required process.

> > Honestly, I find this YAML stuff extremely difficult, especially given
> > the lack of documentation on how to write it and the cryptic error
> > messages from the tooling. It's impossible to get it right before
> > submitting it - and I suspect from what I see above, it's impossible
> > for reviewers to know what is correct either, since some of what you've
> > said above appears to be wrong!
> 
> I would say it is doable - copy example-schema or recent device specific
> schema and customize it... But you started adding some weird stuff which
> was never, never in other bindings.

"weird stuff"? What weird stuff? What wasn't in other bindings? You
make no sense when you make this accusations, because they are totally
untrue.

I started with:

properties:
  compatible:
    - enum:
       - ...
    - const: ...

and dt_bindings_check thew it out. So I looked again at
Documentation/bindings/gpio/*.yaml. I decided maybe the - enum
containing one entry could be confusing matters, so I tried converting
that to a - const. Still failed.

So I had another look at other *.yaml files, and I then tried adding
- items: and indenting the following. Failed.

So then I tried allOf: which passed the checks. That's the evolution
there - trial and error.

Cryptic error messages, nothing else in gpio/ that follows the pattern
I wanted and trial and error led me to what I had in this patch. This
is *no* way to develop bindings.

There has to be a formal definition of this schema language - and
something better than pointing people at other bindings that may or
may not be correct.

So, I repeat myself: writing yaml stuff is utterly horrid and a total
hit and miss affair whether one gets it correct or not.

It seems to me that the problem of validating .dts files hasn't been
solved - the problem has merely been moved to a whole set of different
problems trying to write .yaml files that allow .dts files to be
validated, some of which could be solved by a better understanding of
the syntax, if only it were documented properly.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Krzysztof Kozlowski]

On 27/10/2022 17:49, Russell King (Oracle) wrote:
> On Thu, Oct 27, 2022 at 05:31:49PM -0400, Krzysztof Kozlowski wrote:
>> On 27/10/2022 17:00, Russell King (Oracle) wrote:
>>> On Thu, Oct 27, 2022 at 03:53:25PM -0400, Krzysztof Kozlowski wrote:
>>>> On 27/10/2022 13:03, Russell King (Oracle) wrote:
>>>>> Add the DT binding for the Apple Mac System Management Controller GPIOs.
>>>>>
>>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>>>> ---
>>>>>  .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++
>>>>>  1 file changed, 28 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..a3883d62292d
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
>>>>
>>>> Filename based on compatible, so "apple,smc-gpio.yaml"
>>>
>>> Many of the other yaml files in gpio/ are named as such.
>>
>> Poor patterns, inconsistencies or even bugs like to copy themselves and
>> it is never an argument.
>>
>> The convention for all bindings is to use vendor,device.yaml, matching
>> the compatible when applicable.
>>
>>>
>>>>> @@ -0,0 +1,28 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Apple Mac System Management Controller GPIO
>>>>> +
>>>>> +maintainers:
>>>>> +  - Hector Martin <marcan@marcan.st>
>>>>> +
>>>>> +description:
>>>>> +  This describes the binding for the Apple Mac System Management Controller
>>>>
>>>> Drop "This describes the binding for"
>>>>
>>>>> +  GPIO block.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    allOf:
>>>>
>>>> That's not proper syntax. Look at other examples (e.g. Apple bindings)
>>>> doing it. Probably you wanted items here.
>>>
>>> Really? You're joking. 
>>
>> No. If you look at example-schema then answer should be obvious, so why
>> do you think I am joking?
>>
>>> I had sent an email to Rob to ask how this should
>>> be done because my first guess spat out unhelpful error messages from
>>> dt_bindings_check, and this is the best I could come up with based on
>>> other "examples".
>>>
>>> I tried "- items:" but that made no difference - dt_bindings_check spat
>>> errors, so that's clearly incorrect. Specifically, I tried:
>>>
>>> properties:
>>>   compatible:
>>>     - items:
>>>         - enum:
>>> 	    - apple,t8103-smc
>>> 	- const: apple,smc-gpio
>>>
>>> That doesn't work:
>>
>> Of course, because "-" means list, so "- items" is not correct.
>>
>> Where do you see such pattern? Anywhere following compatible? No. There
>> is no. You just invented something instead of using many, many existing
>> examples.
> 
> No, I did not "invent" something here. I tried to copy it from other
> examples, but I couldn't find something that matched exactly.
> 
> In any case, relying on examples rather than a proper description of
> how this should be done is utterly rediculous. There should be a formal
> definition of the language used to describe this - but there doesn't
> seem to be.

There is...

> 
> So, stuff like "-" means list is just not obvious, and the error

"-" is defined by YAML, so I do not understand what is here not obvious?

> messages make it totally unobvious that's what the problem was.

The error messages could indeed be improved, I agree.

> 
>>>>> +
>>>>> +additionalProperties: false
>>>>
>>>> Missing example, it's necessary to validate these.
>>>
>>> Documentation states that examples are optional according to the
>>> "writing-schema" documentation.
>>
>> Yes, but without it we cannot validate the bindings.
> 
> Please update the writing-schema documentation to state that it's now
> required to validate bindings, so that the documentation is no longer
> stating something that's different from the required process.
> 
>>> Honestly, I find this YAML stuff extremely difficult, especially given
>>> the lack of documentation on how to write it and the cryptic error
>>> messages from the tooling. It's impossible to get it right before
>>> submitting it - and I suspect from what I see above, it's impossible
>>> for reviewers to know what is correct either, since some of what you've
>>> said above appears to be wrong!
>>
>> I would say it is doable - copy example-schema or recent device specific
>> schema and customize it... But you started adding some weird stuff which
>> was never, never in other bindings.
> 
> "weird stuff"? What weird stuff? What wasn't in other bindings? You

The "allOf" in compatible is the weird stuff.

There is basically just one case with it, a special case. There are no
other bindings using such pattern.

> make no sense when you make this accusations, because they are totally
> untrue.
> 
> I started with:
> 
> properties:
>   compatible:
>     - enum:
>        - ...
>     - const: ...
> 
> and dt_bindings_check thew it out. So I looked again at
> Documentation/bindings/gpio/*.yaml. I decided maybe the - enum
> containing one entry could be confusing matters, so I tried converting
> that to a - const. Still failed.

Fourth YAML binding (counting alphabetically) in gpios has it:
Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml

> 
> So I had another look at other *.yaml files, and I then tried adding
> - items: and indenting the following. Failed.
> 
> So then I tried allOf: which passed the checks. That's the evolution
> there - trial and error.

I understand your process. I still think that easier is to start from
example-schema as it has this case exactly.

> 
> Cryptic error messages, nothing else in gpio/ that follows the pattern
> I wanted and trial and error led me to what I had in this patch. This
> is *no* way to develop bindings.
> 
> There has to be a formal definition of this schema language - and
> something better than pointing people at other bindings that may or
> may not be correct.
> 
> So, I repeat myself: writing yaml stuff is utterly horrid and a total
> hit and miss affair whether one gets it correct or not.
> 
> It seems to me that the problem of validating .dts files hasn't been
> solved - the problem has merely been moved to a whole set of different
> problems trying to write .yaml files that allow .dts files to be
> validated, some of which could be solved by a better understanding of
> the syntax, if only it were documented properly.

I repeat also myself, writing C is also difficult and horrid :)

Anyway, your feedback is of course appreciated. Happy to help if you
have some more questions.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC

[Russell King (Oracle)]

On Thu, Oct 27, 2022 at 06:06:21PM -0400, Krzysztof Kozlowski wrote:
> On 27/10/2022 17:49, Russell King (Oracle) wrote:
> > In any case, relying on examples rather than a proper description of
> > how this should be done is utterly rediculous. There should be a formal
> > definition of the language used to describe this - but there doesn't
> > seem to be.
> 
> There is...
> 
> > 
> > So, stuff like "-" means list is just not obvious, and the error
> 
> "-" is defined by YAML, so I do not understand what is here not obvious?

I wonder which bit of "I don't understand YAML" isn't obvious to you?

The "-" means list isn't stated in yaml online tutorials, it seems
one is just supposed to know this. I guess if one been in JS and
similar landscapes it's obvious, but not everyone has.

C is damn easy in comparison! :)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!