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

[Cristian Marussi <cristian.marussi@xxxxxxx>]

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)

Thanks,
Cristian