Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins

From: Cristian Marussi <cristian.marussi@arm.com>
To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	Rob Herring <robh@kernel.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>,
	Linus Walleij <linus.walleij@linaro.org>,
	Dong Aisheng <aisheng.dong@nxp.com>, Jacky Bai <ping.bai@nxp.com>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, imx@lists.linux.dev,
	linux-gpio@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
Date: Wed, 1 May 2024 13:36:57 +0100	[thread overview]
Message-ID: <ZjI3abJUIgo4xgRu@pluto> (raw)
In-Reply-To: <20240428-pinctrl-scmi-oem-v3-v3-4-eda341eb47ed@nxp.com>

On Sun, Apr 28, 2024 at 01:07:50PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add pinctrl-scmi.h to include the function prototype and 'struct
> scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers
> could use it.
> 

Hi Peng,

so you wrote a new alternative SCMI driver using Pinctrl protocol@0x19
so that you can just parse you custom DT bindings and then use the SCMI
pinctrl_ops to set the OEM extensions to configure your platform...
...since your firmware cannot cope with the all SCMI stack footprint....

... you seemed to have solved the issue of having 2 Pinctrl drivers
coexisting under the Linux Pinctrl subsystem while attached to the same
protocol@19 node with patch 5/6 blocklist (if I get that right..)

I think this approach of a standalone SCMI alternative Pinctrl driver
that handles distinctly NXP OEM extensions and DT-parsing is certainly
more preferable than the original series you posted months ago where
custom NXP stuff were simply stuck on top of the Generic SCMI Pinctrl driver...

...what I still dont understand is why you exported data and structure
from pincttl-scmi.c to use it here; when NXP pinctrl is active the
standard Linux generic Pinctrl driver wont be alive, so not probed, so
no data can be shared, the only thing I can imagine is that you are
just trying to avoid duplicating a dozen lines from the logic of
scmi_pinctrl_get_pins() into your new NXP driver.

In this way, though, you are creating a dependency between 2 drivers,
that are not even allowed to cohexist at runtime really (due to the
blocklist trick).

Am I missing something ?

If not, I think it will be much better to just rewrite that few lines of
scmi_pincrtrl_pins_get trivial logic into your NXP driver and keep the 2
drivers fully distinct at all times.

Thanks,
Cristian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel