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

[Peng Fan <peng.fan@xxxxxxx>]

> 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@xxxxxxx>
> > > >
> > > > 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@xxxxxxx>
> > > > ---
> > > >  .../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@xxxxxxx>
> > > > +
> > > > +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