[RFC,v5,5/5] dt-bindings: firmware: arm,scmi: Add support for pinctrl protocol

  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(+)
  

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