| Message ID | e9285b4377242e4d888391be987cbb99caf8c573.1698353854.git.oleksii_moisieiev@epam.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On 27/10/2023 08:28, Oleksii Moisieiev wrote: > Add new SCMI v3.2 pinctrl protocol bindings definitions and example. > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> > > --- > Changes v3 -> v4 > - reworked protocol@19 format > --- > .../bindings/firmware/arm,scmi.yaml | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > index 5824c43e9893..5318fe72354e 100644 > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > @@ -233,6 +233,39 @@ properties: > reg: > const: 0x18 > > + protocol@19: > + type: object > + allOf: > + - $ref: "#/$defs/protocol-node" > + - $ref: "../pinctrl/pinctrl.yaml" This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. Best regards, Krzysztof
On Fri, 27 Oct 2023 06:28:11 +0000, Oleksii Moisieiev wrote: > Add new SCMI v3.2 pinctrl protocol bindings definitions and example. > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> > > --- > Changes v3 -> v4 > - reworked protocol@19 format > --- > .../bindings/firmware/arm,scmi.yaml | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) > 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: ./Documentation/devicetree/bindings/firmware/arm,scmi.yaml:244:15: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/firmware/arm,scmi.yaml:258:19: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/firmware/arm,scmi.yaml:259:19: [error] string value is redundantly quoted with any quotes (quoted-strings) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/e9285b4377242e4d888391be987cbb99caf8c573.1698353854.git.oleksii_moisieiev@epam.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.
On 27.10.23 11:56, Krzysztof Kozlowski wrote: > On 27/10/2023 08:28, Oleksii Moisieiev wrote: >> Add new SCMI v3.2 pinctrl protocol bindings definitions and example. >> >> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> >> >> --- >> Changes v3 -> v4 >> - reworked protocol@19 format >> --- >> .../bindings/firmware/arm,scmi.yaml | 53 +++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> index 5824c43e9893..5318fe72354e 100644 >> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml >> @@ -233,6 +233,39 @@ properties: >> reg: >> const: 0x18 >> >> + protocol@19: >> + type: object >> + allOf: >> + - $ref: "#/$defs/protocol-node" >> + - $ref: "../pinctrl/pinctrl.yaml" > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. > > Thank you. Hi Krzysztof, I'm sorry for the inconvenience. The intention behind sharing this RFC was to initiate the review process for the changes I've been making to the pinctrl driver, aligning it with the updates in the DEN0056E beta2 document. I am still actively working on these changes, and I fully intend to address and resolve the binding issues and all previously mentioned comments in the final v5 patch series. Best regards, Oleksii. > > It does not look like you tested the bindings, at least after quick > look. Please run `make dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > Maybe you need to update your dtschema and yamllint. > > > Best regards, > Krzysztof >
On Fri, Oct 27, 2023 at 8:28 AM Oleksii Moisieiev <Oleksii_Moisieiev@epam.com> wrote: > + keys_pins: keys-pins { > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > + bias-pull-up; > + }; This is kind of interesting and relates to my question about naming groups and functions of GPIO pins. Here we see four pins suspiciously named "GP_*" which I read as "generic purpose" and they are not muxed to *any* function, yes pulled up. I would have expected something like: keys_pins: keys-pins { groups = "GP_5_17_grp", "GP_5_20_grp", "GP_5_22_grp", "GP_2_1_grp"; function = "gpio"; pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; bias-pull-up; }; I hope this illustrates what I see as a problem in not designing in GPIO as an explicit function, I get the impression that these pins are GPIO because it is hardware default. Yours, Linus Walleij
Hi Arm folks, Do you have any comment? I expect that you have had some assumption when you defined SCMI pinctrl protocol specification. On Mon, Nov 06, 2023 at 02:12:36PM +0100, Linus Walleij wrote: > On Fri, Oct 27, 2023 at 8:28???AM Oleksii Moisieiev > <Oleksii_Moisieiev@epam.com> wrote: > > > + keys_pins: keys-pins { > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > > + bias-pull-up; > > + }; > > This is kind of interesting and relates to my question about naming groups and > functions of GPIO pins. > > Here we see four pins suspiciously named "GP_*" which I read as > "generic purpose" > and they are not muxed to *any* function, yes pulled up. > > I would have expected something like: > > keys_pins: keys-pins { > groups = "GP_5_17_grp", "GP_5_20_grp", "GP_5_22_grp", "GP_2_1_grp"; > function = "gpio"; > pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > bias-pull-up; > }; > > I hope this illustrates what I see as a problem in not designing in > GPIO as an explicit > function, I get the impression that these pins are GPIO because it is hardware > default. If you want to stick to "explicit", we may rather introduce a pre-defined sub-node name, "gpio", in a device tree binding, i.e. protocol@19 { // pinctrl protocol ... // other pinmux nodes scmi_gpio: gpio { // "gpio" is a fixed name keys-pins { pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; bias-pull-up; // possibly input or output }; input-pins { groups = "some group"; // any name input-mode; } output-pins { pins = "foo1", "foo2"; // any name output-mode; } } } It would indicate that all the succeeding nodes are for gpio definitions and *virtual* gpio pin numbers will be assigned in the order of appearances in "gpio" node. Then a client driver may refer to a gpio pin (say, GP_2_1?) like in the current manner: foo_device { ... reset-gpios = <&scmi_gpio 3 ...>; } -Takahiro Akashi > > Yours, > Linus Walleij
On Fri, Nov 10, 2023 at 09:58:39AM +0900, Takahiro Akashi wrote: > Hi Arm folks, > > Do you have any comment? > I expect that you have had some assumption when you defined > SCMI pinctrl protocol specification. > [CC Souvik] @Souvik for context see: https://lore.kernel.org/all/CACRpkdZ4GborirSpa3GK_PwMgCvY0ePEmZO+CwnLcP6nAdieow@mail.gmail.com/ Hi, I am not sure what is the full story here, BUT the spec was mainly aimed at supporting PINCTRL in SCMI with the idea to then, later on, base GPIO on top of it, "easily" building on the PINCTRL spec features in the future with a separate series from the one Oleksii is working on...but it like seems the future is already here and maybe we have discovered something to be clarified... Souvik/Oleksii can tell you better what were (if any) further assumptions related to GPIO on top on SCMI/PINCTRL, but the aim of this series was always to be just the basic Generic Pinctrl support when dealing with an SCMI server backend. Regarding the current Pinctrl series by Oleksii, I would also notice that, indeed, some "non-spec-dictated" naming assumptions are ALREADY present somehow, because, currently, the spec and the pinctrl SCMI protocol layer speak/refer about pins/groups/functions, as usual, only in terms of numeric identifiers/IDs (with an associated name of course), while the pinctrl driver (thanks to the Linux pictrl subsystem layer) describes and refers anything in the DT in terms of names: so all of this really works only because the names used in the DT happen to match the names reported by the backend server. My test DT uses just what Oleksii exemplified in the cover letter: pinctrl_i2c2: i2c2 { groups = "i2c2_a", "i2c2_b"; function = "i2c2"; }; pinctrl_mdio: pins_mdio { groups = "avb_mdio"; drive-strength = <24>; }; keys_pins: keys { pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; bias-pull-up; }; with a dummmy test driver referring to it, so as to trigger the drivers core to initialize the pinctrl stuff. But all of this works just because, in the example of my emulated setup, my fake server exposes resources that are exactly named just as how the above DT expects pins/functions/pins to be named, because this is how the Generic Pinctrl subsystem in Linux is supposed to work, right ? The difference is that the names, in the case of pinctrl-scmi, are not hardcoded in the specific pin-controller driver BUT are provided dynamically by the SCMI server at runtime. And this is just a naming convention, between the Linux picntrl subsys AND the SCMI server, that allows the Linux Pinctrl subsys to map, under-hood, names to type/IDs as expected by the SCMI protocol layer (and by the spec): so when you will define and describe a real platform with a DT, you will will have to provide your name references, knowing that the shipped platform SCMI fw will advertise exactly the same (or a superset of them) As such, personally, I would find reasonable to use, equally, some conventional function name like 'gpio' to advertise and configure groups of pins as being used as GPIOs. Maybe, though, both of these expected naming comventions should be explicitly stated in the spec: indeed if you look at some Sensor protocol extensions added in v3.0, in 4.7.2.5.1 "Sensor Axis Descriptors" regarding naming we say: "It is recommended that the name ends with ‘_’ followed by the axis of the sensor in uppercase. For example, the name for the x-axis of a triaxial accelerometer could be “acc_X” or “_X”." ...so maybe some similar remarks could be added here. Souvik is really the one who can have a say about the opportunity (or not) of these kind of explicit advised naming conventions on the spec, so I have CCed him. > On Mon, Nov 06, 2023 at 02:12:36PM +0100, Linus Walleij wrote: > > On Fri, Oct 27, 2023 at 8:28???AM Oleksii Moisieiev > > <Oleksii_Moisieiev@epam.com> wrote: > > > > > + keys_pins: keys-pins { > > > + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > > > + bias-pull-up; > > > + }; > > > > This is kind of interesting and relates to my question about naming groups and > > functions of GPIO pins. > > > > Here we see four pins suspiciously named "GP_*" which I read as > > "generic purpose" > > and they are not muxed to *any* function, yes pulled up. > > > > I would have expected something like: > > > > keys_pins: keys-pins { > > groups = "GP_5_17_grp", "GP_5_20_grp", "GP_5_22_grp", "GP_2_1_grp"; > > function = "gpio"; > > pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > > bias-pull-up; > > }; > > > > I hope this illustrates what I see as a problem in not designing in > > GPIO as an explicit > > function, I get the impression that these pins are GPIO because it is hardware > > default. > > If you want to stick to "explicit", we may rather introduce a pre-defined > sub-node name, "gpio", in a device tree binding, i.e. > > protocol@19 { // pinctrl protocol > ... // other pinmux nodes > > scmi_gpio: gpio { // "gpio" is a fixed name > keys-pins { > pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > bias-pull-up; > // possibly input or output > }; > input-pins { > groups = "some group"; // any name > input-mode; > } > output-pins { > pins = "foo1", "foo2"; // any name > output-mode; > } > } > } > I suppose your proposal of a specially named "gpio" node would be another way, BUT it would also mean describing something in the DT that could be discoverable dynamically querying the server (while making the above assumptions about conventions). Thanks, Cristian
Hi, On 10/11/2023 15:24, Cristian Marussi wrote: > On Fri, Nov 10, 2023 at 09:58:39AM +0900, Takahiro Akashi wrote: >> Hi Arm folks, >> > >> Do you have any comment? >> I expect that you have had some assumption when you defined >> SCMI pinctrl protocol specification. >> > > [CC Souvik] > > @Souvik for context see: > https://lore.kernel.org/all/CACRpkdZ4GborirSpa3GK_PwMgCvY0ePEmZO+CwnLcP6nAdieow@mail.gmail.com/ > > Hi, > > I am not sure what is the full story here, BUT the spec was mainly aimed > at supporting PINCTRL in SCMI with the idea to then, later on, base GPIO > on top of it, "easily" building on the PINCTRL spec features in the future > with a separate series from the one Oleksii is working on...but it like > seems the future is already here and maybe we have discovered something > to be clarified... > > Souvik/Oleksii can tell you better what were (if any) further assumptions > related to GPIO on top on SCMI/PINCTRL, but the aim of this series was > always to be just the basic Generic Pinctrl support when dealing with an > SCMI server backend. The initial assumption always was that GPIOs can be considered as a specific function. Note that the spec does not define the types of function and leaves it to the DT binding (or driver) to figure out the function descriptions/names. > > Regarding the current Pinctrl series by Oleksii, I would also notice that, > indeed, some "non-spec-dictated" naming assumptions are ALREADY present > somehow, because, currently, the spec and the pinctrl SCMI protocol layer > speak/refer about pins/groups/functions, as usual, only in terms of numeric > identifiers/IDs (with an associated name of course), while the pinctrl > driver (thanks to the Linux pictrl subsystem layer) describes and refers > anything in the DT in terms of names: so all of this really works only > because the names used in the DT happen to match the names reported by > the backend server. > > My test DT uses just what Oleksii exemplified in the cover letter: > > pinctrl_i2c2: i2c2 { > groups = "i2c2_a", "i2c2_b"; > function = "i2c2"; > }; > > pinctrl_mdio: pins_mdio { > groups = "avb_mdio"; > drive-strength = <24>; > }; > > keys_pins: keys { > pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; > bias-pull-up; > }; > > > with a dummmy test driver referring to it, so as to trigger the drivers > core to initialize the pinctrl stuff. > > But all of this works just because, in the example of my emulated setup, > my fake server exposes resources that are exactly named just as how the > above DT expects pins/functions/pins to be named, because this is how > the Generic Pinctrl subsystem in Linux is supposed to work, right ? > > The difference is that the names, in the case of pinctrl-scmi, are not > hardcoded in the specific pin-controller driver BUT are provided dynamically > by the SCMI server at runtime. > > And this is just a naming convention, between the Linux picntrl subsys AND > the SCMI server, that allows the Linux Pinctrl subsys to map, under-hood, > names to type/IDs as expected by the SCMI protocol layer (and by the spec): > so when you will define and describe a real platform with a DT, you will > will have to provide your name references, knowing that the shipped platform > SCMI fw will advertise exactly the same (or a superset of them) > > As such, personally, I would find reasonable to use, equally, some > conventional function name like 'gpio' to advertise and configure groups > of pins as being used as GPIOs. As a general principle, we dont try to put naming conventions in the spec if it can be easily resolved via DT. If this is proving to be a hassle then we can "recommend" in the spec that pins which can only be GPIOs are named starting "GPIO". Similar for functions. However looking at Linus' comments below, I am not sure we are at that stage yet? Regards, Souvik > > Maybe, though, both of these expected naming comventions should be > explicitly stated in the spec: indeed if you look at some Sensor protocol > extensions added in v3.0, in 4.7.2.5.1 "Sensor Axis Descriptors" > regarding naming we say: > > "It is recommended that the name ends with ‘_’ > followed by the axis of the sensor in uppercase. For > example, the name for the x-axis of a triaxial > accelerometer could be “acc_X” or “_X”." > > ...so maybe some similar remarks could be added here. > > Souvik is really the one who can have a say about the opportunity (or > not) of these kind of explicit advised naming conventions on the spec, > so I have CCed him. > >> On Mon, Nov 06, 2023 at 02:12:36PM +0100, Linus Walleij wrote: >>> On Fri, Oct 27, 2023 at 8:28???AM Oleksii Moisieiev >>> <Oleksii_Moisieiev@epam.com> wrote: >>> >>>> + keys_pins: keys-pins { >>>> + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; >>>> + bias-pull-up; >>>> + }; >>> >>> This is kind of interesting and relates to my question about naming groups and >>> functions of GPIO pins. >>> >>> Here we see four pins suspiciously named "GP_*" which I read as >>> "generic purpose" >>> and they are not muxed to *any* function, yes pulled up. >>> >>> I would have expected something like: >>> >>> keys_pins: keys-pins { >>> groups = "GP_5_17_grp", "GP_5_20_grp", "GP_5_22_grp", "GP_2_1_grp"; >>> function = "gpio"; >>> pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; >>> bias-pull-up; >>> }; >>> >>> I hope this illustrates what I see as a problem in not designing in >>> GPIO as an explicit >>> function, I get the impression that these pins are GPIO because it is hardware >>> default. >> >> If you want to stick to "explicit", we may rather introduce a pre-defined >> sub-node name, "gpio", in a device tree binding, i.e. >> >> protocol@19 { // pinctrl protocol >> ... // other pinmux nodes >> >> scmi_gpio: gpio { // "gpio" is a fixed name >> keys-pins { >> pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; >> bias-pull-up; >> // possibly input or output >> }; >> input-pins { >> groups = "some group"; // any name >> input-mode; >> } >> output-pins { >> pins = "foo1", "foo2"; // any name >> output-mode; >> } >> } >> } >> > > I suppose your proposal of a specially named "gpio" node would be > another way, BUT it would also mean describing something in the DT that > could be discoverable dynamically querying the server (while making the > above assumptions about conventions). > > Thanks, > Cristian
Hi Souvik, thanks for looking into this! On Mon, Nov 13, 2023 at 1:56 PM Souvik Chakravarty <souvik.chakravarty@arm.com> wrote: > The initial assumption always was that GPIOs can be considered as a > specific function. Note that the spec does not define the types of > function and leaves it to the DT binding (or driver) to figure out the > function descriptions/names. Does this mean that each system using pinctrl-SCMI will need to specify the available pins, groups and functions in a device tree binding? For e.g. DT validation using schema? This creates the problem of where to put it since Documentation/devicetree/bindings/firmware/arm,scmi.yaml is all we have, and for schemas to be applicable the implicit assumption is that this is done per-compatible. If we want to use device tree validation of the strings put into the pinctrl node we need to allow for a per-soc compatible under the pinctrl node like: protocol@19 { compatible = "vendor,soc-scmi-pinctrl"; (...) Then a DT schema can be made to match that and check it. I'm uncertain about that because the SCMI binding has nothing like this at the moment, all the protocol nodes are pretty self-describing and don't seem to need any further configuration to be used, but pin control may be the first instance where we have to add some per-soc configuration into the protocol nodes :/ It's easy to do: + protocol@19: + type: object + allOf: + - $ref: "#/$defs/protocol-node" + - $ref: "../pinctrl/pinctrl.yaml" + unevaluatedProperties: false + + properties: compatible: items: - enum: - vendor1,soc1-scmi-pinctrl - vendor2,soc2-scmi-pinctrl - vendor3,soc3-scmi-pinctrl This should be enough for just establishing the different pin control configurations we can have in the device tree. We are then able to put a more detailed schema for the specific SoC pin control, such as a list of valid groups and functions etc under the ordinary pinctrl bindings such as Documentation/devicetree/bindings/pinctrl/vendor1,soc1-scmi-pinctrl.yaml etc. We should preferably put some pattern like this in place from day 1 so developers know what is expected here. A mock SoC is fine for the time being (we can delete it later when there are some serious ones). I'm uncertain because it feels like a first thing, but I can't really think how it would work otherwise, part of me don't want to pollute the SCMI binding with any per-soc compatibles, but yet since these group and function strings will be per-soc I don't see any other way, if they are supposed to be validated with schema. Yours, Linus Walleij
Hi Linus, On 13/11/2023 13:32, Linus Walleij wrote: > Hi Souvik, > > thanks for looking into this! > > On Mon, Nov 13, 2023 at 1:56 PM Souvik Chakravarty > <souvik.chakravarty@arm.com> wrote: > >> The initial assumption always was that GPIOs can be considered as a >> specific function. Note that the spec does not define the types of >> function and leaves it to the DT binding (or driver) to figure out the >> function descriptions/names. > > Does this mean that each system using pinctrl-SCMI will need > to specify the available pins, groups and functions in a device tree > binding? For e.g. DT validation using schema? Sorry seems I made a typo above ("descriptions/names" should have been "description from names") which resulted in turning things on its head. I really meant that the driver has to figure out the exact type or meaning of what the function does from its name. SCMI still continues to provide the list of pins/groups/functions and their names. Regards, Souvik > > This creates the problem of where to put it since > Documentation/devicetree/bindings/firmware/arm,scmi.yaml > is all we have, and for schemas to be applicable the implicit > assumption is that this is done per-compatible. > > If we want to use device tree validation of the strings put into > the pinctrl node we need to allow for a per-soc compatible > under the pinctrl node like: > > protocol@19 { > compatible = "vendor,soc-scmi-pinctrl"; > (...) > > Then a DT schema can be made to match that and check it. > > I'm uncertain about that because the SCMI binding has nothing > like this at the moment, all the protocol nodes are pretty > self-describing and don't seem to need any further configuration > to be used, but pin control may be the first instance where we > have to add some per-soc configuration into the protocol nodes :/ > > It's easy to do: > > + protocol@19: > + type: object > + allOf: > + - $ref: "#/$defs/protocol-node" > + - $ref: "../pinctrl/pinctrl.yaml" > + unevaluatedProperties: false > + > + properties: > > compatible: > items: > - enum: > - vendor1,soc1-scmi-pinctrl > - vendor2,soc2-scmi-pinctrl > - vendor3,soc3-scmi-pinctrl > > This should be enough for just establishing the different > pin control configurations we can have in the device tree. > > We are then able to put a more detailed schema for the > specific SoC pin control, such as a list of valid groups and > functions etc under the ordinary pinctrl bindings such as > Documentation/devicetree/bindings/pinctrl/vendor1,soc1-scmi-pinctrl.yaml > etc. > > We should preferably put some pattern like this in place from > day 1 so developers know what is expected here. A mock > SoC is fine for the time being (we can delete it later when there > are some serious ones). > > I'm uncertain because it feels like a first thing, but I can't really > think how it would work otherwise, part of me don't want to > pollute the SCMI binding with any per-soc compatibles, but > yet since these group and function strings will be per-soc I don't > see any other way, if they are supposed to be validated > with schema. > > Yours, > Linus Walleij
On Mon, Nov 13, 2023 at 3:23 PM Souvik Chakravarty <souvik.chakravarty@arm.com> wrote: > On 13/11/2023 13:32, Linus Walleij wrote: > > Hi Souvik, > > > > thanks for looking into this! > > > > On Mon, Nov 13, 2023 at 1:56 PM Souvik Chakravarty > > <souvik.chakravarty@arm.com> wrote: > > > >> The initial assumption always was that GPIOs can be considered as a > >> specific function. Note that the spec does not define the types of > >> function and leaves it to the DT binding (or driver) to figure out the > >> function descriptions/names. > > > > Does this mean that each system using pinctrl-SCMI will need > > to specify the available pins, groups and functions in a device tree > > binding? For e.g. DT validation using schema? > > Sorry seems I made a typo above ("descriptions/names" should have been > "description from names") which resulted in turning things on its head. > > I really meant that the driver has to figure out the exact type or > meaning of what the function does from its name. SCMI still continues to > provide the list of pins/groups/functions and their names. Indeed, that's what I imagined. I think the rest of my question spurred by the phrase "leaves it to the DT binding (or driver) to figure out" is actually something Oleksii needs to look into more than a question to you. It should probably come as a review comment to the patch 5/5 itself. Oleksii, what is your take on my question about DT schema validation for different SoCs? Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml index 5824c43e9893..5318fe72354e 100644 --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml @@ -233,6 +233,39 @@ properties: reg: const: 0x18 + protocol@19: + type: object + allOf: + - $ref: "#/$defs/protocol-node" + - $ref: "../pinctrl/pinctrl.yaml" + unevaluatedProperties: false + + properties: + reg: + const: 0x19 + + '#pinctrl-cells': + const: 0 + + patternProperties: + '-pins$': + type: object + allOf: + - $ref: "../pinctrl/pincfg-node.yaml#" + - $ref: "../pinctrl/pinmux-node.yaml#" + unevaluatedProperties: false + + description: + A pin multiplexing sub-node describe how to configure a + set of pins is some desired function. + A single sub-node may define several pin configurations. + This sub-node is using default pinctrl bindings to configure + pin multiplexing and using SCMI protocol to apply specified + configuration using SCMI protocol. + + required: + - reg + additionalProperties: false $defs: @@ -384,6 +417,26 @@ examples: scmi_powercap: protocol@18 { reg = <0x18>; }; + + scmi_pinctrl: protocol@19 { + reg = <0x19>; + #pinctrl-cells = <0>; + + i2c2-pins { + groups = "i2c2_a", "i2c2_b"; + function = "i2c2"; + }; + + mdio-pins { + groups = "avb_mdio"; + drive-strength = <24>; + }; + + keys_pins: keys-pins { + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1"; + bias-pull-up; + }; + }; }; };
Add new SCMI v3.2 pinctrl protocol bindings definitions and example. Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> --- Changes v3 -> v4 - reworked protocol@19 format --- .../bindings/firmware/arm,scmi.yaml | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+)