[v2,1/2] dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions

From: Peng Fan <peng.fan@nxp.com>

i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions. This
patch is to add i.MX95 Pinctrl OEM extensions properties.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
 .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 3 deletions(-)

On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions. This
> patch is to add i.MX95 Pinctrl OEM extensions properties.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
>  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66 ++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index e9d3f043c4ed..e530bfff141a 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -262,9 +262,12 @@ properties:
>      patternProperties:
>        '-pins$':
>          type: object
> -        allOf:
> -          - $ref: /schemas/pinctrl/pincfg-node.yaml#
> -          - $ref: /schemas/pinctrl/pinmux-node.yaml#
> +        anyOf:
> +          - $ref: /schemas/firmware/nxp,imx95-scmi-pinctrl.yaml
> +          - allOf:
> +              - $ref: /schemas/pinctrl/pincfg-node.yaml#
> +              - $ref: /schemas/pinctrl/pinmux-node.yaml#
> +
>          unevaluatedProperties: false
>  
>          description:
> diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
> new file mode 100644
> index 000000000000..f6c18e9fafec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX System Control and Management Interface (SCMI) Pinctrl Protocol
> +
> +maintainers:
> +  - Peng Fan <peng.fan@arm.com>
> +
> +patternProperties:
> +  '[a-f0-9]+$':
> +    type: object
> +    unevaluatedProperties: false
> +
> +    properties:
> +      pins:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: name of the pin
> +        items:
> +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> +                  gpioio00, gpioio01, gpioio02, gpioio03, gpioio04,

ioio?

> +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> +
> +      nxp,func-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Indicates the PAD connections which function module

The description makes no sense...

> +
> +      nxp,pin-conf:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: The PAD configuration

PAD stands for? Or is it pin configuration as the property name says?

This is just a raw register values? Why can't standard pinctrl 
properties be used here. What's the point of using SCMI pinctrl if you 
are just shuttling register values to SCMI?

> +
> +      nxp,daisy-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: The select input register ID
> +
> +      nxp,daisy-conf:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: The select input register value

I remember some daisy chaining stuff in iomuxc, but otherwise 

What's the relationship between these 2 properties? Is it valid if only 
1 is present? I doubt it. Looks like an address and value pair. Either 
the schema needs to define allowed combinations or design the properties 
in a way you don't have too. IOW, make a single property that's a tuple 
of register ID and value.

Rob

Hi Rob,

> Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> for i.MX95 Pinctrl OEM extensions
> 
> On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions. This
> > patch is to add i.MX95 Pinctrl OEM extensions properties.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > ++++++++++++++++++++++
> >  2 files changed, 72 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml

....

> > +
> > +title: i.MX System Control and Management Interface (SCMI) Pinctrl
> > +Protocol
> > +
> > +maintainers:
> > +  - Peng Fan <peng.fan@arm.com>
> > +
> > +patternProperties:
> > +  '[a-f0-9]+$':
> > +    type: object
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      pins:
> > +        $ref: /schemas/types.yaml#/definitions/string
> > +        description: name of the pin
> > +        items:
> > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > +                  gpioio00, gpioio01, gpioio02, gpioio03, gpioio04,
> 
> ioio?

Yes. The name is exported from SCMI firmware.

> 
> > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > +
> > +      nxp,func-id:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: Indicates the PAD connections which function
> > + module
> 
> The description makes no sense...

I will use below description in v3:
For each pin, there are up to 8 muxing options (called ALT modes). For example:
Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance: jtag_mux
010b - Select mux mode: ALT2 mux port: CAN4_RX of instance: imx95_wakeupmix_top
100b - Select mux mode: ALT4 mux port: FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top
101b - Select mux mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
110b - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance: imx95_wakeupmix_top
The nxp,func-id property is selecting one ALT mode.

> 
> > +
> > +      nxp,pin-conf:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: The PAD configuration
> 
> PAD stands for? Or is it pin configuration as the property name says?

It is pin configuration which includes pull up/down, drive strength and etc.

> 
> This is just a raw register values? 

Yes.

Why can't standard pinctrl properties be
> used here. What's the point of using SCMI pinctrl if you are just shuttling
> register values to SCMI?

Standard pinctrl properties will need more memory in our SCMI
firmware design. We have limited on-chip memory for SCMI firmware usage.


> 
> > +
> > +      nxp,daisy-id:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: The select input register ID
> > +
> > +      nxp,daisy-conf:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: The select input register value
> 
> I remember some daisy chaining stuff in iomuxc, but otherwise
> 
> What's the relationship between these 2 properties? Is it valid if only
> 1 is present? I doubt it.

The scmi firmware will convert the daisy-id into register address, and
write the daisy-conf value into the register.

 Looks like an address and value pair. Either the
