> 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@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. > > > > 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