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

The IOMUX Controller in i.MX9 family has Daisy chain that multi pads drive
same module input pin. Each SoC has its own register offset, so
introduce "nxp,iomuxc-daisy-off" property to specify the daisy register
offset. With this property being parsed by driver, there is no need
to hardcode the offset in pinctrl driver for each new SoC.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml      | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
index a96fc6cce502c10ab415e0b26bff1be8c3bc82f5..b5b2a9c8688a7f6525cdb6a32db22681f4f1a0b9 100644
--- a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
@@ -13,6 +13,11 @@ maintainers:
 allOf:
   - $ref: /schemas/pinctrl/pinctrl.yaml
 
+properties:
+  nxp,iomuxc-daisy-off:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Specify the IOMUX Controller first Daisy register's offset
+
 patternProperties:
   'grp$':
     type: object
@@ -51,3 +56,6 @@ patternProperties:
       - fsl,pins
 
 additionalProperties: true
+
+required:
+  - nxp,iomuxc-daisy-off

-- 
2.37.1

On Mon, May 12, 2025 at 10:14:14AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The IOMUX Controller in i.MX9 family has Daisy chain that multi pads drive
> same module input pin. Each SoC has its own register offset, so
> introduce "nxp,iomuxc-daisy-off" property to specify the daisy register
> offset. With this property being parsed by driver, there is no need
> to hardcode the offset in pinctrl driver for each new SoC.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml      | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
> index a96fc6cce502c10ab415e0b26bff1be8c3bc82f5..b5b2a9c8688a7f6525cdb6a32db22681f4f1a0b9 100644
> --- a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
> @@ -13,6 +13,11 @@ maintainers:
>  allOf:
>    - $ref: /schemas/pinctrl/pinctrl.yaml
>  
> +properties:
> +  nxp,iomuxc-daisy-off:

Same comment here as was left on the driver.
I also don't get why there's a property being introduced from something
you can determine based on the soc.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Specify the IOMUX Controller first Daisy register's offset
> +
>  patternProperties:
>    'grp$':
>      type: object
> @@ -51,3 +56,6 @@ patternProperties:
>        - fsl,pins
>  
>  additionalProperties: true
> +
> +required:
> +  - nxp,iomuxc-daisy-off
> 
> -- 
> 2.37.1
> 

Hi Conor,

On Mon, May 12, 2025 at 05:20:17PM +0100, Conor Dooley wrote:
>On Mon, May 12, 2025 at 10:14:14AM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> The IOMUX Controller in i.MX9 family has Daisy chain that multi pads drive
>> same module input pin. Each SoC has its own register offset, so
>> introduce "nxp,iomuxc-daisy-off" property to specify the daisy register
>> offset. With this property being parsed by driver, there is no need
>> to hardcode the offset in pinctrl driver for each new SoC.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>  .../devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml      | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
>> index a96fc6cce502c10ab415e0b26bff1be8c3bc82f5..b5b2a9c8688a7f6525cdb6a32db22681f4f1a0b9 100644
>> --- a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
>> @@ -13,6 +13,11 @@ maintainers:
>>  allOf:
>>    - $ref: /schemas/pinctrl/pinctrl.yaml
>>  
>> +properties:
>> +  nxp,iomuxc-daisy-off:
>
>Same comment here as was left on the driver.
>I also don't get why there's a property being introduced from something
>you can determine based on the soc.

we are targeting a common pinctrl driver for i.MX SCMI based SoC.
So that means pinctrl-imx-scmi.c needs support i.MX95, i.MX94 and i.MX9[X].

Each time we support a new SoC, we need to hardcode the register offset in
the driver. But if using DT here, no need to update the pinctrl driver anymore
when supporting a new i.MX SoC.

Thanks,
Peng

>
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Specify the IOMUX Controller first Daisy register's offset
>> +
>>  patternProperties:
>>    'grp$':
>>      type: object
>> @@ -51,3 +56,6 @@ patternProperties:
>>        - fsl,pins
>>  
>>  additionalProperties: true
>> +
>> +required:
>> +  - nxp,iomuxc-daisy-off
>> 
>> -- 
>> 2.37.1
>>

On Tue, May 13, 2025 at 8:46 AM Peng Fan <peng.fan@oss.nxp.com> wrote:

> >Same comment here as was left on the driver.
> >I also don't get why there's a property being introduced from something
> >you can determine based on the soc.

I agree with Conor's observation.

> we are targeting a common pinctrl driver for i.MX SCMI based SoC.
> So that means pinctrl-imx-scmi.c needs support i.MX95, i.MX94 and i.MX9[X].
>
> Each time we support a new SoC, we need to hardcode the register offset in
> the driver. But if using DT here, no need to update the pinctrl driver anymore
> when supporting a new i.MX SoC.

I understand that it is convenient, but that doesn't mean it is the right
thing to do.

I would advice you to keep this in the driver and use the SoC compatible
to determine the offset, just as is done today.

If information can be deduced from what is already present in the
device tree it is redundant to add stuff like this, and it inevitably
will create copy-paste errors where the wrong offset is used
with the wrong SoC.

Yours,
Linus Walleij

On Tue, May 13, 2025 at 03:20:44PM +0200, Linus Walleij wrote:
>On Tue, May 13, 2025 at 8:46 AM Peng Fan <peng.fan@oss.nxp.com> wrote:
>
>> >Same comment here as was left on the driver.
>> >I also don't get why there's a property being introduced from something
>> >you can determine based on the soc.
>
>I agree with Conor's observation.
>
>> we are targeting a common pinctrl driver for i.MX SCMI based SoC.
>> So that means pinctrl-imx-scmi.c needs support i.MX95, i.MX94 and i.MX9[X].
>>
>> Each time we support a new SoC, we need to hardcode the register offset in
>> the driver. But if using DT here, no need to update the pinctrl driver anymore
>> when supporting a new i.MX SoC.
>
>I understand that it is convenient, but that doesn't mean it is the right
>thing to do.
>
>I would advice you to keep this in the driver and use the SoC compatible
>to determine the offset, just as is done today.
>
>If information can be deduced from what is already present in the
>device tree it is redundant to add stuff like this, and it inevitably
>will create copy-paste errors where the wrong offset is used
>with the wrong SoC.

Got it. Drop this patchset.

Thanks,
Peng

>
>Yours,
>Linus Walleij