> schema needs to define allowed combinations

The firmware will check whether id and conf is valid, is there a need to define
allowed combinations? There will be lots of check to define allowed
combinations.

 or design the properties in a
> way you don't have too. IOW, make a single property that's a tuple of register
> ID and value.

Define tuple will make the property not able to use linux generic pinconf
driver. The current parse_dt_cfg for custom_dt_params only support a
single u32 value and not easy to support tuple.


So could we just ignore the allowed combinations and use not tuple?

Thanks,
Peng.

> 
> Rob

On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> Hi Rob,
> 
> > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> > for i.MX95 Pinctrl OEM extensions
> > 
> > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions. This
> > > patch is to add i.MX95 Pinctrl OEM extensions properties.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > ++++++++++++++++++++++
> > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> 
> ....
> 
> > > +
> > > +title: i.MX System Control and Management Interface (SCMI) Pinctrl
> > > +Protocol
> > > +
> > > +maintainers:
> > > +  - Peng Fan <peng.fan@arm.com>
> > > +
> > > +patternProperties:
> > > +  '[a-f0-9]+$':
> > > +    type: object
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      pins:
> > > +        $ref: /schemas/types.yaml#/definitions/string
> > > +        description: name of the pin
> > > +        items:
> > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > +                  gpioio00, gpioio01, gpioio02, gpioio03, gpioio04,
> > 
> > ioio?
> 
> Yes. The name is exported from SCMI firmware.
> 
> > 
> > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > +
> > > +      nxp,func-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: Indicates the PAD connections which function
> > > + module
> > 
> > The description makes no sense...
> 
> I will use below description in v3:
> For each pin, there are up to 8 muxing options (called ALT modes). For example:
> Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance: jtag_mux
> 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance: imx95_wakeupmix_top
> 100b - Select mux mode: ALT4 mux port: FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top
> 101b - Select mux mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> 110b - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance: imx95_wakeupmix_top
> The nxp,func-id property is selecting one ALT mode.

Looks like constraints on the values. Please add schema for them. I 
don't think you need to list an example. 

Perhaps you should be using 'pinmux' property. That is the standard way 
to specify a numeric mux mode.


> > 
> > > +
> > > +      nxp,pin-conf:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: The PAD configuration
> > 
> > PAD stands for? Or is it pin configuration as the property name says?
> 
> It is pin configuration which includes pull up/down, drive strength and etc.
> 
> > 
> > This is just a raw register values? 
> 
> Yes.
> 
> Why can't standard pinctrl properties be
> > used here. What's the point of using SCMI pinctrl if you are just shuttling
> > register values to SCMI?
> 
> Standard pinctrl properties will need more memory in our SCMI
> firmware design. We have limited on-chip memory for SCMI firmware usage.

Then maybe you shouldn't be using SCMI and just write a pinctrl driver. 
Or, Linux has to parse the DT anyways, so use standard properties and 
transform them into your register values and send that to SCMI.

> > > +
> > > +      nxp,daisy-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: The select input register ID
> > > +
> > > +      nxp,daisy-conf:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: The select input register value
> > 
> > I remember some daisy chaining stuff in iomuxc, but otherwise
> > 
> > What's the relationship between these 2 properties? Is it valid if only
> > 1 is present? I doubt it.
> 
> The scmi firmware will convert the daisy-id into register address, and
> write the daisy-conf value into the register.
> 
>  Looks like an address and value pair. Either the
> > schema needs to define allowed combinations
> 
> The firmware will check whether id and conf is valid, is there a need to define
> allowed combinations? There will be lots of check to define allowed
> combinations.

The firmware can't check that the DT contains nxp,daisy-id but not 
nxp,daisy-conf or vice-versa. Of course the kernel could check that, but 
validating the DT is not the kernel's job.


>  or design the properties in a
> > way you don't have too. IOW, make a single property that's a tuple of register
> > ID and value.
> 
> Define tuple will make the property not able to use linux generic pinconf
> driver. The current parse_dt_cfg for custom_dt_params only support a
> single u32 value and not easy to support tuple.

Sounds like a Linux problem. We're talking about bindings. Fix Linux if 
it doesn't meet your needs.


How many bits of id and conf do you need? This too could just be part of 
'pinmux' property. It's value is entirely h/w specific and up to you. 
You just need to fit it into 32-bits (though we could perhaps enlarge 
it).

Rob

Hi Rob, Sudeep, Cristian, Linus

Although this is dt-binding stuff, I hope you could also help suggest how to
support NXP SCMI OEM extensions. There is less chance NXP will change
SCMI firmware to make linux happy using generic conf stuff, because
the SCMI firmware will also need to work for FreeRTOS/GHS/QNX and etc.

> Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> for i.MX95 Pinctrl OEM extensions
> 
> On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> > Hi Rob,
> >
> > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > properties for i.MX95 Pinctrl OEM extensions
> > >
> > > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions.
> > > > This patch is to add i.MX95 Pinctrl OEM extensions properties.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > > ++++++++++++++++++++++
> > > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> >
> > ....
> >
> > > > +
> > > > +title: i.MX System Control and Management Interface (SCMI)
> > > > +Pinctrl Protocol
> > > > +
> > > > +maintainers:
> > > > +  - Peng Fan <peng.fan@arm.com>
> > > > +
> > > > +patternProperties:
> > > > +  '[a-f0-9]+$':
> > > > +    type: object
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      pins:
> > > > +        $ref: /schemas/types.yaml#/definitions/string
> > > > +        description: name of the pin
> > > > +        items:
> > > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > > +                  gpioio00, gpioio01, gpioio02, gpioio03,
> > > > + gpioio04,
> > >
> > > ioio?
> >
> > Yes. The name is exported from SCMI firmware.
> >
> > >
> > > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > > +
> > > > +      nxp,func-id:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: Indicates the PAD connections which function
> > > > + module
> > >
> > > The description makes no sense...
> >
> > I will use below description in v3:
> > For each pin, there are up to 8 muxing options (called ALT modes). For
> example:
> > Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> > 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance:
> > jtag_mux 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance:
> > imx95_wakeupmix_top 100b - Select mux mode: ALT4 mux port:
> > FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top 101b - Select mux
> > mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> 110b
> > - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance:
> imx95_wakeupmix_top The nxp,func-id property is selecting one ALT mode.
> 
> Looks like constraints on the values. Please add schema for them. I don't
> think you need to list an example.

Will add maxItems: 8, minItems: 1
> 
> Perhaps you should be using 'pinmux' property. That is the standard way to
> specify a numeric mux mode.

Ok let me try. 

> 
> 
> > >
> > > > +
> > > > +      nxp,pin-conf:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: The PAD configuration
> > >
> > > PAD stands for? Or is it pin configuration as the property name says?
> >
> > It is pin configuration which includes pull up/down, drive strength and etc.
> >
> > >
> > > This is just a raw register values?
> >
> > Yes.
> >
> > Why can't standard pinctrl properties be
> > > used here. What's the point of using SCMI pinctrl if you are just shuttling
> > > register values to SCMI?
> >
> > Standard pinctrl properties will need more memory in our SCMI
> > firmware design. We have limited on-chip memory for SCMI firmware usage.
> 
> Then maybe you shouldn't be using SCMI and just write a pinctrl driver.

I think I have to. pinctrl-scmi-imx.c was not preferred by ARM SCMI maintainer.

If Linus, Cristian, Sudeep are ok, I could add back pinctrl-scmi-imx.c for
some OEM mapping/unmapping stuff.

> Or, Linux has to parse the DT anyways, so use standard properties and
> transform them into your register values and send that to SCMI.

Cristian, Sudeep, Linus,

Is this ok for you? Before sending to SCMI, linux driver transform
the bits into register value, and send to SCMI firmware.

> 
> > > > +
> > > > +      nxp,daisy-id:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: The select input register ID
> > > > +
> > > > +      nxp,daisy-conf:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: The select input register value
> > >
> > > I remember some daisy chaining stuff in iomuxc, but otherwise
> > >
> > > What's the relationship between these 2 properties? Is it valid if only
> > > 1 is present? I doubt it.
> >
> > The scmi firmware will convert the daisy-id into register address, and
> > write the daisy-conf value into the register.
> >
> >  Looks like an address and value pair. Either the
> > > schema needs to define allowed combinations
> >
> > The firmware will check whether id and conf is valid, is there a need to
> define
> > allowed combinations? There will be lots of check to define allowed
> > combinations.
> 
> The firmware can't check that the DT contains nxp,daisy-id but not
> nxp,daisy-conf or vice-versa. Of course the kernel could check that, but
> validating the DT is not the kernel's job.
> 
> 
> >  or design the properties in a
> > > way you don't have too. IOW, make a single property that's a tuple of
> register
> > > ID and value.
> >
> > Define tuple will make the property not able to use linux generic pinconf
> > driver. The current parse_dt_cfg for custom_dt_params only support a
> > single u32 value and not easy to support tuple.
> 
> Sounds like a Linux problem. We're talking about bindings. Fix Linux if
> it doesn't meet your needs.
> 
> 
> How many bits of id and conf do you need? This too could just be part of
> 'pinmux' property. It's value is entirely h/w specific and up to you.
> You just need to fit it into 32-bits (though we could perhaps enlarge
> it).

For conf, 4 bits is find. For id, 12 bits is fine I think. 

