* [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs
@ 2025-08-07 1:47 Peng Fan
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Peng Fan @ 2025-08-07 1:47 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi
Cc: imx, linux-arm-kernel, linux-kernel, Peng Fan
To ensure successful builds when CONFIG_IMX_SCMI_[MISC,CPU,LMM]_DRV are not
enabled, this patchset adds static inline stub implementations:
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.
The initial support for SCMI MISC API was to use CONFIG_IMX_SCMI_MISC_EXT
to guard the API. But this is wrong. There was an commit [1] that tried to
address build issue for MISC API, but the better fix should use
CONFIG_IMX_SCMI_MISC_DRV to guard the APIs. Because when user driver
reference the APIs, but CONFIG_IMX_SCMI_MISC_DRV is not defined, there
will be link error.
This patchset is to fix the issues for all the three drivers.
I add Fixes tag to the patchset, so I not delay the sending until RC1.
[1] 540c830212ed ("firmware: imx: remove duplicate scmi_imx_misc_ctrl_get()")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (3):
firmware: imx: Add stub functions for SCMI MISC API
firmware: imx: Add stub functions for SCMI LMM API
firmware: imx: Add stub functions for SCMI CPU API
include/linux/firmware/imx/sm.h | 47 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
---
base-commit: b7d4e259682caccb51a25283655f2c8f02e32d23
change-id: 20250807-imx9-sm-bea018f06042
Best regards,
--
Peng Fan <peng.fan@nxp.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-07 1:47 [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs Peng Fan
@ 2025-08-07 1:47 ` Peng Fan
2025-08-20 13:50 ` Cristian Marussi
2025-08-20 13:55 ` Arnd Bergmann
2025-08-07 1:47 ` [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API Peng Fan
2025-08-07 1:47 ` [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API Peng Fan
2 siblings, 2 replies; 12+ messages in thread
From: Peng Fan @ 2025-08-07 1:47 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi
Cc: imx, linux-arm-kernel, linux-kernel, Peng Fan
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API
2025-08-07 1:47 [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs Peng Fan
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
@ 2025-08-07 1:47 ` Peng Fan
2025-08-20 13:52 ` Cristian Marussi
2025-08-07 1:47 ` [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API Peng Fan
2 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2025-08-07 1:47 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi
Cc: imx, linux-arm-kernel, linux-kernel, Peng Fan
To ensure successful builds when CONFIG_IMX_SCMI_LMM_DRV is not enabled,
this patch adds static inline stub implementations for the following
functions:
- scmi_imx_lmm_operation()
- scmi_imx_lmm_info()
- scmi_imx_lmm_reset_vector_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: 7242bbf418f0 ("firmware: imx: Add i.MX95 SCMI LMM driver")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
include/linux/firmware/imx/sm.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
index 99c15bbb46aa8329b5aa8e03017e152074cdf492..f2a72177bb37c1d46145a60710e3809641e0f5a2 100644
--- a/include/linux/firmware/imx/sm.h
+++ b/include/linux/firmware/imx/sm.h
@@ -56,7 +56,24 @@ enum scmi_imx_lmm_op {
#define SCMI_IMX_LMM_OP_FORCEFUL 0
#define SCMI_IMX_LMM_OP_GRACEFUL BIT(0)
+#if IS_ENABLED(CONFIG_IMX_SCMI_LMM_DRV)
int scmi_imx_lmm_operation(u32 lmid, enum scmi_imx_lmm_op op, u32 flags);
int scmi_imx_lmm_info(u32 lmid, struct scmi_imx_lmm_info *info);
int scmi_imx_lmm_reset_vector_set(u32 lmid, u32 cpuid, u32 flags, u64 vector);
+#else
+static inline int scmi_imx_lmm_operation(u32 lmid, enum scmi_imx_lmm_op op, u32 flags)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_lmm_info(u32 lmid, struct scmi_imx_lmm_info *info)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_lmm_reset_vector_set(u32 lmid, u32 cpuid, u32 flags, u64 vector)
+{
+ return -EOPNOTSUPP;
+}
+#endif
#endif
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API
2025-08-07 1:47 [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs Peng Fan
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
2025-08-07 1:47 ` [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API Peng Fan
@ 2025-08-07 1:47 ` Peng Fan
2025-08-20 13:53 ` Cristian Marussi
2 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2025-08-07 1:47 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi
Cc: imx, linux-arm-kernel, linux-kernel, Peng Fan
To ensure successful builds when CONFIG_IMX_SCMI_CPU_DRV is not enabled,
this patch adds static inline stub implementations for the following
functions:
- scmi_imx_cpu_start()
- scmi_imx_cpu_started()
- scmi_imx_cpu_reset_vector_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: 1055faa5d660 ("firmware: imx: Add i.MX95 SCMI CPU driver")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
include/linux/firmware/imx/sm.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
index f2a72177bb37c1d46145a60710e3809641e0f5a2..a33b45027356751f4b8ad9b9136f0dd302a82520 100644
--- a/include/linux/firmware/imx/sm.h
+++ b/include/linux/firmware/imx/sm.h
@@ -41,10 +41,28 @@ static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val)
}
#endif
+#if IS_ENABLED(CONFIG_IMX_SCMI_CPU_DRV)
int scmi_imx_cpu_start(u32 cpuid, bool start);
int scmi_imx_cpu_started(u32 cpuid, bool *started);
int scmi_imx_cpu_reset_vector_set(u32 cpuid, u64 vector, bool start, bool boot,
bool resume);
+#else
+static inline int scmi_imx_cpu_start(u32 cpuid, bool start)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_cpu_started(u32 cpuid, bool *started)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_cpu_reset_vector_set(u32 cpuid, u64 vector, bool start,
+ bool boot, bool resume)
+{
+ return -EOPNOTSUPP;
+}
+#endif
enum scmi_imx_lmm_op {
SCMI_IMX_LMM_BOOT,
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
@ 2025-08-20 13:50 ` Cristian Marussi
2025-08-20 13:55 ` Arnd Bergmann
1 sibling, 0 replies; 12+ messages in thread
From: Cristian Marussi @ 2025-08-20 13:50 UTC (permalink / raw)
To: Peng Fan
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi, imx,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API
2025-08-07 1:47 ` [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API Peng Fan
@ 2025-08-20 13:52 ` Cristian Marussi
0 siblings, 0 replies; 12+ messages in thread
From: Cristian Marussi @ 2025-08-20 13:52 UTC (permalink / raw)
To: Peng Fan
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi, imx,
linux-arm-kernel, linux-kernel
On Thu, Aug 07, 2025 at 09:47:43AM +0800, Peng Fan wrote:
> To ensure successful builds when CONFIG_IMX_SCMI_LMM_DRV is not enabled,
> this patch adds static inline stub implementations for the following
> functions:
>
> - scmi_imx_lmm_operation()
> - scmi_imx_lmm_info()
> - scmi_imx_lmm_reset_vector_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: 7242bbf418f0 ("firmware: imx: Add i.MX95 SCMI LMM driver")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> include/linux/firmware/imx/sm.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
> index 99c15bbb46aa8329b5aa8e03017e152074cdf492..f2a72177bb37c1d46145a60710e3809641e0f5a2 100644
> --- a/include/linux/firmware/imx/sm.h
> +++ b/include/linux/firmware/imx/sm.h
> @@ -56,7 +56,24 @@ enum scmi_imx_lmm_op {
> #define SCMI_IMX_LMM_OP_FORCEFUL 0
> #define SCMI_IMX_LMM_OP_GRACEFUL BIT(0)
>
> +#if IS_ENABLED(CONFIG_IMX_SCMI_LMM_DRV)
> int scmi_imx_lmm_operation(u32 lmid, enum scmi_imx_lmm_op op, u32 flags);
> int scmi_imx_lmm_info(u32 lmid, struct scmi_imx_lmm_info *info);
> int scmi_imx_lmm_reset_vector_set(u32 lmid, u32 cpuid, u32 flags, u64 vector);
> +#else
> +static inline int scmi_imx_lmm_operation(u32 lmid, enum scmi_imx_lmm_op op, u32 flags)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int scmi_imx_lmm_info(u32 lmid, struct scmi_imx_lmm_info *info)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int scmi_imx_lmm_reset_vector_set(u32 lmid, u32 cpuid, u32 flags, u64 vector)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
> #endif
LGTM.
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Thanks,
Cristian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API
2025-08-07 1:47 ` [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API Peng Fan
@ 2025-08-20 13:53 ` Cristian Marussi
0 siblings, 0 replies; 12+ messages in thread
From: Cristian Marussi @ 2025-08-20 13:53 UTC (permalink / raw)
To: Peng Fan
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Arnd Bergmann, Sudeep Holla, Cristian Marussi, imx,
linux-arm-kernel, linux-kernel
On Thu, Aug 07, 2025 at 09:47:44AM +0800, Peng Fan wrote:
> To ensure successful builds when CONFIG_IMX_SCMI_CPU_DRV is not enabled,
> this patch adds static inline stub implementations for the following
> functions:
>
> - scmi_imx_cpu_start()
> - scmi_imx_cpu_started()
> - scmi_imx_cpu_reset_vector_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: 1055faa5d660 ("firmware: imx: Add i.MX95 SCMI CPU driver")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> include/linux/firmware/imx/sm.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
> index f2a72177bb37c1d46145a60710e3809641e0f5a2..a33b45027356751f4b8ad9b9136f0dd302a82520 100644
> --- a/include/linux/firmware/imx/sm.h
> +++ b/include/linux/firmware/imx/sm.h
> @@ -41,10 +41,28 @@ static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val)
> }
> #endif
>
> +#if IS_ENABLED(CONFIG_IMX_SCMI_CPU_DRV)
> int scmi_imx_cpu_start(u32 cpuid, bool start);
> int scmi_imx_cpu_started(u32 cpuid, bool *started);
> int scmi_imx_cpu_reset_vector_set(u32 cpuid, u64 vector, bool start, bool boot,
> bool resume);
> +#else
> +static inline int scmi_imx_cpu_start(u32 cpuid, bool start)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int scmi_imx_cpu_started(u32 cpuid, bool *started)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int scmi_imx_cpu_reset_vector_set(u32 cpuid, u64 vector, bool start,
> + bool boot, bool resume)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
LGTM.
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Thanks,
Cristian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
2025-08-20 13:50 ` Cristian Marussi
@ 2025-08-20 13:55 ` Arnd Bergmann
2025-08-21 9:56 ` Peng Fan
1 sibling, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2025-08-20 13:55 UTC (permalink / raw)
To: Peng Fan, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi
Cc: imx, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-21 9:56 ` Peng Fan
@ 2025-08-21 8:56 ` Arnd Bergmann
2025-08-22 2:35 ` Peng Fan
0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2025-08-21 8:56 UTC (permalink / raw)
To: Peng Fan
Cc: Peng Fan, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, imx,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-20 13:55 ` Arnd Bergmann
@ 2025-08-21 9:56 ` Peng Fan
2025-08-21 8:56 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2025-08-21 9:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Peng Fan, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, Cristian Marussi, imx,
linux-arm-kernel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-21 8:56 ` Arnd Bergmann
@ 2025-08-22 2:35 ` Peng Fan
2025-08-22 20:07 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2025-08-22 2:35 UTC (permalink / raw)
To: Arnd Bergmann, Peng Fan (OSS)
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
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:
> > 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'.
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
Please help give a look.
Thanks,
Peng.
>
> Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API
2025-08-22 2:35 ` Peng Fan
@ 2025-08-22 20:07 ` Arnd Bergmann
0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2025-08-22 20:07 UTC (permalink / raw)
To: Peng Fan, Peng Fan
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On Fri, Aug 22, 2025, at 04:35, Peng Fan wrote:
>> Subject: Re: [PATCH 1/3] firmware: imx: Add stub functions for SCMI
>> 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
>
> Please help give a look.
Thanks for the pointer. Yes, I agree this makes sense then,
and seems harmless since there are only a few drivers that
use the interfaces. I still think that the use conditional
stubs for optional features is problematic when the definition
is in a loadable module, and want to make sure this
is not just done out of habit, but because of a specific
documented requirement.
Please clarify the changelog then, changing the 'Fixes' tags
to a description that explains why you choose to revert my
earlier patches.
Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-08-22 20:08 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-07 1:47 [PATCH 0/3] firmware: imx: Add stub functions for MISC/CPU/LMM APIs Peng Fan
2025-08-07 1:47 ` [PATCH 1/3] firmware: imx: Add stub functions for SCMI MISC API Peng Fan
2025-08-20 13:50 ` Cristian Marussi
2025-08-20 13:55 ` Arnd Bergmann
2025-08-21 9:56 ` Peng Fan
2025-08-21 8:56 ` Arnd Bergmann
2025-08-22 2:35 ` Peng Fan
2025-08-22 20:07 ` Arnd Bergmann
2025-08-07 1:47 ` [PATCH 2/3] firmware: imx: Add stub functions for SCMI LMM API Peng Fan
2025-08-20 13:52 ` Cristian Marussi
2025-08-07 1:47 ` [PATCH 3/3] firmware: imx: Add stub functions for SCMI CPU API Peng Fan
2025-08-20 13:53 ` Cristian Marussi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).