Received-SPF: pass (google.com: domain of linux-kernel+bounces-157611-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 24 Apr 2024 14:30:07 -0500
From: Rob Herring <robh@kernel.org>
To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, Peng Fan <peng.fan@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Cristian Marussi <cristian.marussi@arm.com>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: firmware: arm,scmi: Add properties
 for i.MX95 Pinctrl OEM extensions
Message-ID: <20240424193007.GA329896-robh@kernel.org>
References: <20240418-pinctrl-scmi-oem-v1-v2-0-3a555a3c58c3@nxp.com>
 <20240418-pinctrl-scmi-oem-v1-v2-1-3a555a3c58c3@nxp.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240418-pinctrl-scmi-oem-v1-v2-1-3a555a3c58c3@nxp.com>

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