Add Apple macsmc GPIO support
Subsystems (3)
cover Add Apple macsmc GPIO support
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
Signature · 2 lines
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
Add the DT binding for the Apple Mac System Management Controller GPIOs.
--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +28 -0
@@ -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 <[email protected]> + +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
Signature · 1 line
2.30.2
Re: [PATCH 1/2] dt-bindings: gpio: add binding for the GPIO block for Apple Mac SMC
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 13 lines · Russell King (Oracle)
Add the DT binding for the Apple Mac System Management Controller GPIOs.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yamlDocumentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +0
-0
Filename based on compatible, so "apple,smc-gpio.yaml"
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +3
-0@@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +10
-0+$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 <[email protected]> + +description: + This describes the binding for the Apple Mac System Management Controller
Drop "This describes the binding for"
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +5
-0+ 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.
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 26
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +8
-0+ - 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.
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 28
Quoted 2 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +2
-0+ +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
Quoted 9 lines · Krzysztof Kozlowski
Quoted 8 lines · Russell King (Oracle)
Add the DT binding for the Apple Mac System Management Controller GPIOs.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +0
-0Filename based on compatible, so "apple,smc-gpio.yaml"
Many of the other yaml files in gpio/ are named as such.
Quoted 17 lines
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +3
-0@@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +10
-0+$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 <[email protected]> + +description: + This describes the binding for the Apple Mac System Management ControllerDrop "This describes the binding for"
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +5
-0+ 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#
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 26
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +8
-0+ - enum: + - apple,t8103-smc + - const: apple,smc-gpio + + gpio-controller: true + + '#gpio-cells': + const: 2Missing required properties. Start from new bindings or example-schema.
What's missing? All the other bindings that I see follow this pattern.
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 28
Quoted 2 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +2
-0+ +additionalProperties: falseMissing 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!
Signature · 2 lines
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
Quoted 18 lines · Russell King (Oracle)
Quoted 9 lines · Krzysztof Kozlowski
Quoted 8 lines · Russell King (Oracle)
Add the DT binding for the Apple Mac System Management Controller GPIOs.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yamlQuoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +0
-0Filename based on compatible, so "apple,smc-gpio.yaml"
Quoted 1 line
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.
Quoted 27 lines
Quoted 18 lines
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +3
-0@@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +10
-0+$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 <[email protected]> + +description: + This describes the binding for the Apple Mac System Management ControllerDrop "This describes the binding for"
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +5
-0+ 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.
Quoted 1 line
Really? You're joking.
No. If you look at example-schema then answer should be obvious, so why do you think I am joking?
Quoted 8 lines
I had sent an email to Rob to ask how this should
be done because my first guess spat out unhelpful error messages fromdt_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_checkspat errors, so that's clearly incorrect. Specifically, I tried:
Quoted 8 lines
properties:
compatible:
- items:
- enum:
- apple,t8103-smc
- const: apple,smc-gpioThat 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.
Quoted 14 lines
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#Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 26
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +8
-0+ - enum: + - apple,t8103-smc + - const: apple,smc-gpio + + gpio-controller: true + + '#gpio-cells': + const: 2Missing required properties. Start from new bindings or example-schema.
Quoted 1 line
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.
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 28
Quoted 3 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +2
-0+ +additionalProperties: falseMissing 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.
Quoted 7 lines
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
Quoted 74 lines · Krzysztof Kozlowski
Quoted 18 lines · Russell King (Oracle)
Quoted 9 lines · Krzysztof Kozlowski
Quoted 8 lines · Russell King (Oracle)
Add the DT binding for the Apple Mac System Management Controller GPIOs.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yamlQuoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +0
-0Filename based on compatible, so "apple,smc-gpio.yaml"
Quoted 1 line
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.
Quoted 27 lines
Quoted 18 lines
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +3
-0@@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +10
-0+$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 <[email protected]> + +description: + This describes the binding for the Apple Mac System Management ControllerDrop "This describes the binding for"
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +5
-0+ 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.
Quoted 1 line
Really? You're joking.
No. If you look at example-schema then answer should be obvious, so why do you think I am joking?
Quoted 15 lines
I had sent an email to Rob to ask how this should
be done because my first guess spat out unhelpful error messages fromdt_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_checkspat errors, so that's clearly incorrect. Specifically, I tried:properties:
compatible:
- items:
- enum:
- apple,t8103-smc
- const: apple,smc-gpio
Quoted 7 lines
Quoted 1 line
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.
Quoted 13 lines
Quoted 13 lines
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#Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 26
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +8
-0+ - enum: + - apple,t8103-smc + - const: apple,smc-gpio + + gpio-controller: true + + '#gpio-cells': + const: 2Missing required properties. Start from new bindings or example-schema.
Quoted 4 lines
Quoted 1 line
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.
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 28
Quoted 2 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +2
-0+ +additionalProperties: falseMissing 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.
Quoted 10 lines
Quoted 6 lines
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.
Signature · 2 lines
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
Quoted 83 lines · Russell King (Oracle)
Quoted 74 lines · Krzysztof Kozlowski
Quoted 18 lines · Russell King (Oracle)
Quoted 9 lines · Krzysztof Kozlowski
Quoted 8 lines · Russell King (Oracle)
Add the DT binding for the Apple Mac System Management Controller GPIOs.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../devicetree/bindings/gpio/gpio-macsmc.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-macsmc.yamlQuoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +0
-0Filename based on compatible, so "apple,smc-gpio.yaml"
Quoted 1 line
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.
Quoted 27 lines
Quoted 18 lines
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +3
-0@@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +10
-0+$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 <[email protected]> + +description: + This describes the binding for the Apple Mac System Management ControllerDrop "This describes the binding for"
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +5
-0+ 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.
Quoted 1 line
Really? You're joking.
No. If you look at example-schema then answer should be obvious, so why do you think I am joking?
Quoted 15 lines
I had sent an email to Rob to ask how this should
be done because my first guess spat out unhelpful error messages fromdt_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_checkspat errors, so that's clearly incorrect. Specifically, I tried:properties:
compatible:
- items:
- enum:
- apple,t8103-smc
- const: apple,smc-gpioQuoted 7 lines
Quoted 1 line
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.
Quoted 7 lines
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...
Quoted 2 lines
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?
Quoted 1 line
messages make it totally unobvious that's what the problem was.
The error messages could indeed be improved, I agree.
Quoted 22 lines
Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml line 20
Quoted 3 lines
Documentation/devicetree/bindings/gpio/gpio-macsmc.yaml new file +2
-0+ +additionalProperties: falseMissing 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.
Quoted 7 lines
Quoted 6 lines
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!
Quoted 5 lines
Quoted 3 lines
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.
Quoted 11 lines
make no sense when you make this accusations, because they are totally untrue.
I started with:
properties:
compatible:
- enum:
- ...
- const: ...
Quoted 4 lines
and
dt_bindings_checkthew it out. So I looked again atDocumentation/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
Quoted 6 lines
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.
Quoted 9 lines
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.
Quoted 8 lines
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
Quoted 6 lines · Krzysztof Kozlowski
Quoted 4 lines · Russell King (Oracle)
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.
Quoted 6 lines
There is...
Quoted 2 lines
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! :)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
PATCH 2/2 gpio: Add new gpio-macsmc driver for Apple Macs
In reply to Russell King (Oracle)From: Hector Martin <[email protected]>
This driver implements the GPIO service on top of the SMC framework on Apple Mac machines. In particular, these are the GPIOs present in the PMU IC which are used to control power to certain on-board devices.
Although the underlying hardware supports various pin config settings (input/output, open drain, etc.), this driver does not implement that functionality and leaves it up to the firmware to configure things properly. We also don't yet support interrupts/events. This is sufficient for device power control, which is the only thing we need to support at this point. More features will be implemented when needed.
To our knowledge, only Apple Silicon Macs implement this SMC feature.
--- drivers/gpio/Kconfig | 11 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-macsmc.c | 245 +++++++++++++++++++++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 drivers/gpio/gpio-macsmc.c
drivers/gpio/Kconfig +11 -0
@@ -1292,6 +1292,17 @@ config GPIO_LP87565 This driver can also be built as a module. If so, the module will be called gpio-lp87565. +config GPIO_MACSMC + tristate "Apple Mac SMC GPIO" + depends on APPLE_SMC + default ARCH_APPLE + help + Support for GPIOs controlled by the SMC microcontroller on Apple Mac + systems. + + This driver can also be built as a module. If so, the module will be + called gpio-macsmc. + config GPIO_MADERA tristate "Cirrus Logic Madera class codecs" depends on PINCTRL_MADERA
drivers/gpio/Makefile +1 -0
@@ -83,6 +83,7 @@ obj-$(CONFIG_GPIO_LP873X) += gpio-lp873x.o obj-$(CONFIG_GPIO_LP87565) += gpio-lp87565.o obj-$(CONFIG_GPIO_LPC18XX) += gpio-lpc18xx.o obj-$(CONFIG_GPIO_LPC32XX) += gpio-lpc32xx.o +obj-$(CONFIG_GPIO_MACSMC) += gpio-macsmc.o obj-$(CONFIG_GPIO_MADERA) += gpio-madera.o obj-$(CONFIG_GPIO_MAX3191X) += gpio-max3191x.o obj-$(CONFIG_GPIO_MAX7300) += gpio-max7300.o
drivers/gpio/gpio-macsmc.c new file +245 -0
245 lines not on this page · raw message
Signature · 1 line
2.30.2