Using pinmux property with some vendor handing code. Not sure
ARM SCMI maintainer is happy or not.

Thanks,
Peng.

> 
> Rob

> Subject: RE: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> for i.MX95 Pinctrl OEM extensions
> 
> Hi Rob, Sudeep, Cristian, Linus

Is it ok to use fsl,pins for i.MX95 SCMI pins, just like previous i.MX processors?
In this way, I need write a new driver, could not reuse pinctrl-scmi.c for i.MX95.
+        properties:
+          fsl,pins:
+            description:
+              each entry consists of 6 integers and represents the mux and config
+              setting for one pin. The first 5 integers <mux_reg conf_reg input_reg
+              mux_val input_val> are specified using a PIN_FUNC_ID macro, which can
+              be found in <arch/arm64/boot/dts/freescale/imx95-pinfunc.h>. The last
+              integer CONFIG is the pad setting value like pull-up on this pin. Please
+              refer to i.MX95 Plus Reference Manual for detailed CONFIG settings.
+            $ref: /schemas/types.yaml#/definitions/uint32-matrix
+            items:
+              items:
+                - description: |
+                    "mux_reg" indicates the offset of mux register.
+                - description: |
+                    "conf_reg" indicates the offset of pad configuration register.
+                - description: |
+                    "input_reg" indicates the offset of select input register.
+                - description: |
+                    "mux_val" indicates the mux value to be applied.
+                - description: |
+                    "input_val" indicates the select input value to be applied.
+                - description: |
+                    "pad_setting" indicates the pad configuration value to be applied.


Thanks,
Peng.
> 
> Although this is dt-binding stuff, I hope you could also help suggest how to
> support NXP SCMI OEM extensions. There is less chance NXP will change
> SCMI firmware to make linux happy using generic conf stuff, because the
> SCMI firmware will also need to work for FreeRTOS/GHS/QNX and etc.
> 
> > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > properties for i.MX95 Pinctrl OEM extensions
> >
> > On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> > > Hi Rob,
> > >
> > > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > > properties for i.MX95 Pinctrl OEM extensions
> > > >
> > > > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions.
> > > > > This patch is to add i.MX95 Pinctrl OEM extensions properties.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > ---
> > > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > > > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > > > ++++++++++++++++++++++
> > > > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > >
> > > ....
> > >
> > > > > +
> > > > > +title: i.MX System Control and Management Interface (SCMI)
> > > > > +Pinctrl Protocol
> > > > > +
> > > > > +maintainers:
> > > > > +  - Peng Fan <peng.fan@arm.com>
> > > > > +
> > > > > +patternProperties:
> > > > > +  '[a-f0-9]+$':
> > > > > +    type: object
> > > > > +    unevaluatedProperties: false
> > > > > +
> > > > > +    properties:
> > > > > +      pins:
> > > > > +        $ref: /schemas/types.yaml#/definitions/string
> > > > > +        description: name of the pin
> > > > > +        items:
> > > > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > > > +                  gpioio00, gpioio01, gpioio02, gpioio03,
> > > > > + gpioio04,
> > > >
> > > > ioio?
> > >
> > > Yes. The name is exported from SCMI firmware.
> > >
> > > >
> > > > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > > > +
> > > > > +      nxp,func-id:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: Indicates the PAD connections which
> > > > > + function module
> > > >
> > > > The description makes no sense...
> > >
> > > I will use below description in v3:
> > > For each pin, there are up to 8 muxing options (called ALT modes).
> > > For
> > example:
> > > Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> > > 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance:
> > > jtag_mux 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance:
> > > imx95_wakeupmix_top 100b - Select mux mode: ALT4 mux port:
> > > FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top 101b - Select mux
> > > mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> > 110b
> > > - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance:
> > imx95_wakeupmix_top The nxp,func-id property is selecting one ALT mode.
> >
> > Looks like constraints on the values. Please add schema for them. I
> > don't think you need to list an example.
> 
> Will add maxItems: 8, minItems: 1
> >
> > Perhaps you should be using 'pinmux' property. That is the standard
> > way to specify a numeric mux mode.
> 
> Ok let me try.
> 
> >
> >
> > > >
> > > > > +
> > > > > +      nxp,pin-conf:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: The PAD configuration
> > > >
> > > > PAD stands for? Or is it pin configuration as the property name says?
> > >
> > > It is pin configuration which includes pull up/down, drive strength and etc.
> > >
> > > >
> > > > This is just a raw register values?
> > >
> > > Yes.
> > >
> > > Why can't standard pinctrl properties be
> > > > used here. What's the point of using SCMI pinctrl if you are just
> > > > shuttling register values to SCMI?
> > >
> > > Standard pinctrl properties will need more memory in our SCMI
> > > firmware design. We have limited on-chip memory for SCMI firmware
> usage.
> >
> > Then maybe you shouldn't be using SCMI and just write a pinctrl driver.
> 
> I think I have to. pinctrl-scmi-imx.c was not preferred by ARM SCMI
> maintainer.
> 
> If Linus, Cristian, Sudeep are ok, I could add back pinctrl-scmi-imx.c for some
> OEM mapping/unmapping stuff.
> 
> > Or, Linux has to parse the DT anyways, so use standard properties and
> > transform them into your register values and send that to SCMI.
> 
> Cristian, Sudeep, Linus,
> 
> Is this ok for you? Before sending to SCMI, linux driver transform the bits into
> register value, and send to SCMI firmware.
> 
> >
> > > > > +
> > > > > +      nxp,daisy-id:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: The select input register ID
> > > > > +
> > > > > +      nxp,daisy-conf:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: The select input register value
> > > >
> > > > I remember some daisy chaining stuff in iomuxc, but otherwise
> > > >
> > > > What's the relationship between these 2 properties? Is it valid if
> > > > only
> > > > 1 is present? I doubt it.
> > >
> > > The scmi firmware will convert the daisy-id into register address,
> > > and write the daisy-conf value into the register.
> > >
> > >  Looks like an address and value pair. Either the
> > > > schema needs to define allowed combinations
> > >
> > > The firmware will check whether id and conf is valid, is there a
> > > need to
> > define
> > > allowed combinations? There will be lots of check to define allowed
> > > combinations.
> >
> > The firmware can't check that the DT contains nxp,daisy-id but not
> > nxp,daisy-conf or vice-versa. Of course the kernel could check that,
> > but validating the DT is not the kernel's job.
> >
> >
> > >  or design the properties in a
> > > > way you don't have too. IOW, make a single property that's a tuple
> > > > of
> > register
> > > > ID and value.
> > >
> > > Define tuple will make the property not able to use linux generic
> > > pinconf driver. The current parse_dt_cfg for custom_dt_params only
> > > support a single u32 value and not easy to support tuple.
> >
> > Sounds like a Linux problem. We're talking about bindings. Fix Linux
> > if it doesn't meet your needs.
> >
> >
> > How many bits of id and conf do you need? This too could just be part
> > of 'pinmux' property. It's value is entirely h/w specific and up to you.
> > You just need to fit it into 32-bits (though we could perhaps enlarge
> > it).
> 
> For conf, 4 bits is find. For id, 12 bits is fine I think.
> 
> Using pinmux property with some vendor handing code. Not sure ARM SCMI
> maintainer is happy or not.
> 
> Thanks,
> Peng.
> 
> >
> > Rob

