* [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header
@ 2025-08-15 10:27 Cristian Marussi
2025-08-15 10:27 ` [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus() Cristian Marussi
2025-08-18 5:35 ` [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Dhruva Gole
0 siblings, 2 replies; 6+ messages in thread
From: Cristian Marussi @ 2025-08-15 10:27 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-pm
Cc: sudeep.holla, james.quinlan, f.fainelli, vincent.guittot,
quic_sibis, dan.carpenter, d-gole, johan+linaro, rafael,
viresh.kumar, quic_mdtipton, Cristian Marussi
Split and relocate the quirks framework header so as to be usable also by
SCMI Drivers and not only by the core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/clock.c | 2 +-
drivers/firmware/arm_scmi/driver.c | 1 +
drivers/firmware/arm_scmi/quirks.h | 33 +++-------------------
include/linux/scmi_quirks.h | 44 ++++++++++++++++++++++++++++++
4 files changed, 50 insertions(+), 30 deletions(-)
create mode 100644 include/linux/scmi_quirks.h
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index afa7981efe82..5599697de37a 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -7,11 +7,11 @@
#include <linux/module.h>
#include <linux/limits.h>
+#include <linux/scmi_quirks.h>
#include <linux/sort.h>
#include "protocols.h"
#include "notify.h"
-#include "quirks.h"
/* Updated only after ALL the mandatory features for that version are merged */
#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index bd56a877fdfc..6f5934cd3a65 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -34,6 +34,7 @@
#include <linux/processor.h>
#include <linux/refcount.h>
#include <linux/slab.h>
+#include <linux/scmi_quirks.h>
#include <linux/xarray.h>
#include "common.h"
diff --git a/drivers/firmware/arm_scmi/quirks.h b/drivers/firmware/arm_scmi/quirks.h
index a71fde85a527..260ae38d617b 100644
--- a/drivers/firmware/arm_scmi/quirks.h
+++ b/drivers/firmware/arm_scmi/quirks.h
@@ -4,49 +4,24 @@
*
* Copyright (C) 2025 ARM Ltd.
*/
-#ifndef _SCMI_QUIRKS_H
-#define _SCMI_QUIRKS_H
+#ifndef _SCMI_QUIRKS_INTERNAL_H
+#define _SCMI_QUIRKS_INTERNAL_H
-#include <linux/static_key.h>
+#include <linux/device.h>
#include <linux/types.h>
#ifdef CONFIG_ARM_SCMI_QUIRKS
-#define DECLARE_SCMI_QUIRK(_qn) \
- DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
-
-/*
- * A helper to associate the actual code snippet to use as a quirk
- * named as _qn.
- */
-#define SCMI_QUIRK(_qn, _blk) \
- do { \
- if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
- (_blk); \
- } while (0)
-
void scmi_quirks_initialize(void);
void scmi_quirks_enable(struct device *dev, const char *vend,
const char *subv, const u32 impl);
#else
-#define DECLARE_SCMI_QUIRK(_qn)
-/* Force quirks compilation even when SCMI Quirks are disabled */
-#define SCMI_QUIRK(_qn, _blk) \
- do { \
- if (0) \
- (_blk); \
- } while (0)
-
static inline void scmi_quirks_initialize(void) { }
static inline void scmi_quirks_enable(struct device *dev, const char *vend,
const char *sub_vend, const u32 impl) { }
#endif /* CONFIG_ARM_SCMI_QUIRKS */
-/* Quirk delarations */
-DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
-DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
-
-#endif /* _SCMI_QUIRKS_H */
+#endif /* _SCMI_QUIRKS_INTERNAL_H */
diff --git a/include/linux/scmi_quirks.h b/include/linux/scmi_quirks.h
new file mode 100644
index 000000000000..11657bd91ffc
--- /dev/null
+++ b/include/linux/scmi_quirks.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * System Control and Management Interface (SCMI) Message Protocol Quirks
+ *
+ * Copyright (C) 2025 ARM Ltd.
+ */
+#ifndef _SCMI_QUIRKS_H
+#define _SCMI_QUIRKS_H
+
+#include <linux/static_key.h>
+#include <linux/types.h>
+
+#ifdef CONFIG_ARM_SCMI_QUIRKS
+
+#define DECLARE_SCMI_QUIRK(_qn) \
+ DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
+
+/*
+ * A helper to associate the actual code snippet to use as a quirk
+ * named as _qn.
+ */
+#define SCMI_QUIRK(_qn, _blk) \
+ do { \
+ if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
+ (_blk); \
+ } while (0)
+
+#else
+
+#define DECLARE_SCMI_QUIRK(_qn)
+/* Force quirks compilation even when SCMI Quirks are disabled */
+#define SCMI_QUIRK(_qn, _blk) \
+ do { \
+ if (0) \
+ (_blk); \
+ } while (0)
+
+#endif /* CONFIG_ARM_SCMI_QUIRKS */
+
+/* Quirk delarations */
+DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
+DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
+
+#endif /* _SCMI_QUIRKS_H */
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus()
2025-08-15 10:27 [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Cristian Marussi
@ 2025-08-15 10:27 ` Cristian Marussi
2025-08-18 6:20 ` Dhruva Gole
2025-08-18 5:35 ` [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Dhruva Gole
1 sibling, 1 reply; 6+ messages in thread
From: Cristian Marussi @ 2025-08-15 10:27 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-pm
Cc: sudeep.holla, james.quinlan, f.fainelli, vincent.guittot,
quic_sibis, dan.carpenter, d-gole, johan+linaro, rafael,
viresh.kumar, quic_mdtipton, Florian Fainelli, Cristian Marussi
From: Florian Fainelli <florian.fainelli@broadcom.com>
Broadcom STB platforms were early adopters of the SCMI framework and as
a result, not all deployed systems have a Device Tree entry where SCMI
protocol 0x13 (PERFORMANCE) is declared as a clock provider, nor are the
CPU Device Tree node(s) referencing protocol 0x13 as their clock
provider.
Leverage the quirks framework recently introduce to match on the
Broadcom SCMI vendor and in that case, disable the Device Tree
properties checks being done by scmi_dev_used_by_cpus().
Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
Fixes: 6c9bb8692272 ("cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs")
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
[Cristian: Moved quirk directly into scmi_dev_used_by_cpus]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
----
@Florian: I reworked this minimally to avoid the global as I was mentioning.
No change around the version match either...so the NOT_FOR_UPSTREAM tag.
(also the if (true) i smaybe a bit idiotic...)
Please check if it is fine and modify as you see fit.
---
drivers/cpufreq/scmi-cpufreq.c | 9 +++++++++
drivers/firmware/arm_scmi/quirks.c | 2 ++
include/linux/scmi_quirks.h | 1 +
3 files changed, 12 insertions(+)
diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index ef078426bfd5..9b7cbc4e87d9 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -19,6 +19,7 @@
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/scmi_protocol.h>
+#include <linux/scmi_quirks.h>
#include <linux/types.h>
#include <linux/units.h>
@@ -393,6 +394,12 @@ static struct cpufreq_driver scmi_cpufreq_driver = {
.set_boost = cpufreq_boost_set_sw,
};
+#define QUIRK_SCMI_CPUFREQ_CHECK_DT_PROPS \
+ ({ \
+ if (true) \
+ return true; \
+ })
+
static bool scmi_dev_used_by_cpus(struct device *scmi_dev)
{
struct device_node *scmi_np = dev_of_node(scmi_dev);
@@ -400,6 +407,8 @@ static bool scmi_dev_used_by_cpus(struct device *scmi_dev)
struct device *cpu_dev;
int cpu, idx;
+ SCMI_QUIRK(scmi_cpufreq_no_check_dt_props, QUIRK_SCMI_CPUFREQ_CHECK_DT_PROPS);
+
if (!scmi_np)
return false;
diff --git a/drivers/firmware/arm_scmi/quirks.c b/drivers/firmware/arm_scmi/quirks.c
index 03960aca3610..aafc7b4b3294 100644
--- a/drivers/firmware/arm_scmi/quirks.c
+++ b/drivers/firmware/arm_scmi/quirks.c
@@ -171,6 +171,7 @@ struct scmi_quirk {
/* Global Quirks Definitions */
DEFINE_SCMI_QUIRK(clock_rates_triplet_out_of_spec, NULL, NULL, NULL);
DEFINE_SCMI_QUIRK(perf_level_get_fc_force, "Qualcomm", NULL, "0x20000-");
+DEFINE_SCMI_QUIRK_EXPORTED(scmi_cpufreq_no_check_dt_props, "brcm-scmi", NULL, "0x2");
/*
* Quirks Pointers Array
@@ -181,6 +182,7 @@ DEFINE_SCMI_QUIRK(perf_level_get_fc_force, "Qualcomm", NULL, "0x20000-");
static struct scmi_quirk *scmi_quirks_table[] = {
__DECLARE_SCMI_QUIRK_ENTRY(clock_rates_triplet_out_of_spec),
__DECLARE_SCMI_QUIRK_ENTRY(perf_level_get_fc_force),
+ __DECLARE_SCMI_QUIRK_ENTRY(scmi_cpufreq_no_check_dt_props),
NULL
};
diff --git a/include/linux/scmi_quirks.h b/include/linux/scmi_quirks.h
index 11657bd91ffc..ee72a5f6e885 100644
--- a/include/linux/scmi_quirks.h
+++ b/include/linux/scmi_quirks.h
@@ -40,5 +40,6 @@
/* Quirk delarations */
DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
+DECLARE_SCMI_QUIRK(scmi_cpufreq_no_check_dt_props);
#endif /* _SCMI_QUIRKS_H */
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header
2025-08-15 10:27 [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Cristian Marussi
2025-08-15 10:27 ` [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus() Cristian Marussi
@ 2025-08-18 5:35 ` Dhruva Gole
2025-08-18 13:19 ` Cristian Marussi
1 sibling, 1 reply; 6+ messages in thread
From: Dhruva Gole @ 2025-08-18 5:35 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, arm-scmi, linux-pm, sudeep.holla,
james.quinlan, f.fainelli, vincent.guittot, quic_sibis,
dan.carpenter, johan+linaro, rafael, viresh.kumar, quic_mdtipton
Hi,
On Aug 15, 2025 at 11:27:35 +0100, Cristian Marussi wrote:
> Split and relocate the quirks framework header so as to be usable also by
> SCMI Drivers and not only by the core.
Could you elaborate a bit more on this reasoning? I am not fully
convinced as to why I shouldn't just include quirks.h in the other scmi
drivers as well?
Oh or perhaps you mean to say scmi driver like scmi cpufreq / clk-scmi
etc.. which lie outside the firmware/arm_scmi folder? If so then that's
not coming out clearly in this patch commit message.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> drivers/firmware/arm_scmi/clock.c | 2 +-
> drivers/firmware/arm_scmi/driver.c | 1 +
> drivers/firmware/arm_scmi/quirks.h | 33 +++-------------------
> include/linux/scmi_quirks.h | 44 ++++++++++++++++++++++++++++++
> 4 files changed, 50 insertions(+), 30 deletions(-)
> create mode 100644 include/linux/scmi_quirks.h
>
> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index afa7981efe82..5599697de37a 100644
> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -7,11 +7,11 @@
>
> #include <linux/module.h>
> #include <linux/limits.h>
> +#include <linux/scmi_quirks.h>
> #include <linux/sort.h>
>
> #include "protocols.h"
> #include "notify.h"
> -#include "quirks.h"
>
> /* Updated only after ALL the mandatory features for that version are merged */
> #define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index bd56a877fdfc..6f5934cd3a65 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -34,6 +34,7 @@
> #include <linux/processor.h>
> #include <linux/refcount.h>
> #include <linux/slab.h>
> +#include <linux/scmi_quirks.h>
> #include <linux/xarray.h>
>
> #include "common.h"
> diff --git a/drivers/firmware/arm_scmi/quirks.h b/drivers/firmware/arm_scmi/quirks.h
> index a71fde85a527..260ae38d617b 100644
> --- a/drivers/firmware/arm_scmi/quirks.h
> +++ b/drivers/firmware/arm_scmi/quirks.h
> @@ -4,49 +4,24 @@
> *
> * Copyright (C) 2025 ARM Ltd.
> */
> -#ifndef _SCMI_QUIRKS_H
> -#define _SCMI_QUIRKS_H
> +#ifndef _SCMI_QUIRKS_INTERNAL_H
> +#define _SCMI_QUIRKS_INTERNAL_H
Or as per your commit message wording, better to call it
_SCMI_QUIRKS_CORE_H ?
>
> -#include <linux/static_key.h>
> +#include <linux/device.h>
> #include <linux/types.h>
>
> #ifdef CONFIG_ARM_SCMI_QUIRKS
>
> -#define DECLARE_SCMI_QUIRK(_qn) \
> - DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
> -
> -/*
> - * A helper to associate the actual code snippet to use as a quirk
> - * named as _qn.
> - */
> -#define SCMI_QUIRK(_qn, _blk) \
> - do { \
> - if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
> - (_blk); \
> - } while (0)
> -
> void scmi_quirks_initialize(void);
> void scmi_quirks_enable(struct device *dev, const char *vend,
> const char *subv, const u32 impl);
>
> #else
>
> -#define DECLARE_SCMI_QUIRK(_qn)
> -/* Force quirks compilation even when SCMI Quirks are disabled */
> -#define SCMI_QUIRK(_qn, _blk) \
> - do { \
> - if (0) \
> - (_blk); \
> - } while (0)
> -
> static inline void scmi_quirks_initialize(void) { }
> static inline void scmi_quirks_enable(struct device *dev, const char *vend,
> const char *sub_vend, const u32 impl) { }
>
> #endif /* CONFIG_ARM_SCMI_QUIRKS */
>
> -/* Quirk delarations */
> -DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
> -DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
> -
> -#endif /* _SCMI_QUIRKS_H */
> +#endif /* _SCMI_QUIRKS_INTERNAL_H */
> diff --git a/include/linux/scmi_quirks.h b/include/linux/scmi_quirks.h
> new file mode 100644
> index 000000000000..11657bd91ffc
> --- /dev/null
> +++ b/include/linux/scmi_quirks.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * System Control and Management Interface (SCMI) Message Protocol Quirks
> + *
> + * Copyright (C) 2025 ARM Ltd.
> + */
> +#ifndef _SCMI_QUIRKS_H
> +#define _SCMI_QUIRKS_H
> +
> +#include <linux/static_key.h>
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_ARM_SCMI_QUIRKS
> +
> +#define DECLARE_SCMI_QUIRK(_qn) \
> + DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
> +
> +/*
> + * A helper to associate the actual code snippet to use as a quirk
> + * named as _qn.
> + */
> +#define SCMI_QUIRK(_qn, _blk) \
> + do { \
> + if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
> + (_blk); \
> + } while (0)
> +
> +#else
> +
> +#define DECLARE_SCMI_QUIRK(_qn)
> +/* Force quirks compilation even when SCMI Quirks are disabled */
> +#define SCMI_QUIRK(_qn, _blk) \
> + do { \
> + if (0) \
> + (_blk); \
> + } while (0)
Did you happen to run checkpatch on this?
8<---------------------------------------------------------------------------
WARNING: Argument '_qn' is not used in function-like macro
#142: FILE: include/linux/scmi_quirks.h:32:
+#define SCMI_QUIRK(_qn, _blk) \
+ do { \
+ if (0) \
+ (_blk); \
+ } while (0)
total: 0 errors, 2 warnings, 0 checks, 116 lines checked
--------------------------------------------------------------------------->8
> +
> +#endif /* CONFIG_ARM_SCMI_QUIRKS */
> +
> +/* Quirk delarations */
> +DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
> +DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
> +
> +#endif /* _SCMI_QUIRKS_H */
> --
> 2.50.1
>
--
Best regards,
Dhruva Gole
Texas Instruments Incorporated
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus()
2025-08-15 10:27 ` [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus() Cristian Marussi
@ 2025-08-18 6:20 ` Dhruva Gole
2025-08-18 13:21 ` Cristian Marussi
0 siblings, 1 reply; 6+ messages in thread
From: Dhruva Gole @ 2025-08-18 6:20 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, arm-scmi, linux-pm, sudeep.holla,
james.quinlan, f.fainelli, vincent.guittot, quic_sibis,
dan.carpenter, johan+linaro, rafael, viresh.kumar, quic_mdtipton,
Florian Fainelli
On Aug 15, 2025 at 11:27:36 +0100, Cristian Marussi wrote:
> From: Florian Fainelli <florian.fainelli@broadcom.com>
>
> Broadcom STB platforms were early adopters of the SCMI framework and as
> a result, not all deployed systems have a Device Tree entry where SCMI
> protocol 0x13 (PERFORMANCE) is declared as a clock provider, nor are the
> CPU Device Tree node(s) referencing protocol 0x13 as their clock
> provider.
>
> Leverage the quirks framework recently introduce to match on the
> Broadcom SCMI vendor and in that case, disable the Device Tree
> properties checks being done by scmi_dev_used_by_cpus().
>
> Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
> Fixes: 6c9bb8692272 ("cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs")
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> [Cristian: Moved quirk directly into scmi_dev_used_by_cpus]
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>
> ----
> @Florian: I reworked this minimally to avoid the global as I was mentioning.
> No change around the version match either...so the NOT_FOR_UPSTREAM tag.
> (also the if (true) i smaybe a bit idiotic...)
> Please check if it is fine and modify as you see fit.
> ---
> drivers/cpufreq/scmi-cpufreq.c | 9 +++++++++
> drivers/firmware/arm_scmi/quirks.c | 2 ++
> include/linux/scmi_quirks.h | 1 +
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index ef078426bfd5..9b7cbc4e87d9 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -19,6 +19,7 @@
> #include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/scmi_protocol.h>
> +#include <linux/scmi_quirks.h>
> #include <linux/types.h>
> #include <linux/units.h>
>
> @@ -393,6 +394,12 @@ static struct cpufreq_driver scmi_cpufreq_driver = {
> .set_boost = cpufreq_boost_set_sw,
> };
>
> +#define QUIRK_SCMI_CPUFREQ_CHECK_DT_PROPS \
> + ({ \
> + if (true) \
> + return true; \
> + })
> +
Probably another checkpatch warning to fix:
8<--------------------------------------------------------------------
WARNING: Macros with flow control statements should be avoided
#50: FILE: drivers/cpufreq/scmi-cpufreq.c:397:
+#define QUIRK_SCMI_CPUFREQ_CHECK_DT_PROPS \
+ ({ \
+ if (true) \
+ return true; \
+ })
total: 0 errors, 1 warnings, 0 checks, 47 lines checked
-------------------------------------------------------------------->8
--
Best regards,
Dhruva Gole
Texas Instruments Incorporated
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header
2025-08-18 5:35 ` [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Dhruva Gole
@ 2025-08-18 13:19 ` Cristian Marussi
0 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2025-08-18 13:19 UTC (permalink / raw)
To: Dhruva Gole
Cc: Cristian Marussi, linux-kernel, linux-arm-kernel, arm-scmi,
linux-pm, sudeep.holla, james.quinlan, f.fainelli,
vincent.guittot, quic_sibis, dan.carpenter, johan+linaro, rafael,
viresh.kumar, quic_mdtipton
On Mon, Aug 18, 2025 at 11:05:35AM +0530, Dhruva Gole wrote:
> Hi,
>
> On Aug 15, 2025 at 11:27:35 +0100, Cristian Marussi wrote:
Ho Dhruva,
> > Split and relocate the quirks framework header so as to be usable also by
> > SCMI Drivers and not only by the core.
>
> Could you elaborate a bit more on this reasoning? I am not fully
> convinced as to why I shouldn't just include quirks.h in the other scmi
> drivers as well?
>
You can include quirks.h directly BUT you will have to use an ugly
#include ../drivers/firmware/arm_scmi/quirks.h
...AND also you will endup exposing a couple of internal functions used
by the quirk framework like:
void scmi_quirks_initialize(void);
void scmi_quirks_enable(struct device *dev, const char *vend,
const char *subv, const u32 impl);
..so I split out the interface needed for quirking and relocated the
file to include/quirks
> Oh or perhaps you mean to say scmi driver like scmi cpufreq / clk-scmi
> etc.. which lie outside the firmware/arm_scmi folder? If so then that's
> not coming out clearly in this patch commit message.
Yes when I say SCMI drivers I mean general drivers that use the SCMI
stack BUT that are not part of the SCMI core....basically those drivers
that use the API in include/linux/scmi_protocol.h
Anyway...it seems like the only user of quirks in the SCMI drivers has
just disappeared...so maybe this small patch can just wait...
>
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > drivers/firmware/arm_scmi/clock.c | 2 +-
> > drivers/firmware/arm_scmi/driver.c | 1 +
> > drivers/firmware/arm_scmi/quirks.h | 33 +++-------------------
> > include/linux/scmi_quirks.h | 44 ++++++++++++++++++++++++++++++
> > 4 files changed, 50 insertions(+), 30 deletions(-)
> > create mode 100644 include/linux/scmi_quirks.h
> >
> > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> > index afa7981efe82..5599697de37a 100644
> > --- a/drivers/firmware/arm_scmi/clock.c
> > +++ b/drivers/firmware/arm_scmi/clock.c
> > @@ -7,11 +7,11 @@
> >
> > #include <linux/module.h>
> > #include <linux/limits.h>
> > +#include <linux/scmi_quirks.h>
> > #include <linux/sort.h>
> >
> > #include "protocols.h"
> > #include "notify.h"
> > -#include "quirks.h"
> >
> > /* Updated only after ALL the mandatory features for that version are merged */
> > #define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000
> > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > index bd56a877fdfc..6f5934cd3a65 100644
> > --- a/drivers/firmware/arm_scmi/driver.c
> > +++ b/drivers/firmware/arm_scmi/driver.c
> > @@ -34,6 +34,7 @@
> > #include <linux/processor.h>
> > #include <linux/refcount.h>
> > #include <linux/slab.h>
> > +#include <linux/scmi_quirks.h>
> > #include <linux/xarray.h>
> >
> > #include "common.h"
> > diff --git a/drivers/firmware/arm_scmi/quirks.h b/drivers/firmware/arm_scmi/quirks.h
> > index a71fde85a527..260ae38d617b 100644
> > --- a/drivers/firmware/arm_scmi/quirks.h
> > +++ b/drivers/firmware/arm_scmi/quirks.h
> > @@ -4,49 +4,24 @@
> > *
> > * Copyright (C) 2025 ARM Ltd.
> > */
> > -#ifndef _SCMI_QUIRKS_H
> > -#define _SCMI_QUIRKS_H
> > +#ifndef _SCMI_QUIRKS_INTERNAL_H
> > +#define _SCMI_QUIRKS_INTERNAL_H
>
> Or as per your commit message wording, better to call it
> _SCMI_QUIRKS_CORE_H ?
>
...well mayeb yes....I have not bother to change the filename
anyway ....
> >
> > -#include <linux/static_key.h>
> > +#include <linux/device.h>
> > #include <linux/types.h>
> >
> > #ifdef CONFIG_ARM_SCMI_QUIRKS
> >
> > -#define DECLARE_SCMI_QUIRK(_qn) \
> > - DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
> > -
> > -/*
> > - * A helper to associate the actual code snippet to use as a quirk
> > - * named as _qn.
> > - */
> > -#define SCMI_QUIRK(_qn, _blk) \
> > - do { \
> > - if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
> > - (_blk); \
> > - } while (0)
> > -
> > void scmi_quirks_initialize(void);
> > void scmi_quirks_enable(struct device *dev, const char *vend,
> > const char *subv, const u32 impl);
> >
> > #else
> >
> > -#define DECLARE_SCMI_QUIRK(_qn)
> > -/* Force quirks compilation even when SCMI Quirks are disabled */
> > -#define SCMI_QUIRK(_qn, _blk) \
> > - do { \
> > - if (0) \
> > - (_blk); \
> > - } while (0)
> > -
> > static inline void scmi_quirks_initialize(void) { }
> > static inline void scmi_quirks_enable(struct device *dev, const char *vend,
> > const char *sub_vend, const u32 impl) { }
> >
> > #endif /* CONFIG_ARM_SCMI_QUIRKS */
> >
> > -/* Quirk delarations */
> > -DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
> > -DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
> > -
> > -#endif /* _SCMI_QUIRKS_H */
> > +#endif /* _SCMI_QUIRKS_INTERNAL_H */
> > diff --git a/include/linux/scmi_quirks.h b/include/linux/scmi_quirks.h
> > new file mode 100644
> > index 000000000000..11657bd91ffc
> > --- /dev/null
> > +++ b/include/linux/scmi_quirks.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * System Control and Management Interface (SCMI) Message Protocol Quirks
> > + *
> > + * Copyright (C) 2025 ARM Ltd.
> > + */
> > +#ifndef _SCMI_QUIRKS_H
> > +#define _SCMI_QUIRKS_H
> > +
> > +#include <linux/static_key.h>
> > +#include <linux/types.h>
> > +
> > +#ifdef CONFIG_ARM_SCMI_QUIRKS
> > +
> > +#define DECLARE_SCMI_QUIRK(_qn) \
> > + DECLARE_STATIC_KEY_FALSE(scmi_quirk_ ## _qn)
> > +
> > +/*
> > + * A helper to associate the actual code snippet to use as a quirk
> > + * named as _qn.
> > + */
> > +#define SCMI_QUIRK(_qn, _blk) \
> > + do { \
> > + if (static_branch_unlikely(&(scmi_quirk_ ## _qn))) \
> > + (_blk); \
> > + } while (0)
> > +
> > +#else
> > +
> > +#define DECLARE_SCMI_QUIRK(_qn)
> > +/* Force quirks compilation even when SCMI Quirks are disabled */
> > +#define SCMI_QUIRK(_qn, _blk) \
> > + do { \
> > + if (0) \
> > + (_blk); \
> > + } while (0)
>
> Did you happen to run checkpatch on this?
> 8<---------------------------------------------------------------------------
> WARNING: Argument '_qn' is not used in function-like macro
> #142: FILE: include/linux/scmi_quirks.h:32:
> +#define SCMI_QUIRK(_qn, _blk) \
> + do { \
> + if (0) \
> + (_blk); \
> + } while (0)
>
> total: 0 errors, 2 warnings, 0 checks, 116 lines checked
> --------------------------------------------------------------------------->8
>
Oh yes I did on all the series...BUT this specific error on this branch
of the #if was already present in the original patch that added the
Quirk framework...I did not know how to avoid this warning and given it
is pretty much harmless I just ignored it...
>
> > +
> > +#endif /* CONFIG_ARM_SCMI_QUIRKS */
> > +
> > +/* Quirk delarations */
> > +DECLARE_SCMI_QUIRK(clock_rates_triplet_out_of_spec);
> > +DECLARE_SCMI_QUIRK(perf_level_get_fc_force);
> > +
> > +#endif /* _SCMI_QUIRKS_H */
> > --
> > 2.50.1
> >
>
Thanks for the review.
Cristian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus()
2025-08-18 6:20 ` Dhruva Gole
@ 2025-08-18 13:21 ` Cristian Marussi
0 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2025-08-18 13:21 UTC (permalink / raw)
To: Dhruva Gole
Cc: Cristian Marussi, linux-kernel, linux-arm-kernel, arm-scmi,
linux-pm, sudeep.holla, james.quinlan, f.fainelli,
vincent.guittot, quic_sibis, dan.carpenter, johan+linaro, rafael,
viresh.kumar, quic_mdtipton, Florian Fainelli
On Mon, Aug 18, 2025 at 11:50:44AM +0530, Dhruva Gole wrote:
> On Aug 15, 2025 at 11:27:36 +0100, Cristian Marussi wrote:
> > From: Florian Fainelli <florian.fainelli@broadcom.com>
> >
> > Broadcom STB platforms were early adopters of the SCMI framework and as
> > a result, not all deployed systems have a Device Tree entry where SCMI
> > protocol 0x13 (PERFORMANCE) is declared as a clock provider, nor are the
> > CPU Device Tree node(s) referencing protocol 0x13 as their clock
> > provider.
> >
> > Leverage the quirks framework recently introduce to match on the
> > Broadcom SCMI vendor and in that case, disable the Device Tree
> > properties checks being done by scmi_dev_used_by_cpus().
> >
> > Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
> > Fixes: 6c9bb8692272 ("cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs")
> > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > [Cristian: Moved quirk directly into scmi_dev_used_by_cpus]
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >
> > ----
> > @Florian: I reworked this minimally to avoid the global as I was mentioning.
> > No change around the version match either...so the NOT_FOR_UPSTREAM tag.
> > (also the if (true) i smaybe a bit idiotic...)
> > Please check if it is fine and modify as you see fit.
> > ---
> > drivers/cpufreq/scmi-cpufreq.c | 9 +++++++++
> > drivers/firmware/arm_scmi/quirks.c | 2 ++
> > include/linux/scmi_quirks.h | 1 +
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> > index ef078426bfd5..9b7cbc4e87d9 100644
> > --- a/drivers/cpufreq/scmi-cpufreq.c
> > +++ b/drivers/cpufreq/scmi-cpufreq.c
> > @@ -19,6 +19,7 @@
> > #include <linux/pm_qos.h>
> > #include <linux/slab.h>
> > #include <linux/scmi_protocol.h>
> > +#include <linux/scmi_quirks.h>
> > #include <linux/types.h>
> > #include <linux/units.h>
> >
> > @@ -393,6 +394,12 @@ static struct cpufreq_driver scmi_cpufreq_driver = {
> > .set_boost = cpufreq_boost_set_sw,
> > };
> >
> > +#define QUIRK_SCMI_CPUFREQ_CHECK_DT_PROPS \
> > + ({ \
> > + if (true) \
> > + return true; \
> > + })
> > +
>
> Probably another checkpatch warning to fix:
> 8<--------------------------------------------------------------------
> WARNING: Macros with flow control statements should be avoided
> #50: FILE: drivers/cpufreq/scmi-cpufreq.c:397:
Yes I saw this too....but seemed harmless and anyway Quirks are really
very peculiar piece of code...
Thanks
Cristian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-18 14:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 10:27 [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Cristian Marussi
2025-08-15 10:27 ` [PATCH 2/2] [NOT_FOR_UPSTREAM] cpufreq: scmi: Add quirk to disable checks in scmi_dev_used_by_cpus() Cristian Marussi
2025-08-18 6:20 ` Dhruva Gole
2025-08-18 13:21 ` Cristian Marussi
2025-08-18 5:35 ` [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header Dhruva Gole
2025-08-18 13:19 ` 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).