* [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 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 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
* 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 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
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).