On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> Hi Rob,
> 
> > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> > for i.MX95 Pinctrl OEM extensions
> > 
> > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions. This
> > > patch is to add i.MX95 Pinctrl OEM extensions properties.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > ++++++++++++++++++++++
> > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> 
> ....
> 
> > > +
> > > +title: i.MX System Control and Management Interface (SCMI) Pinctrl
> > > +Protocol
> > > +
> > > +maintainers:
> > > +  - Peng Fan <peng.fan@arm.com>
> > > +
> > > +patternProperties:
> > > +  '[a-f0-9]+$':
> > > +    type: object
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      pins:
> > > +        $ref: /schemas/types.yaml#/definitions/string
> > > +        description: name of the pin
> > > +        items:
> > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > +                  gpioio00, gpioio01, gpioio02, gpioio03, gpioio04,
> > 
> > ioio?
> 
> Yes. The name is exported from SCMI firmware.
> 
> > 
> > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > +
> > > +      nxp,func-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: Indicates the PAD connections which function
> > > + module
> > 
> > The description makes no sense...
> 
> I will use below description in v3:
> For each pin, there are up to 8 muxing options (called ALT modes). For example:
> Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance: jtag_mux
> 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance: imx95_wakeupmix_top
> 100b - Select mux mode: ALT4 mux port: FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top
> 101b - Select mux mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> 110b - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance: imx95_wakeupmix_top
> The nxp,func-id property is selecting one ALT mode.
> 
> > 
> > > +
> > > +      nxp,pin-conf:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: The PAD configuration
> > 
> > PAD stands for? Or is it pin configuration as the property name says?
> 
> It is pin configuration which includes pull up/down, drive strength and etc.
> 
> > 
> > This is just a raw register values? 
> 
> Yes.
> 
> Why can't standard pinctrl properties be
> > used here. What's the point of using SCMI pinctrl if you are just shuttling
> > register values to SCMI?
> 
> Standard pinctrl properties will need more memory in our SCMI
> firmware design. We have limited on-chip memory for SCMI firmware usage.
> 

So you sort of strip-down the server due to hw constraints, but is it
the SCMI server still compliant with the spec ? (as in supporting all
the mandatory commands and features for the protocols you decided to
support)

