| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250818053535.owst4ilq2oxfgqnq@lcpd911>
Date: Mon, 18 Aug 2025 11:05:35 +0530
From: Dhruva Gole <d-gole@...com>
To: Cristian Marussi <cristian.marussi@....com>
CC: <linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<arm-scmi@...r.kernel.org>, <linux-pm@...r.kernel.org>,
<sudeep.holla@....com>, <james.quinlan@...adcom.com>,
<f.fainelli@...il.com>, <vincent.guittot@...aro.org>,
<quic_sibis@...cinc.com>, <dan.carpenter@...aro.org>,
<johan+linaro@...nel.org>, <rafael@...nel.org>,
<viresh.kumar@...aro.org>, <quic_mdtipton@...cinc.com>
Subject: Re: [PATCH 1/2] firmware: arm_scmi: Rework quirks framework header
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@....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
Powered by blists - more mailing lists