Thread (12 messages) 12 messages, 4 authors, 2025-08-22

RE: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API

From: Peng Fan <peng.fan@nxp.com>
Date: 2025-08-22 02:35:29
Also in: imx, lkml

Hi Arnd,
Subject: Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI
MISC API

On Thu, Aug 21, 2025, at 11:56, Peng Fan wrote:
quoted
On Wed, Aug 20, 2025 at 03:55:20PM +0200, Arnd Bergmann wrote:
quoted
On Thu, Aug 7, 2025, at 03:47, Peng Fan wrote:


When a caller of this function is in a built-in driver but the
IMX_SCMI_MISC_DRV code is in a loadable module, you still get a
link
quoted
quoted
failure, see 514b2262ade4 ("firmware: arm_scmi:
Fix i.MX build dependency") for an example.

As you still need the correct Kconfig dependencies, I think your patch
here is not helpful.
The consumer driver still needs Kconfig dependcies, such as
  depends on IMX_SCMI_MISC_DRV || !IMX_SCMI_MISC_DRV

So when IMX_SCMI_MISC_DRV is module built, the consumer driver
will
quoted
also be module built.

But if IMX_SCMI_MISC_DRV is n, the consumer driver is y, there will
be
quoted
link error.

The consumer driver is to support platform A and platform B.

Platform A does not require the real API in IMX_SCMI_MISC_DRV.
Platform B requires the real API in IMX_SCMI_MISC_DRV.

So when producing an image for platform A, IMX_SCMI_MISC_DRV
could set
quoted
to n to make Image smaller. Introducing the stub API is mainly for
this case.

Hope this is clear
I see. In this case the stub helpers are not wrong, but I still find them
more error-prone than not having them and using IS_ENABLED() checks
as in commit 101c9023594a
("ASoC: fsl_mqs: Support accessing registers by scmi interface"):

+static int fsl_mqs_sm_read(void *context, unsigned int reg, unsigned
+int *val) {
+       struct fsl_mqs *mqs_priv = context;
+       int num = 1;
+
+       if (IS_ENABLED(CONFIG_IMX_SCMI_MISC_DRV) &&
+           mqs_priv->soc->ctrl_off == reg)
+               return
+ scmi_imx_misc_ctrl_get(SCMI_IMX_CTRL_MQS1_SETTINGS, &num,
val);
+
+       return -EINVAL;
+};

The logic is the same here in the end, but the link failure is easier to
trigger and repair if someone gets it wrong.

Also, for drivers that actually need the exported interface, the
dependency becomes the simpler 'depends on IMX_SCMI_MISC_DRV'.
Yeah, but since consumer drivers supports multiple platforms,
if platform A not requires the real API in IMX_SCMI_MISC_DRV,
no need to link the real API.
Which driver using this symbol are you actually looking at? I see you
have three similar patches for a couple of interfaces, and want to make
sure the added complexity is really needed here. I do a lot of
randconfig build tests, so quite often I end up being the one that runs
into the subtle link failures from these.
The patch is here
https://lore.kernel.org/linux-remoteproc/20250821-imx95-rproc-1-v5-0-e93191dfac51@nxp.com/T/#macc660f61873742de447ac8c20d34f2d494ff712 (local)

Please help give a look.

Thanks,
Peng.
      Arnd
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help