Thanks,
Cristian

> Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> for i.MX95 Pinctrl OEM extensions
> 
> On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> > Hi Rob,
> >
> > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > properties for i.MX95 Pinctrl OEM extensions
> > >
> > > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions.
> > > > This patch is to add i.MX95 Pinctrl OEM extensions properties.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > > ++++++++++++++++++++++
> > > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> >
> > ....
> >
> > > > +
> > > > +title: i.MX System Control and Management Interface (SCMI)
> > > > +Pinctrl Protocol
> > > > +
> > > > +maintainers:
> > > > +  - Peng Fan <peng.fan@arm.com>
> > > > +
> > > > +patternProperties:
> > > > +  '[a-f0-9]+$':
> > > > +    type: object
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      pins:
> > > > +        $ref: /schemas/types.yaml#/definitions/string
> > > > +        description: name of the pin
> > > > +        items:
> > > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > > +                  gpioio00, gpioio01, gpioio02, gpioio03,
> > > > + gpioio04,
> > >
> > > ioio?
> >
> > Yes. The name is exported from SCMI firmware.
> >
> > >
> > > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > > +
> > > > +      nxp,func-id:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: Indicates the PAD connections which function
> > > > + module
> > >
> > > The description makes no sense...
> >
> > I will use below description in v3:
> > For each pin, there are up to 8 muxing options (called ALT modes). For
> example:
> > Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> > 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance:
> > jtag_mux 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance:
> > imx95_wakeupmix_top 100b - Select mux mode: ALT4 mux port:
> > FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top 101b - Select mux
> > mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> 110b
> > - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance:
> imx95_wakeupmix_top The nxp,func-id property is selecting one ALT mode.
> >
> > >
> > > > +
> > > > +      nxp,pin-conf:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        description: The PAD configuration
> > >
> > > PAD stands for? Or is it pin configuration as the property name says?
> >
> > It is pin configuration which includes pull up/down, drive strength and etc.
> >
> > >
> > > This is just a raw register values?
> >
> > Yes.
> >
> > Why can't standard pinctrl properties be
> > > used here. What's the point of using SCMI pinctrl if you are just
> > > shuttling register values to SCMI?
> >
> > Standard pinctrl properties will need more memory in our SCMI firmware
> > design. We have limited on-chip memory for SCMI firmware usage.
> >
> 
> So you sort of strip-down the server due to hw constraints, but is it the SCMI
> server still compliant with the spec ? (as in supporting all the mandatory
> commands and features for the protocols you decided to
> support)

Yes, the SCMI server is compliant with the spec. It is the SCMI firmware
returns num function with value zero, num group with value zero.

And only OEM pin configure type supported in our SCMI firmware.

I have posted out v3 following Rob's suggestion, using property
'pinmux', but now we could not reuse pinctrl-scmi.c

Thanks,
Peng.

> 
> Thanks,
> Cristian

On Wed, May 01, 2024 at 12:11:41PM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> > for i.MX95 Pinctrl OEM extensions
> > 
> > On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> > > Hi Rob,
> > >
> > > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > > properties for i.MX95 Pinctrl OEM extensions
> > > >
> > > > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions.
> > > > > This patch is to add i.MX95 Pinctrl OEM extensions properties.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > ---
> > > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > > > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > > > ++++++++++++++++++++++
> > > > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > >
> > > ....
> > >
> > > > > +
> > > > > +title: i.MX System Control and Management Interface (SCMI)
> > > > > +Pinctrl Protocol
> > > > > +
> > > > > +maintainers:
> > > > > +  - Peng Fan <peng.fan@arm.com>
> > > > > +
> > > > > +patternProperties:
> > > > > +  '[a-f0-9]+$':
> > > > > +    type: object
> > > > > +    unevaluatedProperties: false
> > > > > +
> > > > > +    properties:
> > > > > +      pins:
> > > > > +        $ref: /schemas/types.yaml#/definitions/string
> > > > > +        description: name of the pin
> > > > > +        items:
> > > > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > > > +                  gpioio00, gpioio01, gpioio02, gpioio03,
> > > > > + gpioio04,
> > > >
> > > > ioio?
> > >
> > > Yes. The name is exported from SCMI firmware.
> > >
> > > >
> > > > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > > > +
> > > > > +      nxp,func-id:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: Indicates the PAD connections which function
> > > > > + module
> > > >
> > > > The description makes no sense...
> > >
> > > I will use below description in v3:
> > > For each pin, there are up to 8 muxing options (called ALT modes). For
> > example:
> > > Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> > > 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance:
> > > jtag_mux 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance:
> > > imx95_wakeupmix_top 100b - Select mux mode: ALT4 mux port:
> > > FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top 101b - Select mux
> > > mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> > 110b
> > > - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance:
> > imx95_wakeupmix_top The nxp,func-id property is selecting one ALT mode.
> > >
> > > >
> > > > > +
> > > > > +      nxp,pin-conf:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        description: The PAD configuration
> > > >
> > > > PAD stands for? Or is it pin configuration as the property name says?
> > >
> > > It is pin configuration which includes pull up/down, drive strength and etc.
> > >
> > > >
> > > > This is just a raw register values?
> > >
> > > Yes.
> > >
> > > Why can't standard pinctrl properties be
> > > > used here. What's the point of using SCMI pinctrl if you are just
> > > > shuttling register values to SCMI?
> > >
> > > Standard pinctrl properties will need more memory in our SCMI firmware
> > > design. We have limited on-chip memory for SCMI firmware usage.
> > >
> > 
> > So you sort of strip-down the server due to hw constraints, but is it the SCMI
> > server still compliant with the spec ? (as in supporting all the mandatory
> > commands and features for the protocols you decided to
> > support)
> 
> Yes, the SCMI server is compliant with the spec. It is the SCMI firmware
> returns num function with value zero, num group with value zero.
> 
> And only OEM pin configure type supported in our SCMI firmware.
> 
> I have posted out v3 following Rob's suggestion, using property
> 'pinmux', but now we could not reuse pinctrl-scmi.c

As I wrote in a separated thread (only 5 mins ago), if you have written
a distinct SCMI driver to use protocol@19 ops to suit your needs after
having parsed your DT...why do you want to re-use pinctrl-scmi.c and so
create a dependency between the 2 ?

The only thing that they have in common is that they are SCMI drivers
using SCMI Pinctrl protocol@19 operations in their own different way.

Thanks,
Cristian

> Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
> for i.MX95 Pinctrl OEM extensions
> 
> On Wed, May 01, 2024 at 12:11:41PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > properties for i.MX95 Pinctrl OEM extensions
> > >
> > > On Thu, Apr 25, 2024 at 09:06:00AM +0000, Peng Fan wrote:
> > > > Hi Rob,
> > > >
> > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add
> > > > > properties for i.MX95 Pinctrl OEM extensions
> > > > >
> > > > > On Thu, Apr 18, 2024 at 10:53:17AM +0800, Peng Fan (OSS) wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > i.MX95 Pinctrl is managed by SCMI firmware using OEM extensions.
> > > > > > This patch is to add i.MX95 Pinctrl OEM extensions properties.
> > > > > >
> > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++-
> > > > > >  .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 66
> > > > > > ++++++++++++++++++++++
> > > > > >  2 files changed, 72 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > >
> > > > ....
> > > >
> > > > > > +
> > > > > > +title: i.MX System Control and Management Interface (SCMI)
> > > > > > +Pinctrl Protocol
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Peng Fan <peng.fan@arm.com>
> > > > > > +
> > > > > > +patternProperties:
> > > > > > +  '[a-f0-9]+$':
> > > > > > +    type: object
> > > > > > +    unevaluatedProperties: false
> > > > > > +
> > > > > > +    properties:
> > > > > > +      pins:
> > > > > > +        $ref: /schemas/types.yaml#/definitions/string
> > > > > > +        description: name of the pin
> > > > > > +        items:
> > > > > > +          enum: [ daptdi, daptmsswdio, daptclkswclk, daptdotraceswo,
> > > > > > +                  gpioio00, gpioio01, gpioio02, gpioio03,
> > > > > > + gpioio04,
> > > > >
> > > > > ioio?
> > > >
> > > > Yes. The name is exported from SCMI firmware.
> > > >
> > > > >
> > > > > > +                  gpioio05, gpioio06, gpioio07, gpioio08, gpioio09,
> > > > > > +                  gpioio10, gpioio11, gpioio12, gpioio13, gpioio14,
> > > > > > +                  gpioio15, gpioio16, gpioio17, gpioio18, gpioio19,
> > > > > > +                  gpioio20, gpioio21, gpioio22, gpioio23, gpioio24,
> > > > > > +                  gpioio25, gpioio26, gpioio27, gpioio28, gpioio29,
> > > > > > +                  gpioio30, gpioio31, gpioio32, gpioio33, gpioio34,
> > > > > > +                  gpioio35, gpioio36, gpioio37, ccmclko1, ccmclko2,
> > > > > > +                  ccmclko3, ccmclko4, enet1mdc, enet1mdio, enet1td3,
> > > > > > +                  enet1td2, enet1td1, enet1td0, enet1txctl, enet1txc,
> > > > > > +                  enet1rxctl, enet1rxc, enet1rd0, enet1rd1, enet1rd2,
> > > > > > +                  enet1rd3, enet2mdc, enet2mdio, enet2td3, enet2td2,
> > > > > > +                  enet2td1, enet2td0, enet2txctl, enet2txc, enet2rxctl,
> > > > > > +                  enet2rxc, enet2rd0, enet2rd1, enet2rd2, enet2rd3,
> > > > > > +                  sd1clk, sd1cmd, sd1data0, sd1data1, sd1data2, sd1data3,
> > > > > > +                  sd1data4, sd1data5, sd1data6, sd1data7, sd1strobe,
> > > > > > +                  sd2vselect, sd3clk, sd3cmd, sd3data0, sd3data1,
> > > > > > +                  sd3data2, sd3data3, xspi1data0, xspi1data1, xspi1data2,
> > > > > > +                  xspi1data3, xspi1data4, xspi1data5, xspi1data6,
> > > > > > +                  xspi1data7, xspi1dqs, xspi1sclk, xspi1ss0b, xspi1ss1b,
> > > > > > +                  sd2cdb, sd2clk, sd2cmd, sd2data0, sd2data1, sd2data2,
> > > > > > +                  sd2data3, sd2resetb, i2c1scl, i2c1sda, i2c2scl, i2c2sda,
> > > > > > +                  uart1rxd, uart1txd, uart2rxd, uart2txd, pdmclk,
> > > > > > +                  pdmbitstream0, pdmbitstream1, sai1txfs, sai1txc,
> > > > > > +                  sai1txd0, sai1rxd0, wdogany, fccuerr0 ]
> > > > > > +
> > > > > > +      nxp,func-id:
> > > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +        description: Indicates the PAD connections which
> > > > > > + function module
> > > > >
> > > > > The description makes no sense...
> > > >
> > > > I will use below description in v3:
> > > > For each pin, there are up to 8 muxing options (called ALT modes).
> > > > For
> > > example:
> > > > Select 1 of 5 iomux modes to be used for pad: DAP_TCLK_SWCLK.
> > > > 000b - Select mux mode: ALT0 mux port: JTAG_MUX_TCK of instance:
> > > > jtag_mux 010b - Select mux mode: ALT2 mux port: CAN4_RX of instance:
> > > > imx95_wakeupmix_top 100b - Select mux mode: ALT4 mux port:
> > > > FLEXIO1_FLEXIO30 of instance: imx95_wakeupmix_top 101b - Select
> > > > mux
> > > > mode: ALT5 mux port: GPIO3_IO30 of instance: imx95_wakeupmix_top
> > > 110b
> > > > - Select mux mode: ALT6 mux port: LPUART5_CTS_B of instance:
> > > imx95_wakeupmix_top The nxp,func-id property is selecting one ALT
> mode.
> > > >
> > > > >
> > > > > > +
> > > > > > +      nxp,pin-conf:
> > > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +        description: The PAD configuration
> > > > >
> > > > > PAD stands for? Or is it pin configuration as the property name says?
> > > >
> > > > It is pin configuration which includes pull up/down, drive strength and
> etc.
> > > >
> > > > >
> > > > > This is just a raw register values?
> > > >
> > > > Yes.
> > > >
> > > > Why can't standard pinctrl properties be
> > > > > used here. What's the point of using SCMI pinctrl if you are
> > > > > just shuttling register values to SCMI?
> > > >
> > > > Standard pinctrl properties will need more memory in our SCMI
> > > > firmware design. We have limited on-chip memory for SCMI firmware
> usage.
> > > >
> > >
> > > So you sort of strip-down the server due to hw constraints, but is
> > > it the SCMI server still compliant with the spec ? (as in supporting
> > > all the mandatory commands and features for the protocols you
> > > decided to
> > > support)
> >
> > Yes, the SCMI server is compliant with the spec. It is the SCMI
> > firmware returns num function with value zero, num group with value zero.
> >
> > And only OEM pin configure type supported in our SCMI firmware.
> >
> > I have posted out v3 following Rob's suggestion, using property
> > 'pinmux', but now we could not reuse pinctrl-scmi.c
> 
> As I wrote in a separated thread (only 5 mins ago), if you have written a
> distinct SCMI driver to use protocol@19 ops to suit your needs after having
> parsed your DT...why do you want to re-use pinctrl-scmi.c and so create a
> dependency between the 2 ?
> 
> The only thing that they have in common is that they are SCMI drivers using
> SCMI Pinctrl protocol@19 operations in their own different way.

Yes. I have replied there. I thought to reuse pinctrl-scmi.c with minimal
changes, but since the binding changes in v3, I need to made big changes.

Anyway I agree with your suggestions to decouple the two drivers.
I will do that in v4. Before that, I need wait for dt-maintainers to review
the binding changes in v3.

Thanks,
Peng.

> 
> Thanks,
> Cristian