1. Patchew
  2. linux
  3. [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs
  4. View patch

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

Peng Fan posted 3 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
Posted by Peng Fan 1 month, 4 weeks ago 
To ensure successful builds when CONFIG_IMX_SCMI_MISC_DRV is not enabled,
this patch adds static inline stub implementations for the following
functions:

  - scmi_imx_misc_ctrl_get()
  - scmi_imx_misc_ctrl_set()

These stubs return -EOPNOTSUPP to indicate that the functionality is not
supported in the current configuration. This avoids potential build or
link errors in code that conditionally calls these functions based on
feature availability.

Fixes: 540c830212ed ("firmware: imx: remove duplicate scmi_imx_misc_ctrl_get()")
Fixes: 0b4f8a68b292 ("firmware: imx: Add i.MX95 MISC driver")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 include/linux/firmware/imx/sm.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
index d4212bc42b2c17fb8f94d58856a75beb5611ce4b..99c15bbb46aa8329b5aa8e03017e152074cdf492 100644
--- a/include/linux/firmware/imx/sm.h
+++ b/include/linux/firmware/imx/sm.h
@@ -26,8 +26,20 @@
 #define SCMI_IMX94_CTRL_SAI3_MCLK	5U	/*!< WAKE SAI3 MCLK */
 #define SCMI_IMX94_CTRL_SAI4_MCLK	6U	/*!< WAKE SAI4 MCLK */
 
+#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_DRV)
 int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
 int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+#else
+static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val)
+{
+	return -EOPNOTSUPP;
+}
+#endif
 
 int scmi_imx_cpu_start(u32 cpuid, bool start);
 int scmi_imx_cpu_started(u32 cpuid, bool *started);

-- 
2.37.1
Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
Posted by Arnd Bergmann 1 month, 2 weeks ago 
On Thu, Aug 7, 2025, at 03:47, Peng Fan wrote:
> To ensure successful builds when CONFIG_IMX_SCMI_MISC_DRV is not enabled,
> this patch adds static inline stub implementations for the following
> functions:
>
>   - scmi_imx_misc_ctrl_get()
>   - scmi_imx_misc_ctrl_set()
>
> These stubs return -EOPNOTSUPP to indicate that the functionality is not
> supported in the current configuration. This avoids potential build or
> link errors in code that conditionally calls these functions based on
> feature availability.
>
> Fixes: 540c830212ed ("firmware: imx: remove duplicate scmi_imx_misc_ctrl_get()")
> Fixes: 0b4f8a68b292 ("firmware: imx: Add i.MX95 MISC driver")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

I don't think this does what you describe, at least not reliably:
 
> +#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_DRV)
>  int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
>  int scmi_imx_misc_ctrl_set(u32 id, u32 val);
> +#else
> +static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
> +{
> +	return -EOPNOTSUPP;
> +}

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

     Arnd
Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
Posted by Peng Fan 1 month, 2 weeks ago 
Hi Arnd,

On Wed, Aug 20, 2025 at 03:55:20PM +0200, Arnd Bergmann wrote:
>On Thu, Aug 7, 2025, at 03:47, Peng Fan wrote:
>> To ensure successful builds when CONFIG_IMX_SCMI_MISC_DRV is not enabled,
>> this patch adds static inline stub implementations for the following
>> functions:
>>
>>   - scmi_imx_misc_ctrl_get()
>>   - scmi_imx_misc_ctrl_set()
>>
>> These stubs return -EOPNOTSUPP to indicate that the functionality is not
>> supported in the current configuration. This avoids potential build or
>> link errors in code that conditionally calls these functions based on
>> feature availability.
>>
>> Fixes: 540c830212ed ("firmware: imx: remove duplicate scmi_imx_misc_ctrl_get()")
>> Fixes: 0b4f8a68b292 ("firmware: imx: Add i.MX95 MISC driver")
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>
>I don't think this does what you describe, at least not reliably:
> 
>> +#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_DRV)
>>  int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
>>  int scmi_imx_misc_ctrl_set(u32 id, u32 val);
>> +#else
>> +static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>
>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 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
also be module built.

But if IMX_SCMI_MISC_DRV is n, the consumer driver is y, there will be
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
to n to make Image smaller. Introducing the stub API is mainly for this
case.

Hope this is clear

Thanks,
Peng

>
>     Arnd
>
Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
Posted by Arnd Bergmann 1 month, 2 weeks ago 
On Thu, Aug 21, 2025, at 11:56, Peng Fan wrote:
> On Wed, Aug 20, 2025 at 03:55:20PM +0200, Arnd Bergmann wrote:
>>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 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
> also be module built.
>
> But if IMX_SCMI_MISC_DRV is n, the consumer driver is y, there will be
> 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
> 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'.

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.

      Arnd
Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
Posted by Cristian Marussi 1 month, 2 weeks ago 
On Thu, Aug 07, 2025 at 09:47:42AM +0800, Peng Fan wrote:
> To ensure successful builds when CONFIG_IMX_SCMI_MISC_DRV is not enabled,
> this patch adds static inline stub implementations for the following
> functions:
> 
>   - scmi_imx_misc_ctrl_get()
>   - scmi_imx_misc_ctrl_set()
> 
> These stubs return -EOPNOTSUPP to indicate that the functionality is not
> supported in the current configuration. This avoids potential build or
> link errors in code that conditionally calls these functions based on
> feature availability.
> 
> Fixes: 540c830212ed ("firmware: imx: remove duplicate scmi_imx_misc_ctrl_get()")
> Fixes: 0b4f8a68b292 ("firmware: imx: Add i.MX95 MISC driver")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  include/linux/firmware/imx/sm.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
> index d4212bc42b2c17fb8f94d58856a75beb5611ce4b..99c15bbb46aa8329b5aa8e03017e152074cdf492 100644
> --- a/include/linux/firmware/imx/sm.h
> +++ b/include/linux/firmware/imx/sm.h
> @@ -26,8 +26,20 @@
>  #define SCMI_IMX94_CTRL_SAI3_MCLK	5U	/*!< WAKE SAI3 MCLK */
>  #define SCMI_IMX94_CTRL_SAI4_MCLK	6U	/*!< WAKE SAI4 MCLK */
>  
> +#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_DRV)
>  int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
>  int scmi_imx_misc_ctrl_set(u32 id, u32 val);
> +#else
> +static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif

LGTM.
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.