* Re: [PATCH RFC v7 7/9] PM / devfreq: Introduce the QCOM SCMI Memlat devfreq driver
From: Bjorn Andersson @ 2026-07-02 17:21 UTC (permalink / raw)
To: Pragnesh Papaniya
Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sibi Sankar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Konrad Dybcio, Rajendra Nayak, Pankaj Patil, linux-arm-msm,
linux-kernel, arm-scmi, linux-arm-kernel, devicetree, linux-pm,
linux-tegra, Amir Vajid, Ramakrishna Gottimukkula
In-Reply-To: <20260610-rfc_v7_scmi_memlat-v7-7-f3f68c608f25@oss.qualcomm.com>
On Wed, Jun 10, 2026 at 02:21:34PM +0530, Pragnesh Papaniya wrote:
> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>
> On Qualcomm Glymur, Mahua and X1E/X1P (Hamoa) SoCs, the memlat governor and
> the mechanism to control the various caches and RAM is hosted on the CPU
> Control Processor (CPUCP), and configuration and control of this governor
> is exposed through the QCOM SCMI Generic Extension Protocol, addressed via
> the "MEMLAT" algorithm string.
>
This explains that there's a bunch of functionality running on CPUCP and
there's a "MEMLAT" string.
> Introduce a devfreq SCMI client driver that uses the MEMLAT algorithm
> string to detect memory-latency-bound workloads and control the
> frequency/level of the memory buses (DDR, LLCC and DDR_QOS).
You established that there's stuff running in the firmware, now we're
introducing a client driver to control memory buses.
But where did you explain how these two "facts" are related? Why is
there a client driver, what is the actual distribution of roles in this
dance?
> Model each bus
> as a devfreq device using the remote devfreq governor. This provides basic
> insight into device operation via trans_stat and allows further tuning of
> the remote governor's parameters from userspace.
>
Does this mean that the driver is "optional", and only exists to give
insight and a way to tune the firmware operation?
If that's the case, why is it a devfreq driver?
Please write your commit messages (and patches) such that people outside
your immediate team can understand why the patches are needed and what
the code is supposed to do.
> Co-developed-by: Amir Vajid <amir.vajid@oss.qualcomm.com>
> Signed-off-by: Amir Vajid <amir.vajid@oss.qualcomm.com>
> Co-developed-by: Ramakrishna Gottimukkula <ramakrishna.gottimukkula@oss.qualcomm.com>
> Signed-off-by: Ramakrishna Gottimukkula <ramakrishna.gottimukkula@oss.qualcomm.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Co-developed-by: Pragnesh Papaniya <pragnesh.papaniya@oss.qualcomm.com>
> Signed-off-by: Pragnesh Papaniya <pragnesh.papaniya@oss.qualcomm.com>
> ---
> drivers/devfreq/Kconfig | 13 +
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/scmi-qcom-memlat-cfg.h | 573 +++++++++++++++++++++++++++
> drivers/devfreq/scmi-qcom-memlat-devfreq.c | 616 +++++++++++++++++++++++++++++
> 4 files changed, 1203 insertions(+)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 2caa87554914..98b5a50d3189 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -169,6 +169,19 @@ config ARM_SUN8I_A33_MBUS_DEVFREQ
> This adds the DEVFREQ driver for the MBUS controller in some
> Allwinner sun8i (A33 through H3) and sun50i (A64 and H5) SoCs.
>
> +config SCMI_QCOM_MEMLAT_DEVFREQ
> + tristate "Qualcomm Technologies Inc. SCMI client driver"
> + depends on QCOM_SCMI_GENERIC_EXT || COMPILE_TEST
> + select DEVFREQ_GOV_REMOTE
> + help
> + This driver uses the MEMLAT (memory latency) algorithm string
Is "driver uses X algorithm string" idiomatic SCMI terms?
> + hosted on QCOM SCMI Vendor Protocol to detect memory latency
> + workloads and control frequency/level of the various memory
> + buses (DDR/LLCC/DDR_QOS).
> +
> + This driver defines/documents the parameter IDs used while configuring
> + the memory buses.
Imagine an person outside your team, sitting there in menuconfig
wondering if they should enable this driver or not.
There's a sentence in the middle ("control frequency/level of various
memory buses" - that sounds like something I want. But "detect memory
latency", is it just monitoring or does that part relate to the
controlling part? "This driver defines" so what are those parameters
used for, do I need some other driver for the control part? Is this last
paragraph adding value to my understanding for that
CONFIG_SCMI_QCOM_MEMLAT_DEVFREQ does?
> +
> source "drivers/devfreq/event/Kconfig"
>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index cde57c8cda76..b11f94e2f485 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o
> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
> +obj-$(CONFIG_SCMI_QCOM_MEMLAT_DEVFREQ) += scmi-qcom-memlat-devfreq.o
>
> # DEVFREQ Event Drivers
> obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
> diff --git a/drivers/devfreq/scmi-qcom-memlat-cfg.h b/drivers/devfreq/scmi-qcom-memlat-cfg.h
> new file mode 100644
> index 000000000000..1ab8b61ea271
> --- /dev/null
> +++ b/drivers/devfreq/scmi-qcom-memlat-cfg.h
Are the entities declared in this header file used by anything other
than scmi-qcom-memlat-devfreq.c? If not why is it a separate header file?
> @@ -0,0 +1,573 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __DRIVERS_DEVFREQ_SCMI_QCOM_MEMLAT_CONFIG_H__
> +#define __DRIVERS_DEVFREQ_SCMI_QCOM_MEMLAT_CONFIG_H__
> +
> +/*
> + * Memlat Effective Frequency Calculation Method
> + * CPUCP_EFFECTIVE_FREQ_METHOD_0 - Uses CPU Cycles and CONST Cycles to calculate
> + * CPUCP_EFFECTIVE_FREQ_METHOD_1 - Uses CPU Cycles and time period
> + */
> +#define CPUCP_EFFECTIVE_FREQ_CALC_METHOD_0 0
> +#define CPUCP_EFFECTIVE_FREQ_CALC_METHOD_1 1
#define ZERO 0
#define ONE 1
Can these be given real names, or could the struct member even be a
boolean?
That said, "ZERO" isn't used in your patch...
> +
> +#define EV_CPU_CYCLES 0
> +#define EV_CNT_CYCLES 1
> +#define EV_INST_RETIRED 2
> +#define EV_STALL_BACKEND_MEM 3
> +#define EV_L2_D_RFILL 5
> +#define INVALID_IDX 0xff
The names of these defines aren't awesome names to put in include
files...
> +
> +#define MEMLAT_ALGO_STR 0x4D454D4C4154ULL /* MEMLAT */
As you've seen a thousand times on LKML already, we want lower-case hex
digits...
> +
> +struct scmi_qcom_map_table {
> + unsigned int cpu_freq;
> + unsigned int mem_freq;
Unit soup
> +};
> +
> +struct scmi_qcom_opp_data {
> + unsigned long freq;
> + unsigned int level;
The arrays of opp_data is all static const, and you seem to only define
"level" for ddr_qos, which if I read the code correctly you later
treat in some special way anyways.
> +};
> +
> +struct scmi_qcom_memory_range {
> + unsigned int min_freq;
> + unsigned int max_freq;
> +};
> +
> +enum common_ev_idx {
> + INST_IDX,
> + CYC_IDX,
> + CONST_CYC_IDX,
> + FE_STALL_IDX,
> + BE_STALL_IDX,
> + NUM_COMMON_EVS
> +};
> +
> +enum grp_ev_idx {
> + MISS_IDX,
> + WB_IDX,
> + ACC_IDX,
> + NUM_GRP_EVS
> +};
> +
> +/*
> + * CPUCP firmware identifies memory groups by a small integer (the hw_type
> + * carried in node_msg / scalar_param_msg / map_param_msg / ev_map_msg).
Ok, sounds reasonable.
> The
> + * encoding is shared between the cfg tables below and scmi_qcom_devfreq_get_cur_freq()
> + * which special-cases DDR_QOS as a level-based bus rather than a frequency-scaled one.
The constants are shared between A and B, both being described as
in-driver users. So is this constants shared between the first (as you
said above) or an enumeration within the driver?
> + */
> +enum scmi_qcom_memlat_hw_type {
> + MEMLAT_HW_DDR = 0,
> + MEMLAT_HW_LLCC = 1,
> + MEMLAT_HW_DDR_QOS_COMPUTE = 2,
Please use "enum" for enumerations and #define for constants. (These
aren't enumerations, as they are defined constants)
> +};
> +
> +struct scmi_qcom_monitor_cfg {
> + const struct scmi_qcom_map_table *table;
> + const char *name;
> + u32 be_stall_floor;
What is a "be stall floor"? Also, it seems to be 1 in all your cases. Is
it boolean? Is it constant?
> + u32 cpu_mask;
> + u32 ipm_ceil;
> + int table_len;
Why is this signed?
> +};
> +
> +struct scmi_qcom_memory_cfg {
> + const struct scmi_qcom_monitor_cfg *monitor_cfg;
> + const struct scmi_qcom_opp_data *mem_table;
> + struct scmi_qcom_memory_range memory_range;
> + const u32 *grp_ev;
> + const char *name;
> + u32 memory_type;
As the memory type isn't an enumeration, I asked you above to make it
constants. But in the form you wrote it, there's no reason for this not
to have its enum type.
> + int monitor_cnt;
> + int num_opps;
Aren't these unsigned?
> +};
> +
> +struct scmi_qcom_memlat_cfg_data {
> + const struct scmi_qcom_memory_cfg *memory_cfg;
> + const u32 *common_ev;
> + u32 cpucp_freq_method;
> + u32 cpucp_sample_ms;
> + int memory_cnt;
Unsigned...
> +};
> +
> +static const u32 glymur_common_ev[NUM_COMMON_EVS] = {
> + [INST_IDX] = EV_INST_RETIRED,
> + [CYC_IDX] = EV_CPU_CYCLES,
> + [CONST_CYC_IDX] = EV_CNT_CYCLES,
> + [FE_STALL_IDX] = INVALID_IDX,
> + [BE_STALL_IDX] = EV_STALL_BACKEND_MEM,
> +};
> +
> +static const u32 glymur_ddr_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const u32 glymur_llcc_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const u32 glymur_ddr_qos_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const u32 hamoa_common_ev[NUM_COMMON_EVS] = {
> + [INST_IDX] = EV_INST_RETIRED,
> + [CYC_IDX] = EV_CPU_CYCLES,
> + [CONST_CYC_IDX] = EV_CNT_CYCLES,
> + [FE_STALL_IDX] = INVALID_IDX,
> + [BE_STALL_IDX] = EV_STALL_BACKEND_MEM,
> +};
> +
> +static const u32 hamoa_ddr_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const u32 hamoa_llcc_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const u32 hamoa_ddr_qos_grp_ev[NUM_GRP_EVS] = {
> + [MISS_IDX] = EV_L2_D_RFILL,
> + [WB_IDX] = INVALID_IDX,
> + [ACC_IDX] = INVALID_IDX,
> +};
> +
> +static const struct scmi_qcom_opp_data glymur_llcc_table[] = {
> + { .freq = 315000000 },
> + { .freq = 479000000 },
> + { .freq = 545000000 },
> + { .freq = 725000000 },
> + { .freq = 840000000 },
> + { .freq = 959000000 },
> + { .freq = 1090000000 },
> + { .freq = 1211000000 },
> +};
> +
> +static const struct scmi_qcom_opp_data hamoa_llcc_table[] = {
> + { .freq = 300000000 },
> + { .freq = 466000000 },
> + { .freq = 600000000 },
> + { .freq = 806000000 },
> + { .freq = 933000000 },
> + { .freq = 1066000000 },
> +};
> +
> +static const struct scmi_qcom_opp_data glymur_ddr_table[] = {
> + { .freq = 200000000 },
> + { .freq = 547000000 },
> + { .freq = 1353000000 },
> + { .freq = 1555000000 },
> + { .freq = 1708000000 },
> + { .freq = 2092000000 },
> + { .freq = 2736000000 },
> + { .freq = 3187000000 },
> + { .freq = 3686000000 },
> + { .freq = 4224000000 },
> + { .freq = 4761000000 },
> +};
> +
> +static const struct scmi_qcom_opp_data hamoa_ddr_table[] = {
> + { .freq = 200000000 },
> + { .freq = 547000000 },
> + { .freq = 768000000 },
> + { .freq = 1555000000 },
> + { .freq = 1708000000 },
> + { .freq = 2092000000 },
> + { .freq = 2736000000 },
> + { .freq = 3187000000 },
> + { .freq = 3686000000 },
> + { .freq = 4224000000 },
> +};
> +
> +/*
> + * DDR_QOS is a level-based bus (0 = nominal, 1 = boost), not a
> + * frequency-scaled one.
"level-based"? Looks more like "boolean"?
So I presume "not a frequency-scaled one" should be interpreted as the
.freq is bogus?
> The OPP entries below use synthetic frequencies
> + * (1 / 100) purely as distinct devfreq keys so trans_stat can show
> + * level transitions.
1/100? 0.01?
Why are the frequencies 1 and 100, why not 0 and 1, or 0 and 100?
> scmi_qcom_devfreq_get_cur_freq() maps the firmware
> + * level back to the matching key.
What is the "key" here?
> + */
> +static const struct scmi_qcom_opp_data glymur_ddr_qos_table[] = {
> + { .freq = 1, .level = 0 },
> + { .freq = 100, .level = 1 },
> +};
> +
> +static const struct scmi_qcom_memory_cfg glymur_memory_cfg[] = {
> + {
> + .memory_type = MEMLAT_HW_DDR,
> + .name = "ddr",
> + .mem_table = glymur_ddr_table,
> + .num_opps = ARRAY_SIZE(glymur_ddr_table),
> + .grp_ev = glymur_ddr_grp_ev,
> + .monitor_cnt = 4,
> + .memory_range = { .min_freq = 547000, .max_freq = 4761000},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0x3f,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 8,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 960, .mem_freq = 547000 },
> + { .cpu_freq = 1133, .mem_freq = 1353000 },
> + { .cpu_freq = 1594, .mem_freq = 1555000 },
> + { .cpu_freq = 1920, .mem_freq = 1708000 },
> + { .cpu_freq = 2228, .mem_freq = 2736000 },
> + { .cpu_freq = 2362, .mem_freq = 3187000 },
> + { .cpu_freq = 2650, .mem_freq = 3686000 },
> + { .cpu_freq = 2938, .mem_freq = 4761000 },
Why are these tables hard coded in the driver? Are they constant?
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xfc0,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 8,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 356, .mem_freq = 547000 },
> + { .cpu_freq = 1018, .mem_freq = 1353000 },
> + { .cpu_freq = 1536, .mem_freq = 1555000 },
> + { .cpu_freq = 1748, .mem_freq = 1708000 },
> + { .cpu_freq = 2324, .mem_freq = 2736000 },
> + { .cpu_freq = 2496, .mem_freq = 3187000 },
> + { .cpu_freq = 2900, .mem_freq = 3686000 },
> + { .cpu_freq = 3514, .mem_freq = 4761000 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0x3f000,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 8,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 356, .mem_freq = 547000 },
> + { .cpu_freq = 1018, .mem_freq = 1353000 },
> + { .cpu_freq = 1536, .mem_freq = 1555000 },
> + { .cpu_freq = 1748, .mem_freq = 1708000 },
> + { .cpu_freq = 2324, .mem_freq = 2736000 },
> + { .cpu_freq = 2496, .mem_freq = 3187000 },
> + { .cpu_freq = 2900, .mem_freq = 3686000 },
> + { .cpu_freq = 3514, .mem_freq = 4761000 },
> + }
> + },
> + {
> + .name = "mon_3",
> + .cpu_mask = 0x3ffff,
> + .table_len = 4,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2823, .mem_freq = 547000 },
> + { .cpu_freq = 3034, .mem_freq = 1555000 },
> + { .cpu_freq = 3226, .mem_freq = 1708000 },
> + { .cpu_freq = 5012, .mem_freq = 2092000 },
> + }
> + },
> + },
> + },
> + {
> + .memory_type = MEMLAT_HW_LLCC,
> + .name = "llcc",
> + .mem_table = glymur_llcc_table,
> + .num_opps = ARRAY_SIZE(glymur_llcc_table),
> + .grp_ev = glymur_llcc_grp_ev,
> + .monitor_cnt = 3,
> + .memory_range = { .min_freq = 315000, .max_freq = 1211000},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0x3f,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 7,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 960, .mem_freq = 315000 },
> + { .cpu_freq = 1113, .mem_freq = 479000 },
> + { .cpu_freq = 1594, .mem_freq = 545000 },
> + { .cpu_freq = 1920, .mem_freq = 725000 },
> + { .cpu_freq = 2362, .mem_freq = 840000 },
> + { .cpu_freq = 2650, .mem_freq = 959000 },
> + { .cpu_freq = 2938, .mem_freq = 1211000 },
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xfc0,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 7,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 356, .mem_freq = 315000 },
> + { .cpu_freq = 1018, .mem_freq = 479000 },
> + { .cpu_freq = 1536, .mem_freq = 545000 },
> + { .cpu_freq = 1748, .mem_freq = 725000 },
> + { .cpu_freq = 2496, .mem_freq = 840000 },
> + { .cpu_freq = 2900, .mem_freq = 959000 },
> + { .cpu_freq = 3514, .mem_freq = 1211000 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0x3f000,
> + .ipm_ceil = 60000000,
> + .be_stall_floor = 1,
> + .table_len = 7,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 356, .mem_freq = 315000 },
> + { .cpu_freq = 1018, .mem_freq = 479000 },
> + { .cpu_freq = 1536, .mem_freq = 545000 },
> + { .cpu_freq = 1748, .mem_freq = 725000 },
> + { .cpu_freq = 2496, .mem_freq = 840000 },
> + { .cpu_freq = 2900, .mem_freq = 959000 },
> + { .cpu_freq = 3514, .mem_freq = 1211000 },
> + }
> + },
> + },
> + },
> + {
> + .memory_type = MEMLAT_HW_DDR_QOS_COMPUTE,
> + .name = "ddr-qos",
> + .monitor_cnt = 3,
> + .mem_table = glymur_ddr_qos_table,
> + .num_opps = ARRAY_SIZE(glymur_ddr_qos_table),
> + .grp_ev = glymur_ddr_qos_grp_ev,
> + .memory_range = { .min_freq = 0, .max_freq = 1},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0x3f,
> + .ipm_ceil = 80000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2362, .mem_freq = 0 },
> + { .cpu_freq = 2938, .mem_freq = 1 },
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xfc0,
> + .ipm_ceil = 80000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2496, .mem_freq = 0 },
> + { .cpu_freq = 3514, .mem_freq = 1 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0x3f000,
> + .ipm_ceil = 80000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2496, .mem_freq = 0 },
> + { .cpu_freq = 3514, .mem_freq = 1 },
> + }
> + },
> + },
> + },
> +};
> +
> +static const struct scmi_qcom_memory_cfg hamoa_memory_cfg[] = {
> + {
> + .memory_type = MEMLAT_HW_DDR,
> + .name = "ddr",
> + .mem_table = hamoa_ddr_table,
> + .num_opps = ARRAY_SIZE(hamoa_ddr_table),
> + .grp_ev = hamoa_ddr_grp_ev,
> + .monitor_cnt = 4,
> + .memory_range = { .min_freq = 200000, .max_freq = 4224000},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0xf,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 547000 },
> + { .cpu_freq = 1440, .mem_freq = 768000 },
> + { .cpu_freq = 1671, .mem_freq = 1555000 },
> + { .cpu_freq = 2189, .mem_freq = 2092000 },
> + { .cpu_freq = 2516, .mem_freq = 3187000 },
> + { .cpu_freq = 3860, .mem_freq = 4224000 },
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xf0,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 547000 },
> + { .cpu_freq = 1440, .mem_freq = 768000 },
> + { .cpu_freq = 1671, .mem_freq = 1555000 },
> + { .cpu_freq = 2189, .mem_freq = 2092000 },
> + { .cpu_freq = 2516, .mem_freq = 3187000 },
> + { .cpu_freq = 3860, .mem_freq = 4224000 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0xf00,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 547000 },
> + { .cpu_freq = 1440, .mem_freq = 768000 },
> + { .cpu_freq = 1671, .mem_freq = 1555000 },
> + { .cpu_freq = 2189, .mem_freq = 2092000 },
> + { .cpu_freq = 2516, .mem_freq = 3187000 },
> + { .cpu_freq = 3860, .mem_freq = 4224000 },
> + }
> + },
> + {
> + .name = "mon_3",
> + .cpu_mask = 0xfff,
> + .table_len = 4,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 1440, .mem_freq = 547000 },
> + { .cpu_freq = 2189, .mem_freq = 768000 },
> + { .cpu_freq = 2516, .mem_freq = 1555000 },
> + { .cpu_freq = 3860, .mem_freq = 2092000 },
> + }
> + },
> + },
> + },
> + {
> + .memory_type = MEMLAT_HW_LLCC,
> + .name = "llcc",
> + .mem_table = hamoa_llcc_table,
> + .num_opps = ARRAY_SIZE(hamoa_llcc_table),
> + .grp_ev = hamoa_llcc_grp_ev,
> + .monitor_cnt = 3,
> + .memory_range = { .min_freq = 300000, .max_freq = 1066000},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0xf,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 300000 },
> + { .cpu_freq = 1440, .mem_freq = 466000 },
> + { .cpu_freq = 1671, .mem_freq = 600000 },
> + { .cpu_freq = 2189, .mem_freq = 806000 },
> + { .cpu_freq = 2516, .mem_freq = 933000 },
> + { .cpu_freq = 3860, .mem_freq = 1066000 },
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xf0,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 300000 },
> + { .cpu_freq = 1440, .mem_freq = 466000 },
> + { .cpu_freq = 1671, .mem_freq = 600000 },
> + { .cpu_freq = 2189, .mem_freq = 806000 },
> + { .cpu_freq = 2516, .mem_freq = 933000 },
> + { .cpu_freq = 3860, .mem_freq = 1066000 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0xf00,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 6,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 999, .mem_freq = 300000 },
> + { .cpu_freq = 1440, .mem_freq = 466000 },
> + { .cpu_freq = 1671, .mem_freq = 600000 },
> + { .cpu_freq = 2189, .mem_freq = 806000 },
> + { .cpu_freq = 2516, .mem_freq = 933000 },
> + { .cpu_freq = 3860, .mem_freq = 1066000 },
> + }
> + },
> + },
> + },
> + {
> + .memory_type = MEMLAT_HW_DDR_QOS_COMPUTE,
> + .name = "ddr-qos",
> + .monitor_cnt = 3,
> + .mem_table = glymur_ddr_qos_table,
> + .num_opps = ARRAY_SIZE(glymur_ddr_qos_table),
> + .grp_ev = hamoa_ddr_qos_grp_ev,
> + .memory_range = { .min_freq = 0, .max_freq = 1},
> + .monitor_cfg = (const struct scmi_qcom_monitor_cfg[]) {
> + {
> + .name = "mon_0",
> + .cpu_mask = 0xf,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2189, .mem_freq = 0 },
> + { .cpu_freq = 3860, .mem_freq = 1 },
> + }
> + },
> + {
> + .name = "mon_1",
> + .cpu_mask = 0xf0,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2189, .mem_freq = 0 },
> + { .cpu_freq = 3860, .mem_freq = 1 },
> + }
> + },
> + {
> + .name = "mon_2",
> + .cpu_mask = 0xf00,
> + .ipm_ceil = 20000000,
> + .be_stall_floor = 1,
> + .table_len = 2,
> + .table = (const struct scmi_qcom_map_table[]) {
> + { .cpu_freq = 2189, .mem_freq = 0 },
> + { .cpu_freq = 3860, .mem_freq = 1 },
> + }
> + },
> + },
> + },
> +};
> +
> +static const struct scmi_qcom_memlat_cfg_data glymur_memlat_data = {
> + .memory_cfg = glymur_memory_cfg,
> + .common_ev = glymur_common_ev,
> + .cpucp_freq_method = CPUCP_EFFECTIVE_FREQ_CALC_METHOD_1,
> + .cpucp_sample_ms = 4,
> + .memory_cnt = ARRAY_SIZE(glymur_memory_cfg),
> +};
> +
> +static const struct scmi_qcom_memlat_cfg_data hamoa_memlat_data = {
> + .memory_cfg = hamoa_memory_cfg,
> + .common_ev = hamoa_common_ev,
> + .cpucp_freq_method = CPUCP_EFFECTIVE_FREQ_CALC_METHOD_1,
> + .cpucp_sample_ms = 4,
> + .memory_cnt = ARRAY_SIZE(hamoa_memory_cfg),
> +};
It would be wonderful to declare scmi_qcom_memlat_configs[] in the
vicinity of these two definitions.
> +
> +#endif
> diff --git a/drivers/devfreq/scmi-qcom-memlat-devfreq.c b/drivers/devfreq/scmi-qcom-memlat-devfreq.c
> new file mode 100644
> index 000000000000..99ae229acfdc
> --- /dev/null
> +++ b/drivers/devfreq/scmi-qcom-memlat-devfreq.c
> @@ -0,0 +1,616 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/cpu.h>
> +#include <linux/devfreq.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_qcom_protocol.h>
> +#include <linux/units.h>
> +
> +#define MAX_NAME_LEN 20
> +#define MAX_MAP_ENTRIES 10
The names used for these defines are a bit generic...
> +
> +#include "scmi-qcom-memlat-cfg.h"
> +
> +/**
> + * enum scmi_memlat_protocol_cmd - parameter_ids supported by the "MEMLAT" algo_str hosted
> + * by the Qualcomm Generic Vendor Protocol on the SCMI controller.
This is supposed to be the short description, yet it's the same length
as the long description on the very next line.
> + *
> + * MEMLAT (Memory Latency) monitors the counters to detect memory latency bound workloads
> + * and scales the frequency/levels of the memory buses accordingly.
> + *
> + * @MEMLAT_SET_MEM_GROUP: initializes the frequency/level scaling functions for the memory bus.
> + * @MEMLAT_SET_MONITOR: configures the monitor to work on a specific memory bus.
> + * @MEMLAT_SET_COMMON_EV_MAP: set up common counters used to monitor the cpu frequency.
> + * @MEMLAT_SET_GRP_EV_MAP: set up any specific counters used to monitor the memory bus.
> + * @MEMLAT_IPM_CEIL: set the IPM (Instruction Per Misses) ceiling per monitor.
> + * @MEMLAT_BE_STALL_FLOOR: set the back-end stall floor per monitor.
> + * @MEMLAT_SAMPLE_MS: set the sampling period for all the monitors.
> + * @MEMLAT_MON_FREQ_MAP: setup the cpufreq to memfreq map.
> + * @MEMLAT_SET_MIN_FREQ: set the min frequency of the memory bus.
> + * @MEMLAT_SET_MAX_FREQ: set the max frequency of the memory bus.
> + * @MEMLAT_GET_CUR_FREQ: query the current frequency/level of the memory bus.
> + * @MEMLAT_START_TIMER: start all the monitors with the requested sampling period.
> + * @MEMLAT_STOP_TIMER: stop all the running monitors.
> + * @MEMLAT_SET_EFFECTIVE_FREQ_METHOD: set the method used to determine cpu frequency.
> + */
> +enum scmi_memlat_protocol_cmd {
Don't use "enum" when you don't mean "enumeration".
> + MEMLAT_SET_MEM_GROUP = 16,
> + MEMLAT_SET_MONITOR,
> + MEMLAT_SET_COMMON_EV_MAP,
> + MEMLAT_SET_GRP_EV_MAP,
> + MEMLAT_IPM_CEIL = 23,
> + MEMLAT_BE_STALL_FLOOR = 25,
> + MEMLAT_SAMPLE_MS = 31,
> + MEMLAT_MON_FREQ_MAP,
> + MEMLAT_SET_MIN_FREQ,
> + MEMLAT_SET_MAX_FREQ,
> + MEMLAT_GET_CUR_FREQ,
> + MEMLAT_START_TIMER = 36,
> + MEMLAT_STOP_TIMER,
> + MEMLAT_SET_EFFECTIVE_FREQ_METHOD = 39,
> +};
> +
> +struct cpucp_map_table {
> + __le16 v1;
> + __le16 v2;
v1 v2?
As far as I can tell these two numbers are cpu and memory frequency.
> +};
> +
> +struct map_param_msg {
> + __le32 hw_type;
> + __le32 mon_idx;
> + __le32 nr_rows;
> + struct cpucp_map_table tbl[MAX_MAP_ENTRIES];
> +} __packed;
Does this really need the __packed? And if so, why is the only message
that you explicitly mark as packed?
> +
> +struct node_msg {
> + __le32 cpumask;
> + __le32 hw_type;
> + __le32 mon_type;
> + __le32 mon_idx;
> + char mon_name[MAX_NAME_LEN];
> +};
> +
> +struct scalar_param_msg {
> + __le32 hw_type;
> + __le32 mon_idx;
> + __le32 val;
> +};
> +
> +struct ev_map_msg {
> + __le32 num_evs;
> + __le32 hw_type;
> + __le32 cid[NUM_COMMON_EVS];
> +};
> +
> +struct scmi_qcom_memlat_map {
> + unsigned int cpufreq_mhz;
> + unsigned int memfreq_khz;
unit soup
> +};
> +
> +struct scmi_qcom_monitor_info {
> + struct scmi_qcom_memlat_map *freq_map;
> + char name[MAX_NAME_LEN];
> + u32 mon_idx;
> + u32 mon_type;
> + u32 ipm_ceil;
> + u32 be_stall_floor;
> + u32 mask;
> + u32 freq_map_len;
Most of these values are merely copied from const tables to heap on
probe - verbatim - and then put in messages. Would it be possible to
just reference the static configuration, or are the any dynamic aspects
that I'm missing?
> +};
> +
> +struct scmi_qcom_memory_info {
> + struct scmi_qcom_monitor_info **monitor;
> + u32 hw_type;
> + int monitor_cnt;
Why signed?
> + u32 min_freq;
> + u32 max_freq;
Throughout the driver we see frequencies expressed in mhz, khz, and hz.
Suffix these to help the reader.
> + struct devfreq_dev_profile profile;
> + struct devfreq *devfreq;
> + struct platform_device *pdev;
> + struct scmi_protocol_handle *ph;
> + const struct qcom_generic_ext_ops *ops;
> +};
> +
> +struct scmi_qcom_memlat_info {
> + struct scmi_protocol_handle *ph;
> + const struct qcom_generic_ext_ops *ops;
> + const struct scmi_qcom_memlat_cfg_data *cfg_data;
> + struct scmi_qcom_memory_info **memory;
> + u32 cpucp_freq_method;
> + u32 cpucp_sample_ms;
> + int memory_cnt;
Sounds like an unsigned to me
> +};
> +
> +static int configure_cpucp_common_events(struct scmi_qcom_memlat_info *info,
> + const struct scmi_qcom_memlat_cfg_data *cfg_data)
> +{
> + const struct qcom_generic_ext_ops *ops = info->ops;
> + struct ev_map_msg msg = {};
> + int i;
> +
> + msg.num_evs = cpu_to_le32(NUM_COMMON_EVS);
> + /* Common events apply to all groups; INVALID_IDX flags "no specific group". */
Does hw_type of INVALID_IDX mean "all groups", or "any (no specific)
group"?
Is the protocol defined that you pass "invalid index" for both hw_type
and cid? Or do we just reuse the INVALID_IDX define because it happens
to have the right value?
Why not "#define HW_TYPE_ALL 0xff"? Then you can skip the second half of
the comment above, which only purpose is to clarify why the code looks
"wrong".
> + msg.hw_type = cpu_to_le32(INVALID_IDX);
> + for (i = 0; i < NUM_COMMON_EVS; i++)
> + msg.cid[i] = cpu_to_le32(cfg_data->common_ev[i]);
> +
> + return ops->set_param(info->ph, &msg, sizeof(msg), MEMLAT_ALGO_STR,
This is always info->ops->set_param(info->ph, ..., ..., MEMLAT_ALGO_STR,
...), why not create a helper unction that takes info, the command,
message and size instead?
Would make the code less noisy.
> + MEMLAT_SET_COMMON_EV_MAP);
> +}
> +
> +static int configure_cpucp_grp(struct device *dev, struct scmi_qcom_memlat_info *info,
> + const struct scmi_qcom_memlat_cfg_data *cfg_data,
> + int memory_index)
> +{
> + const u32 *grp_ev = cfg_data->memory_cfg[memory_index].grp_ev;
> + struct scmi_qcom_memory_info *memory = info->memory[memory_index];
> + const struct qcom_generic_ext_ops *ops = info->ops;
> + struct ev_map_msg ev_msg = {};
> + struct node_msg msg = {};
> + int ret;
> + int i;
> +
> + msg.cpumask = cpu_to_le32(*cpumask_bits(cpu_possible_mask));
> + msg.hw_type = cpu_to_le32(memory->hw_type);
> + msg.mon_type = 0;
> + msg.mon_idx = 0;
> + ret = ops->set_param(info->ph, &msg, sizeof(msg), MEMLAT_ALGO_STR, MEMLAT_SET_MEM_GROUP);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to configure mem type %d\n",
> + memory->hw_type);
> +
> + ev_msg.num_evs = cpu_to_le32(NUM_GRP_EVS);
> + ev_msg.hw_type = cpu_to_le32(memory->hw_type);
> + for (i = 0; i < NUM_GRP_EVS; i++)
> + ev_msg.cid[i] = cpu_to_le32(grp_ev[i]);
> +
> + ret = ops->set_param(info->ph, &ev_msg, sizeof(ev_msg), MEMLAT_ALGO_STR,
> + MEMLAT_SET_GRP_EV_MAP);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to configure event map for mem type %d\n",
> + memory->hw_type);
> +
> + return ret;
> +}
> +
> +static int configure_cpucp_mon(struct device *dev, struct scmi_qcom_memlat_info *info,
> + int memory_index, int monitor_index)
> +{
> + const struct qcom_generic_ext_ops *ops = info->ops;
> + struct scmi_qcom_memory_info *memory = info->memory[memory_index];
> + struct scmi_qcom_monitor_info *monitor = memory->monitor[monitor_index];
> + struct scalar_param_msg scalar_msg = {};
> + struct map_param_msg map_msg = {};
> + struct node_msg msg = {};
So you have scalar_msg, map_msg, and msg... Why is "msg" just "msg" and
not "node_msg", is it special?
If you made scalar_param_msg the "msg" instead, this function would be
significantly less characters :)
In fact, is it significant that these local variables are denoted to be
messages? If you name them "scalar", "map" and "node" you will reduce
the noise below quite a bit.
Same goes for all other functions that has multiple messages - where the
message type is the more significant fact, and you choose to name the
variable "this is a message".
> + int ret;
> + int i;
> +
> + msg.cpumask = cpu_to_le32(monitor->mask);
> + msg.hw_type = cpu_to_le32(memory->hw_type);
> + msg.mon_type = cpu_to_le32(monitor->mon_type);
> + msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + strscpy(msg.mon_name, monitor->name, sizeof(msg.mon_name));
> + ret = ops->set_param(info->ph, &msg, sizeof(msg), MEMLAT_ALGO_STR, MEMLAT_SET_MONITOR);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to configure monitor %s\n",
> + monitor->name);
> +
> + scalar_msg.hw_type = cpu_to_le32(memory->hw_type);
> + scalar_msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + scalar_msg.val = cpu_to_le32(monitor->ipm_ceil);
> + ret = ops->set_param(info->ph, &scalar_msg, sizeof(scalar_msg), MEMLAT_ALGO_STR,
> + MEMLAT_IPM_CEIL);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to set ipm ceil for %s\n",
> + monitor->name);
> +
> + scalar_msg.hw_type = cpu_to_le32(memory->hw_type);
> + scalar_msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + scalar_msg.val = cpu_to_le32(monitor->be_stall_floor);
> + ret = ops->set_param(info->ph, &scalar_msg, sizeof(scalar_msg), MEMLAT_ALGO_STR,
> + MEMLAT_BE_STALL_FLOOR);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to set be_stall_floor for %s\n",
> + monitor->name);
> +
> + map_msg.hw_type = cpu_to_le32(memory->hw_type);
> + map_msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + map_msg.nr_rows = cpu_to_le32(monitor->freq_map_len);
> + for (i = 0; i < monitor->freq_map_len; i++) {
> + map_msg.tbl[i].v1 = cpu_to_le16(monitor->freq_map[i].cpufreq_mhz);
> +
> + /*
> + * Wire format v2 is u16 in MHz; convert from kHz.
Why is the freq_map cpufreq defined in MHz, but the memfreq in kHz - and
then we convert the memory frequency to MHz before we write it?
Why are they not both defined in MHz?
> For DDR_QOS
> + * the table holds level indices (0 / 1) rather than real
> + * frequencies, so pass them through unchanged.
"holds level indices", "pass them through unchanged"? What are you
trying to say?
> + */
> + if (monitor->freq_map[i].memfreq_khz > 1)
If you change memfreq_khz to memfreq_mhz you can remove the whole
comment, the condition, as well as the divide. There might be a reason
for you to do this, but the comment is poor, so I simply can't tell.
> + map_msg.tbl[i].v2 = cpu_to_le16(monitor->freq_map[i].memfreq_khz / 1000);
> + else
> + map_msg.tbl[i].v2 = cpu_to_le16(monitor->freq_map[i].memfreq_khz);
> + }
> + ret = ops->set_param(info->ph, &map_msg, sizeof(map_msg), MEMLAT_ALGO_STR,
> + MEMLAT_MON_FREQ_MAP);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to configure freq_map for %s\n",
> + monitor->name);
> +
> + scalar_msg.hw_type = cpu_to_le32(memory->hw_type);
> + scalar_msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + scalar_msg.val = cpu_to_le32(memory->min_freq);
> + ret = ops->set_param(info->ph, &scalar_msg, sizeof(scalar_msg), MEMLAT_ALGO_STR,
> + MEMLAT_SET_MIN_FREQ);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to set min_freq for %s\n",
> + monitor->name);
> +
> + scalar_msg.hw_type = cpu_to_le32(memory->hw_type);
> + scalar_msg.mon_idx = cpu_to_le32(monitor->mon_idx);
> + scalar_msg.val = cpu_to_le32(memory->max_freq);
> + ret = ops->set_param(info->ph, &scalar_msg, sizeof(scalar_msg), MEMLAT_ALGO_STR,
> + MEMLAT_SET_MAX_FREQ);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to set max_freq for %s\n", monitor->name);
> +
> + return ret;
> +}
> +
> +static int scmi_qcom_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> + struct scmi_qcom_memory_info *memory = dev_get_drvdata(dev);
> + const struct qcom_generic_ext_ops *ops = memory->ops;
> + struct scalar_param_msg scalar_msg = {};
> + u32 max_freq_khz = 0;
> + __le32 cur_freq;
> + int ret, i;
> +
> + /*
> + * MEMLAT_GET_CUR_FREQ returns target_freq for a single (hw_type,
> + * mon_idx) tuple. The bus's actual voted frequency is the max across
> + * all configured monitors in the group, so query each one and pick
> + * the highest vote.
You can say this with fewer words.
> + */
> + for (i = 0; i < memory->monitor_cnt; i++) {
> + scalar_msg.hw_type = cpu_to_le32(memory->hw_type);
> + scalar_msg.mon_idx = cpu_to_le32(memory->monitor[i]->mon_idx);
> + scalar_msg.val = 0;
> +
> + ret = ops->get_param(memory->ph, &scalar_msg, sizeof(scalar_msg),
> + MEMLAT_ALGO_STR, MEMLAT_GET_CUR_FREQ,
> + sizeof(cur_freq));
> + if (ret < 0) {
> + dev_err(dev, "failed to get current frequency for %s\n",
> + memory->monitor[i]->name);
> + return ret;
We're grabbing a bunch of these, is it really fatal if one fails?
> + }
> +
> + /* qcom_scmi_common_xfer() returns the response into the same tx buffer. */
What is qcom_scmi_common_xfer()?
You can express in C the fact that msg is a in/out - by declaring it as
a union.
That way you don't need a comment to describe why you're reinterpreting
the first 4 bytes of msg as the result of the query.
> + memcpy(&cur_freq, &scalar_msg, sizeof(cur_freq));
> + if (le32_to_cpu(cur_freq) > max_freq_khz)
> + max_freq_khz = le32_to_cpu(cur_freq);
And you could then le32_to_cpu() out of the union to a local variable
once, to avoid the double le32_to_cpu().
> + }
> +
> + /*
> + * Frequency-scaled buses (DDR/LLCC) report cur_freq in kHz; convert
> + * to Hz to match the devfreq OPP table units. Level-based buses
> + * (e.g. DDR_QOS_COMPUTE) configure max_freq == 1 because the firmware
> + * reports a 0/1 level rather than a frequency, and the matching
> + * synthetic OPP keys (1 / 100) live in glymur_ddr_qos_table.
You surely don't need 5 lines of solid text to express this.
> + */
> + if (memory->max_freq > 1)
> + *freq = max_freq_khz * 1000UL;
> + else
> + *freq = max_freq_khz ? 100 : 1;
> +
> + return 0;
> +}
> +
> +static void scmi_qcom_memlat_unwind(struct scmi_qcom_memlat_info *info, int count)
> +{
> + for (int i = 0; i < count; i++) {
> + struct scmi_qcom_memory_info *memory = info->memory[i];
> +
> + if (IS_ERR_OR_NULL(memory) || IS_ERR_OR_NULL(memory->pdev))
I don't get it, how can "memory" be IS_ERR()? Isn't it the result of the
devm_kcalloc() in scmi_qcom_memlat_parse_cfg()?
> + continue;
> +
> + dev_pm_opp_remove_all_dynamic(&memory->pdev->dev);
> + platform_device_unregister(memory->pdev);
> + }
> +}
> +
> +static int scmi_qcom_memlat_configure_events(struct scmi_device *sdev,
> + struct scmi_qcom_memlat_info *info)
> +{
> + const struct qcom_generic_ext_ops *ops = info->ops;
> + struct scmi_protocol_handle *ph = info->ph;
> + __le32 sample_ms, freq_method;
> + int i, j, ret;
> +
> + /* Configure common events ids */
> + ret = configure_cpucp_common_events(info, info->cfg_data);
> + if (ret < 0)
> + return dev_err_probe(&sdev->dev, ret, "failed to configure common events\n");
> +
> + for (i = 0; i < info->memory_cnt; i++) {
> + /* Configure per group parameters */
> + ret = configure_cpucp_grp(&sdev->dev, info, info->cfg_data, i);
> + if (ret < 0)
> + return ret;
> +
> + for (j = 0; j < info->memory[i]->monitor_cnt; j++) {
> + /* Configure per monitor parameters */
> + ret = configure_cpucp_mon(&sdev->dev, info, i, j);
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + /* Set loop sampling time */
> + sample_ms = cpu_to_le32(info->cpucp_sample_ms);
> + ret = ops->set_param(ph, &sample_ms, sizeof(sample_ms),
> + MEMLAT_ALGO_STR, MEMLAT_SAMPLE_MS);
> + if (ret < 0)
> + return dev_err_probe(&sdev->dev, ret, "failed to set sample_ms\n");
> +
> + /* Set the effective cpu frequency calculation method */
> + freq_method = cpu_to_le32(info->cpucp_freq_method);
> + ret = ops->set_param(ph, &freq_method, sizeof(freq_method),
> + MEMLAT_ALGO_STR, MEMLAT_SET_EFFECTIVE_FREQ_METHOD);
> + if (ret < 0)
> + return dev_err_probe(&sdev->dev, ret,
> + "failed to set effective frequency calc method\n");
> +
> + /* Start sampling and voting timer */
> + ret = ops->start_activity(ph, NULL, 0, MEMLAT_ALGO_STR, MEMLAT_START_TIMER);
> + if (ret < 0)
> + return dev_err_probe(&sdev->dev, ret, "failed to start memory group timer\n");
> +
> + for (i = 0; i < info->memory_cnt; i++) {
> + struct scmi_qcom_memory_info *memory = info->memory[i];
> + struct platform_device *pdev = memory->pdev;
> + struct devfreq_dev_profile *profile = &memory->profile;
> +
> + /* sampling time should be double the devfreq observing time */
That's interesting, tell me more...
> + profile->polling_ms = max(1U, info->cpucp_sample_ms / 2);
> + profile->get_cur_freq = scmi_qcom_devfreq_get_cur_freq;
> + profile->initial_freq = memory->min_freq > 1 ?
> + (memory->min_freq * 1000UL) : memory->min_freq;
What is the purpose of this conditional?
You support memories with an actual frequency, and what you call
"level". But for any memory with a "level" resource other than a boolean
this logic fails.
For min_freq == 0 we get initial_freq of 0, both for frequency-based and
level-based memories.
So the only case where this actually does anything is if you have a
boolean resource with minimum value of 1 (i.e. a constant). Do we have
those?
> +
> + platform_set_drvdata(pdev, memory);
> +
> + memory->devfreq = devm_devfreq_add_device(&pdev->dev, profile,
> + DEVFREQ_GOV_REMOTE, NULL);
> + if (IS_ERR(memory->devfreq)) {
> + dev_err(&sdev->dev, "failed to add devfreq device\n");
I personally don't like all the dev_err_probe() that you have sprinkled
throughout the driver, but I know that many others do - and this one
isn't consistent.
> + /* Stop sampling and voting timer */
> + ret = ops->stop_activity(ph, NULL, 0, MEMLAT_ALGO_STR, MEMLAT_STOP_TIMER);
> + if (ret < 0)
> + dev_err_probe(&sdev->dev, ret,
> + "failed to stop memory group timer\n");
The significant event is that adding the devfreq device failed, but the
error message that you retain is "failed to stop memory group timer"...
> + return PTR_ERR(memory->devfreq);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct scmi_qcom_memlat_map *
> +scmi_qcom_parse_memlat_map(struct device *dev, const struct scmi_qcom_monitor_cfg *mon_cfg)
> +{
> + struct scmi_qcom_memlat_map *map_table;
> + const struct scmi_qcom_map_table *table;
> +
> + if (mon_cfg->table_len > MAX_MAP_ENTRIES)
> + return ERR_PTR(-EINVAL);
> +
> + map_table = devm_kcalloc(dev, mon_cfg->table_len, sizeof(*map_table),
> + GFP_KERNEL);
> + if (!map_table)
> + return ERR_PTR(-ENOMEM);
> +
> + for (int i = 0; i < mon_cfg->table_len; i++) {
> + table = &mon_cfg->table[i];
> +
> + map_table[i].cpufreq_mhz = table->cpu_freq;
> + map_table[i].memfreq_khz = table->mem_freq;
> + }
> +
> + return map_table;
> +}
> +
> +static const struct of_device_id scmi_qcom_memlat_configs[] = {
> + { .compatible = "qcom,glymur", .data = &glymur_memlat_data},
> + { .compatible = "qcom,mahua", .data = &glymur_memlat_data},
> + { .compatible = "qcom,x1e80100", .data = &hamoa_memlat_data},
> + { .compatible = "qcom,x1p42100", .data = &hamoa_memlat_data},
> + { }
> +};
> +
> +static int scmi_qcom_memlat_parse_cfg(struct scmi_device *sdev, struct scmi_qcom_memlat_info *info)
> +{
> + const struct scmi_qcom_memlat_cfg_data *cfg_data;
> + struct scmi_qcom_monitor_info *monitor;
> + struct scmi_qcom_memory_info *memory;
> + int ret, i, j;
> +
> + cfg_data = of_machine_get_match_data(scmi_qcom_memlat_configs);
> + if (!cfg_data) {
> + /*
> + * The SCMI generic-ext protocol can bind on Qualcomm SoCs that
> + * do not ship CPUCP memlat. Bail out quietly in that case rather
> + * than printing an error on every such system.
> + */
> + dev_dbg(&sdev->dev, "no memlat config data for this platform\n");
> + return -ENODEV;
> + }
> +
> + info->memory = devm_kcalloc(&sdev->dev, cfg_data->memory_cnt,
> + sizeof(*info->memory), GFP_KERNEL);
> + if (!info->memory)
> + return -ENOMEM;
> +
> + for (i = 0; i < cfg_data->memory_cnt; i++) {
> + const struct scmi_qcom_memory_cfg *memory_cfg = &cfg_data->memory_cfg[i];
> + struct platform_device_info pdevinfo = { 0 };
> +
> + pdevinfo.parent = &sdev->dev;
> + pdevinfo.name = memory_cfg->name;
> + pdevinfo.id = PLATFORM_DEVID_NONE;
> +
> + memory = devm_kzalloc(&sdev->dev, sizeof(*memory), GFP_KERNEL);
> + if (!memory)
> + return -ENOMEM;
> +
> + memory->ops = info->ops;
> + memory->ph = info->ph;
> + memory->hw_type = memory_cfg->memory_type;
> + memory->monitor_cnt = memory_cfg->monitor_cnt;
> + memory->min_freq = memory_cfg->memory_range.min_freq;
> + memory->max_freq = memory_cfg->memory_range.max_freq;
> +
> + memory->pdev = platform_device_register_full(&pdevinfo);
Why are there platform_devices in here?
I'm guessing that you want the struct devices to hang the opp tables
off? But why a whole platform_device?
> + if (IS_ERR(memory->pdev))
> + return dev_err_probe(&sdev->dev, PTR_ERR(memory->pdev),
> + "failed to register platform device\n");
> +
> + info->memory[i] = memory;
> + /* Track progress so probe() can unwind on a later failure. */
Please clean up your own mess. If this function becomes too long for you
to be able to get things in order, that's just another indication that
you need to refactor this function.
> + info->memory_cnt = i + 1;
> +
> + for (j = 0; j < memory_cfg->num_opps; j++) {
> + const struct scmi_qcom_opp_data *table = &memory_cfg->mem_table[j];
> + struct platform_device *pdev = memory->pdev;
> + struct dev_pm_opp_data data = {};
> +
> + data.freq = table->freq;
> + data.level = table->level;
> +
> + ret = dev_pm_opp_add_dynamic(&pdev->dev, &data);
> + if (ret)
> + return dev_err_probe(&sdev->dev, ret, "failed to add OPP\n");
> + }
> +
> + memory->monitor = devm_kcalloc(&sdev->dev, memory_cfg->monitor_cnt,
> + sizeof(*memory->monitor), GFP_KERNEL);
> + if (!memory->monitor)
> + return -ENOMEM;
> +
> + for (j = 0; j < memory_cfg->monitor_cnt; j++) {
> + const struct scmi_qcom_monitor_cfg *mon_cfg = &memory_cfg->monitor_cfg[j];
> +
> + monitor = devm_kzalloc(&sdev->dev, sizeof(*monitor), GFP_KERNEL);
> + if (!monitor)
> + return -ENOMEM;
> +
> + monitor->ipm_ceil = mon_cfg->ipm_ceil;
> + /* mon_type 0 = IPM-based latency monitor; 1 = stall-only (compute) */
> + monitor->mon_type = monitor->ipm_ceil ? 0 : 1;
So the actual mon_type in the message is just a 0 or 1? Is this 0 the
same as the 0 in configure_cpucp_grp()? Should we have a couple of
defines for these - instead of the comment?
> + monitor->be_stall_floor = mon_cfg->be_stall_floor;
> + monitor->mask = mon_cfg->cpu_mask;
> + monitor->freq_map_len = mon_cfg->table_len;
> +
> + monitor->freq_map = scmi_qcom_parse_memlat_map(&sdev->dev, mon_cfg);
It might be a good idea to do this before you're starting to register
platform_devices and opps, as it would simplify the cleanup.
> + if (IS_ERR(monitor->freq_map))
> + return dev_err_probe(&sdev->dev, PTR_ERR(monitor->freq_map),
> + "failed to populate cpufreq-memfreq map\n");
There are two ways scmi_qcom_parse_memlat_map() can fail, either the
driver data is wrong and you get EINVAL (a developer BUG) or you get
ENOMEM, which is already printed...
> +
> + strscpy(monitor->name, mon_cfg->name, sizeof(monitor->name));
Do you really need a copy? As far as I can see the next access is
another copy out of this struct.
> + monitor->mon_idx = j;
> + memory->monitor[j] = monitor;
> + }
> + }
> +
> + info->cfg_data = cfg_data;
> + info->cpucp_freq_method = cfg_data->cpucp_freq_method;
> + info->cpucp_sample_ms = cfg_data->cpucp_sample_ms;
So much code just moving data between two representations...
> +
> + return 0;
> +}
> +
> +static int scmi_qcom_devfreq_memlat_probe(struct scmi_device *sdev)
> +{
> + const struct scmi_handle *handle = sdev->handle;
> + const struct qcom_generic_ext_ops *ops;
> + struct scmi_qcom_memlat_info *info;
> + struct scmi_protocol_handle *ph;
> + int ret;
> +
> + if (!handle)
> + return -ENODEV;
> +
> + info = devm_kzalloc(&sdev->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_QCOM_GENERIC, &ph);
> + if (IS_ERR(ops))
> + return PTR_ERR(ops);
> +
> + info->ops = ops;
> + info->ph = ph;
> +
> + ret = scmi_qcom_memlat_parse_cfg(sdev, info);
> + if (ret) {
> + scmi_qcom_memlat_unwind(info, info->memory_cnt);
scmi_qcom_memlat_unwind() unwinds the work done by
scmi_qcom_memlat_parse_cfg(), this isn't really idiomatic. Please have
scmi_qcom_memlat_parse_cfg() clean up its own mess on failure.
> + return ret;
> + }
> +
> + ret = scmi_qcom_memlat_configure_events(sdev, info);
> + if (ret) {
> + scmi_qcom_memlat_unwind(info, info->memory_cnt);
> + return ret;
> + }
> +
> + dev_set_drvdata(&sdev->dev, info);
> +
> + return ret;
> +}
> +
> +static void scmi_qcom_devfreq_memlat_remove(struct scmi_device *sdev)
> +{
> + struct scmi_qcom_memlat_info *info = dev_get_drvdata(&sdev->dev);
> + struct scmi_protocol_handle *ph;
> + const struct qcom_generic_ext_ops *ops;
> + int ret;
> +
> + if (!info)
How is that possible?
> + return;
> +
> + ph = info->ph;
> + ops = info->ops;
> +
> + ret = ops->stop_activity(ph, NULL, 0, MEMLAT_ALGO_STR, MEMLAT_STOP_TIMER);
> + if (ret < 0)
> + dev_err(&sdev->dev, "failed to stop memory group timer\n");
> +
> + scmi_qcom_memlat_unwind(info, info->memory_cnt);
> +}
> +
> +static const struct scmi_device_id scmi_id_table[] = {
> + { SCMI_PROTOCOL_QCOM_GENERIC, "qcom-generic-ext" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(scmi, scmi_id_table);
> +
> +static struct scmi_driver scmi_qcom_devfreq_memlat_driver = {
> + .name = "scmi-qcom-devfreq-memlat",
> + .probe = scmi_qcom_devfreq_memlat_probe,
> + .remove = scmi_qcom_devfreq_memlat_remove,
> + .id_table = scmi_id_table,
> +};
> +module_scmi_driver(scmi_qcom_devfreq_memlat_driver);
> +
> +MODULE_AUTHOR("Pragnesh Papaniya <pragnesh.papaniya@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("SCMI QCOM DEVFREQ MEMLAT driver");
Let go of the shift key!
s/QCOM/Qualcomm/ at least.
Regards,
Bjorn
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH RFC 6/8] clk: sunxi-ng: a733: Add mod clocks support
From: Enzo Adriano @ 2026-07-02 17:24 UTC (permalink / raw)
To: Junhui Liu
Cc: Andre Przywara, Brian Masney, Michael Turquette, Stephen Boyd,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Philipp Zabel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Richard Cochran,
linux-clk, linux-arm-kernel, linux-sunxi, linux-riscv, devicetree,
netdev, linux-kernel
In-Reply-To: <20260310-a733-clk-v1-6-36b4e9b24457@pigmoral.tech>
Hi Junhui,
Register check for the mod clocks in this patch against the public A733
User Manual V0.92: 88 of the 94 mod/mux clock entries match the manual's
register offsets, with the documented gating/mux/divider layout at each
offset. The six that have no register in the public manual are the set
already discussed in this thread for provenance notes - avs (0x0880),
spi4 (0x0F28), sgpio (0x1060), lpc (0x1080), gmac1-phy (0x1420) - plus
one more: tcon-lcd2 (0x1510); the manual only documents VO0_TCONLCD0
(0x1500) and VO0_TCONLCD1 (0x1508).
Thanks,
Enzo
^ permalink raw reply
* [PATCH v8 16/39] drm/display: bridge_connector: Wire up HDMI 2.0 scrambler callbacks
From: Cristian Ciocaltea @ 2026-07-02 14:46 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Sandy Huang, Heiko Stübner, Andy Yan, Daniel Stone,
Dave Stevenson, Maíra Canal, Raspberry Pi Kernel Maintenance
Cc: kernel, dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260702-dw-hdmi-qp-scramb-v8-0-d79890d00b6a@collabora.com>
Connect the bridge connector's .scrambler_{enable|disable} callbacks to
the underlying bridge's .hdmi_scrambler_{enable|disable} funcs when
HDMI 2.0 conformance is advertised.
This completes the bridge connector plumbing so that the SCDC
scrambling helpers can control source-side scrambling through the
bridge chain.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 40 +++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 17eb2548efad..ceb961a9adb5 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -578,6 +578,32 @@ static int drm_bridge_connector_write_spd_infoframe(struct drm_connector *connec
return bridge->funcs->hdmi_write_spd_infoframe(bridge, buffer, len);
}
+static int drm_bridge_connector_scrambler_enable(struct drm_connector *connector)
+{
+ struct drm_bridge_connector *bridge_connector =
+ to_drm_bridge_connector(connector);
+ struct drm_bridge *bridge;
+
+ bridge = bridge_connector->bridge_hdmi;
+ if (!bridge)
+ return -EINVAL;
+
+ return bridge->funcs->hdmi_scrambler_enable(bridge);
+}
+
+static int drm_bridge_connector_scrambler_disable(struct drm_connector *connector)
+{
+ struct drm_bridge_connector *bridge_connector =
+ to_drm_bridge_connector(connector);
+ struct drm_bridge *bridge;
+
+ bridge = bridge_connector->bridge_hdmi;
+ if (!bridge)
+ return -EINVAL;
+
+ return bridge->funcs->hdmi_scrambler_disable(bridge);
+}
+
static const struct drm_edid *
drm_bridge_connector_read_edid(struct drm_connector *connector)
{
@@ -603,7 +629,7 @@ static const struct drm_connector_hdmi_funcs drm_bridge_connector_hdmi_funcs = {
.clear_infoframe = drm_bridge_connector_clear_hdmi_infoframe,
.write_infoframe = drm_bridge_connector_write_hdmi_infoframe,
},
- /* audio, hdr_drm and spd are set dynamically during init */
+ /* scrambler, audio, hdr_drm and spd are set dynamically during init */
};
static const struct drm_connector_infoframe_funcs drm_bridge_connector_hdmi_audio_infoframe = {
@@ -912,6 +938,11 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
!bridge->funcs->hdmi_clear_spd_infoframe))
return ERR_PTR(-EINVAL);
+ if (bridge->supported_hdmi_ver >= HDMI_VERSION_2_0 &&
+ (!bridge->funcs->hdmi_scrambler_enable ||
+ !bridge->funcs->hdmi_scrambler_disable))
+ return ERR_PTR(-EINVAL);
+
bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
if (bridge->supported_hdmi_ver)
@@ -1018,6 +1049,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
bridge_connector->hdmi_funcs.spd =
drm_bridge_connector_hdmi_spd_infoframe;
+ if (bridge_connector->bridge_hdmi->supported_hdmi_ver >= HDMI_VERSION_2_0) {
+ bridge_connector->hdmi_funcs.scrambler_enable =
+ drm_bridge_connector_scrambler_enable;
+ bridge_connector->hdmi_funcs.scrambler_disable =
+ drm_bridge_connector_scrambler_disable;
+ }
+
ret = drmm_connector_hdmi_init_with_caps(drm, connector,
bridge_connector->bridge_hdmi->vendor,
bridge_connector->bridge_hdmi->product,
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 net-next 07/14] net: enetc: differentiate phylink capabilities for pseudo-MAC and standalone MAC
From: Maxime Chevallier @ 2026-07-02 17:30 UTC (permalink / raw)
To: wei.fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, kuba, pabeni, linux, wei.fang,
chleroy
Cc: imx, netdev, linux-kernel, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20260702025714.456233-8-wei.fang@oss.nxp.com>
Hi,
On 7/2/26 04:57, wei.fang@oss.nxp.com wrote:
> From: Claudiu Manoil <claudiu.manoil@nxp.com>
>
> The ENETC pseudo-MACs are proprietary internal links that do not
> implement any standard MII interface, so restrict their supported PHY
> interface modes to PHY_INTERFACE_MODE_INTERNAL only.
>
> Since pseudo-MACs can operate at any speed between 10Mbps and 25Gbps
> in multiples of 10Mbps, set their MAC capabilities to cover the full
> range of standard full-duplex speeds: 10/100/1000/2500/5000/10000/
> 20000/25000 Mbps.
>
> For standalone ENETC, expand the supported interface modes to include
> 10GBASER and XGMII in addition to the existing RGMII, SGMII, 1000BASEX,
> 2500BASEX and USXGMII modes, with MAC capabilities up to 10G. MAC_1000
> is replaced with MAC_1000FD to explicitly exclude 1000M half-duplex,
> which is not supported.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply
* Re: [PATCH v2 net-next 06/14] net: enetc: simplify enetc4_set_port_speed()
From: Maxime Chevallier @ 2026-07-02 17:32 UTC (permalink / raw)
To: wei.fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, kuba, pabeni, linux, wei.fang,
chleroy
Cc: imx, netdev, linux-kernel, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20260702025714.456233-7-wei.fang@oss.nxp.com>
On 7/2/26 04:57, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> Since phylink only passes valid speed values to mac_link_up, the switch
> statement with its default fallback to SPEED_10 is unnecessary. Replace
> it with a direct call to PCR_PSPEED_VAL(). Also update PCR_PSPEED_VAL()
> to use FIELD_PREP() for proper field masking instead of an open-coded
> shift.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply
* Re: [PATCH RFC 7/8] clk: sunxi-ng: a733: Add bus clock gates
From: Enzo Adriano @ 2026-07-02 17:34 UTC (permalink / raw)
To: Junhui Liu
Cc: Andre Przywara, Brian Masney, Michael Turquette, Stephen Boyd,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Philipp Zabel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Richard Cochran,
linux-clk, linux-arm-kernel, linux-sunxi, linux-riscv, devicetree,
netdev, linux-kernel
In-Reply-To: <20260310-a733-clk-v1-7-36b4e9b24457@pigmoral.tech>
Hi Junhui,
Following Andre's suggestion I went through the bus gates in this patch
and compared every register offset and bit position against the public
A733 User Manual V0.92. Findings below; everything not listed matched
the manual (122 of the 135 gate entries verified clean).
1) UART1-UART6 gate bits look wrong:
> +static SUNXI_CCU_GATE_HWS(bus_uart1_clk, "bus-uart1", apb_uart_hws, 0xe04, BIT(1), 0);
> +static SUNXI_CCU_GATE_HWS(bus_uart2_clk, "bus-uart2", apb_uart_hws, 0xe08, BIT(2), 0);
> +static SUNXI_CCU_GATE_HWS(bus_uart3_clk, "bus-uart3", apb_uart_hws, 0xe0c, BIT(3), 0);
> +static SUNXI_CCU_GATE_HWS(bus_uart4_clk, "bus-uart4", apb_uart_hws, 0xe10, BIT(4), 0);
> +static SUNXI_CCU_GATE_HWS(bus_uart5_clk, "bus-uart5", apb_uart_hws, 0xe14, BIT(5), 0);
> +static SUNXI_CCU_GATE_HWS(bus_uart6_clk, "bus-uart6", apb_uart_hws, 0xe18, BIT(6), 0);
Each UART has its own BGR register, and in every one of them the
gating bit is bit 0. Manual section 4.1.6.141 (0x0E04 UART1 Bus Gating
Reset Register): bit 16 "UART1_RST", bits 15:1 reserved ("/"), bit 0
"UART1_GATING - Gating Clock for UART1, 0: Mask, 1: Pass". Sections
4.1.6.142-4.1.6.146 have the same layout for UART2-UART6. So these six
entries should all use BIT(0); as written, enabling any of bus-uart1..6
sets a reserved bit and the UART clock stays gated. (bus-uart0 at 0xe00
BIT(0) and the uartN resets at bit 16 all match the manual.)
2) SYSDAP gate offset looks wrong:
> +static SUNXI_CCU_GATE_HWS(bus_sysdap_clk, "bus-sysdap", apb1_hws,
> + 0x88c, BIT(0), 0);
Manual section 4.1.6.92 puts SYSDAP_BGR_REG at 0x07AC (bit 16
"SYSDAP_RST", bit 0 "SYSDAP_GATING"), and patch 8/8's reset map already
uses { 0x7ac, BIT(16) } for RST_BUS_SYSDAP, so the gate here presumably
wants 0x7ac as well. There is no CCU register at 0x88C in the manual.
3) Gates without a register in the public manual (V0.92) - these could
use a short provenance note near the entry, as discussed for other IDs:
- bus-spi4 (0x0F2C)
- bus-sgpio (0x1064)
- bus-lpc (0x1084)
- bus-gmac1 (0x142C) [same question as the GMAC1 clock IDs]
- bus-tcon-lcd2 (0x1514, and the tcon-lcd2 mod clock at 0x1510) - the
manual documents only VO0_TCONLCD0 (0x1500/0x1504) and VO0_TCONLCD1
(0x1508/0x150C)
- mbus-gmac1 (0x05E4 bit 12) - bit 12 is not described in the MBUS
Gate Enable Register section
The remaining bus/mbus gate entries in this patch all match the manual's
offsets and bit positions.
Thanks,
Enzo
^ permalink raw reply
* Re: [PATCH v2 net-next 08/14] net: enetc: remove invalid code from enetc4_pl_mac_link_up()
From: Maxime Chevallier @ 2026-07-02 17:35 UTC (permalink / raw)
To: wei.fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, kuba, pabeni, linux, wei.fang,
chleroy
Cc: imx, netdev, linux-kernel, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20260702025714.456233-9-wei.fang@oss.nxp.com>
On 7/2/26 04:57, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> When adding phylink MAC operations support to the NETC switch driver,
> Russell King pointed out several pieces of invalid logic in the
> .mac_link_up() implementation (see [1] and [2]):
>
> 1) Half-duplex backpressure is not supported by the kernel, Ethernet
> relies on packet dropping for congestion management.
>
> 2) phylink_autoneg_inband() is unnecessary, as RGMII in-band status is
> not supported.
>
> 3) TX and RX pause are disabled in half-duplex mode, so there is no
> need to override them in .mac_link_up().
>
> The same invalid logic is also present in enetc4_pl_mac_link_up(), so
> remove the invalid code from it.
>
> Link: https://lore.kernel.org/imx/acEIQqI-_oyCym8O@shell.armlinux.org.uk/ # 1
> Link: https://lore.kernel.org/imx/acEFwqmAvWls_9Ef@shell.armlinux.org.uk/ # 2
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply
* Re: [PATCH v6] soc: aspeed: lpc-snoop: Fix usercopy overflow in snoop_file_read
From: Karthikeyan KS @ 2026-07-02 17:35 UTC (permalink / raw)
To: andrew
Cc: joel, andrew, Kees Cook, linux-arm-kernel, linux-aspeed,
linux-kernel, linux-hardening
In-Reply-To: <033f2657ae6a94ad13d22f717a2900afb75d892d.camel@codeconstruct.com.au>
Hi Andrew,
Thanks for your consistent support and guidance.
I learned a lot from you.
Much Appreciated.
Thanks,
Karthikeyan
^ permalink raw reply
* ✅ PASS: Test report for for-kernelci (7.2.0-rc1, upstream-arm-next, a52d6c71)
From: cki-project @ 2026-07-02 17:43 UTC (permalink / raw)
To: will, catalin.marinas, linux-arm-kernel
Hi, we tested your kernel and here are the results:
Overall result: PASSED
Merge: OK
Compile: OK
Test: OK
Tested-by: CKI Project <cki-project@redhat.com>
Kernel information:
Commit message: selftests/arm64: fix spelling errors in comments
You can find all the details about the test run at
https://datawarehouse.cki-project.org/kcidb/checkouts/redhat:2647180691
If you find a failure unrelated to your changes, please ask the test maintainer to review it.
This will prevent the failures from being incorrectly reported in the future.
Please reply to this email if you have any questions about the tests that we
ran or if you have any suggestions on how to make future tests more effective.
,-. ,-.
( C ) ( K ) Continuous
`-',-.`-' Kernel
( I ) Integration
`-'
______________________________________________________________________________
^ permalink raw reply
* [PATCH v8 28/39] drm/bridge: dw-hdmi-qp: Drop unused .setup_hpd() phy op
From: Cristian Ciocaltea @ 2026-07-02 14:46 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Sandy Huang, Heiko Stübner, Andy Yan, Daniel Stone,
Dave Stevenson, Maíra Canal, Raspberry Pi Kernel Maintenance
Cc: kernel, dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260702-dw-hdmi-qp-scramb-v8-0-d79890d00b6a@collabora.com>
With the transition to .{enable|disable}_hpd() phy ops complete in both
the bridge and platform drivers, .setup_hpd() has no remaining callers.
Drop it.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 3 ---
include/drm/bridge/dw_hdmi_qp.h | 1 -
2 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 4d41ad7319a5..8ce005673517 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -1314,9 +1314,6 @@ static void dw_hdmi_qp_init_hw(struct dw_hdmi_qp *hdmi)
/* Clear DONE and ERROR interrupts */
dw_hdmi_qp_write(hdmi, I2CM_OP_DONE_CLEAR | I2CM_NACK_RCVD_CLEAR,
MAINUNIT_1_INT_CLEAR);
-
- if (hdmi->phy.ops->setup_hpd)
- hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
}
struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
index b4fb1c578a5b..ad33f9cafdeb 100644
--- a/include/drm/bridge/dw_hdmi_qp.h
+++ b/include/drm/bridge/dw_hdmi_qp.h
@@ -16,7 +16,6 @@ struct dw_hdmi_qp_phy_ops {
int (*init)(struct dw_hdmi_qp *hdmi, void *data);
void (*disable)(struct dw_hdmi_qp *hdmi, void *data);
enum drm_connector_status (*read_hpd)(struct dw_hdmi_qp *hdmi, void *data);
- void (*setup_hpd)(struct dw_hdmi_qp *hdmi, void *data);
void (*enable_hpd)(struct dw_hdmi_qp *hdmi, void *data);
void (*disable_hpd)(struct dw_hdmi_qp *hdmi, void *data);
};
--
2.54.0
^ permalink raw reply related
* [PATCH 11/28] KVM: arm64: Expose FEAT_NV2p1 to NV guests
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
Since NV2p1 is reducing the number of traps, it is valuable to expose
it to NV guests. Do so.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/nested.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfb96edbdc43c..9972dea42d12a 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1728,7 +1728,7 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
* You get EITHER
*
* - FEAT_VHE without FEAT_E2H0
- * - FEAT_NV limited to FEAT_NV2
+ * - FEAT_NV limited to FEAT_NV2(p1)
* - HCR_EL2.NV1 being RES0
*
* OR
@@ -1740,7 +1740,11 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features)) {
val = 0;
} else {
- val = SYS_FIELD_PREP_ENUM(ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY);
+ if (cpus_have_final_cap(ARM64_HAS_NV2P1))
+ val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR4_EL1, NV_frac, NV2P1);
+ else
+ val = SYS_FIELD_PREP_ENUM(ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY);
+ val &= ~ID_AA64MMFR4_EL1_E2H0;
val |= SYS_FIELD_PREP_ENUM(ID_AA64MMFR4_EL1, E2H0, NI_NV1);
}
break;
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] media: rkvdec: Propagate platform_get_irq() errors
From: Detlev Casanova @ 2026-07-02 17:57 UTC (permalink / raw)
To: Narasimharao Vadlamudi
Cc: Ezequiel Garcia, Mauro Carvalho Chehab, Heiko Stuebner,
Hans Verkuil, Boris Brezillon, linux-media, linux-rockchip,
linux-arm-kernel, linux-kernel, Narasimharao Vadlamudi
In-Reply-To: <20260701111203.62092-1-ahmisaranrao@gmail.com>
Hi Vadlamudi,
On Wednesday, July 1, 2026 7:12:03 a.m. Eastern Daylight Time Narasimharao
Vadlamudi wrote:
> platform_get_irq() returns a positive IRQ number on success and a
> negative error code on failure. It no longer returns zero. The driver
> currently returns -ENXIO for all failures, which loses useful errors
> such as -EPROBE_DEFER.
>
> Return the error from platform_get_irq() directly.
>
> Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> Signed-off-by: Narasimharao Vadlamudi <ahmisaranrao@gmail.com>
> ---
> drivers/media/platform/rockchip/rkvdec/rkvdec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> b/drivers/media/platform/rockchip/rkvdec/rkvdec.c index
> 1d1e9bfef8e9..37683a6bb40f 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -1818,8 +1818,8 @@ static int rkvdec_probe(struct platform_device *pdev)
> vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0)
> - return -ENXIO;
> + if (irq < 0)
> + return irq;
>
> ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> rkvdec_irq_handler,
IRQF_ONESHOT,
Thank you for the patch ! And sorry for the HTML email before. So for the
record:
Reviewed-by: Detlev Casanova <detlev.casanova@collabora.com>
--
Detlev.
^ permalink raw reply
* Re: [PATCH v2 0/2] rockchip: Fix devices suspend freeze on RK3568/RK3566
From: Heiko Stuebner @ 2026-07-02 18:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonas Karlman
Cc: Diederik de Haas, Greg Kroah-Hartman, devicetree, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel, Jonas Karlman
In-Reply-To: <20260624192726.781864-1-jonas@kwiboo.se>
Hi Jonas,
Am Mittwoch, 24. Juni 2026, 21:27:23 Mitteleuropäische Sommerzeit schrieb Jonas Karlman:
> This series fixes a system freeze during suspend in ohci_suspend() due
> to clk_usbphy1_480m not being enabled when EHCI/OHCI registers are
> accessed on e.g. a Raxa ROCK 3C board.
>
> Following pm_test modes work on my ROCK 3C with the missing usbphy clk
> refs added:
>
> echo N > /sys/module/printk/parameters/console_suspend
>
> echo devices > /sys/power/pm_test
> echo platform > /sys/power/pm_test
> echo processors > /sys/power/pm_test
> echo core > /sys/power/pm_test
>
> echo mem > /sys/power/state
>
> Changes in v2:
> - Include rockchip,rk3588-ehci in the EHCI constraint
> - Make clocks prop required for EHCI and OHCI
> - Collect t-b tag
>
> Jonas Karlman (2):
> dt-bindings: usb: Add Rockchip RK3568 compatible for EHCI and OHCI
it seems I wasn't in Cc for the first patch, so can't know if and when
that gets/got applied.
Also I think I remember Greg preferring not having to split apart patch
series', so I think the binding patch might be better off standing alone.
Heiko
^ permalink raw reply
* Re: [PATCH RFC 8/8] clk: sunxi-ng: a733: Add reset lines
From: Enzo Adriano @ 2026-07-02 18:11 UTC (permalink / raw)
To: Junhui Liu
Cc: Andre Przywara, Brian Masney, Michael Turquette, Stephen Boyd,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Philipp Zabel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Richard Cochran,
linux-clk, linux-arm-kernel, linux-sunxi, linux-riscv, devicetree,
netdev, linux-kernel
In-Reply-To: <20260310-a733-clk-v1-8-36b4e9b24457@pigmoral.tech>
Hi Junhui,
Same exercise for the reset map: I compared all 121 entries' offsets
and bit positions against the public A733 User Manual V0.92. 115 match
the manual exactly (including the multi-bit GMAC0 entry - 0x141C bits
16/17 per section 4.1.6.212 - and RST_BUS_SYSDAP at 0x7ac, which is
what makes the gate offset in patch 7/8 stand out).
The six that have no register in the public manual, in case you want
to add provenance notes near them (as already discussed for other
undocumented IDs in this series):
- RST_BUS_SPI4 { 0x0F2C, BIT(16) }
- RST_BUS_SGPIO { 0x1064, BIT(16) }
- RST_BUS_LPC { 0x1084, BIT(16) }
- RST_BUS_GMAC1 { 0x142C, BIT(16) } [GMAC1 question]
- RST_BUS_GMAC1_AXI { 0x142C, BIT(17) } [GMAC1 question]
- RST_BUS_TCON_LCD2 { 0x1514, BIT(16) } - the manual documents only
VO0_TCONLCD0_BGR (0x1504) and VO0_TCONLCD1_BGR (0x150C); there is
no TCONLCD2 register set in V0.92
Everything else in the reset map checks out against the manual.
This analysis was done with AI assistance (Claude Code, claude-fable-5)
and each finding was checked against the cited sources.
Thanks,
Enzo
^ permalink raw reply
* [PATCH 17/28] KVM: arm64: Add NV3 control bits to HCRX_EL2 sanitisation
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
Expose the FEAT_NV3 control bits to the sanitisation code so that
KVM stops moaning about the unattributed bits.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/config.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
index 8d5e4aacf49c4..b9a9d65b973e6 100644
--- a/arch/arm64/kvm/config.c
+++ b/arch/arm64/kvm/config.c
@@ -225,6 +225,7 @@ struct reg_feat_map_desc {
#define FEAT_HCX ID_AA64MMFR1_EL1, HCX, IMP
#define FEAT_S2PIE ID_AA64MMFR3_EL1, S2PIE, IMP
#define FEAT_GCIE ID_AA64PFR2_EL1, GCIE, IMP
+#define FEAT_NV3 ID_AA64MMFR4_EL1, NV_frac, NV3
static bool not_feat_aa64el3(struct kvm *kvm)
{
@@ -904,6 +905,11 @@ static const DECLARE_FEAT_MAP_FGT(hdfgwtr2_desc, hdfgwtr2_masks,
static const struct reg_bits_to_feat_map hcrx_feat_map[] = {
+ NEEDS_FEAT(HCRX_EL2_NVTGE |
+ HCRX_EL2_NVnTTLB |
+ HCRX_EL2_NVnTTLBIS |
+ HCRX_EL2_NVnTTLBOS,
+ FEAT_NV3),
NEEDS_FEAT(HCRX_EL2_SRMASKEn, FEAT_SRMASK),
NEEDS_FEAT(HCRX_EL2_PACMEn, feat_pauth_lr),
NEEDS_FEAT(HCRX_EL2_EnFPM, FEAT_FPMR),
--
2.47.3
^ permalink raw reply related
* [PATCH v8 14/39] drm/display: bridge_connector: Use cached connector status in .get_modes()
From: Cristian Ciocaltea @ 2026-07-02 14:46 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Sandy Huang, Heiko Stübner, Andy Yan, Daniel Stone,
Dave Stevenson, Maíra Canal, Raspberry Pi Kernel Maintenance
Cc: kernel, dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip,
Dmitry Baryshkov
In-Reply-To: <20260702-dw-hdmi-qp-scramb-v8-0-d79890d00b6a@collabora.com>
Replace the active drm_bridge_connector_detect() call in get_modes()
with a read of the already-cached connector->status.
The .get_modes() callback is only invoked from
drm_helper_probe_single_connector_modes(), which has already retrieved
the connector status. Calling detect again is redundant and triggers a
duplicate hotplug event. This is also a prerequisite for switching to
the .detect_ctx() hook, which requires a drm_modeset_acquire_ctx not
available in the .get_modes() path.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 46104506fe32..57238ba49d70 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -326,12 +326,10 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = {
static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector,
struct drm_bridge *bridge)
{
- enum drm_connector_status status;
const struct drm_edid *drm_edid;
int n;
- status = drm_bridge_connector_detect(connector, false);
- if (status != connector_status_connected)
+ if (connector->status != connector_status_connected)
goto no_edid;
drm_edid = drm_bridge_edid_read(bridge, connector);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v7 2/8] media: subdev: Add media_async_register_subdev() helper
From: Laurent Pinchart @ 2026-07-02 18:19 UTC (permalink / raw)
To: Frank Li
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch, Frank Li,
Martin Kepplinger-Novakovic, Rui Miguel Silva, Purism Kernel Team,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, Guoniu Zhou, devicetree, linux-arm-kernel
In-Reply-To: <akapt6v_DBJSAnj0@SMW015318>
On Thu, Jul 02, 2026 at 01:11:03PM -0500, Frank Li wrote:
> On Thu, Jul 02, 2026 at 07:15:19PM +0300, Laurent Pinchart wrote:
> > Hi Frank,
> >
> > Have you missed the comment in v6 ?
> >
> > https://lore.kernel.org/all/20260629084654.GB3054459@killaraus.ideasonboard.com/
>
> Sorry, I forget emphased it in change log.
>
> "For the reason stated by Sakari in patch 1/9 (dependency from MC to
> V4L2), I don't think a "media_async_register_subdev()" function is a
> good idea."
>
> The reason already NOT existed in v7, I removed v4l2_fwnode_endpoint from
> media_pad. So MC will not depdent to V4L2.
>
> So this comments is not suit for this version.
media_async_register_subdev(), with its "media_" prefix, appears part of
the MC API, but it operates on a subdev. At least the function name
should be changed in my opinion.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v7 2/8] media: subdev: Add media_async_register_subdev() helper
From: Frank Li @ 2026-07-02 18:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch, Frank Li,
Martin Kepplinger-Novakovic, Rui Miguel Silva, Purism Kernel Team,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, Guoniu Zhou, devicetree, linux-arm-kernel
In-Reply-To: <20260702161519.GI3534761@killaraus.ideasonboard.com>
On Thu, Jul 02, 2026 at 07:15:19PM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> Have you missed the comment in v6 ?
>
> https://lore.kernel.org/all/20260629084654.GB3054459@killaraus.ideasonboard.com/
Sorry, I forget emphased it in change log.
"For the reason stated by Sakari in patch 1/9 (dependency from MC to
V4L2), I don't think a "media_async_register_subdev()" function is a
good idea."
The reason already NOT existed in v7, I removed v4l2_fwnode_endpoint from
media_pad. So MC will not depdent to V4L2.
So this comments is not suit for this version.
Frank
^ permalink raw reply
* [PATCH 08/28] arm64: Add ARM64_HAS_NV2P1 capability
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
As we're about to deal with FEAT_NV2P1, add a new capability that
will be used to key any support for it.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/tools/cpucaps | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 9b85a84f6fd49..242dc211d8efa 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -51,6 +51,7 @@ HAS_LS64_V
HAS_LSUI
HAS_MOPS
HAS_NESTED_VIRT
+HAS_NV2P1
HAS_BBML2_NOABORT
HAS_PAN
HAS_PMUV3
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] Revert "arm64: dts: rockchip: Further describe the WiFi for the Pinephone Pro"
From: Heiko Stuebner @ 2026-07-02 18:23 UTC (permalink / raw)
To: oklopfer37, Oren Klopfer
Cc: linux-arm-kernel, linux-rockchip, linux-kernel, Peter Robinson,
Thorsten Leemhuis, stable
In-Reply-To: <20260615225014.219115-1-oklopfer37@gmail.com>
Hi Oren,
Am Dienstag, 16. Juni 2026, 00:50:13 Mitteleuropäische Sommerzeit schrieb Oren Klopfer:
> This reverts commit 096bd8c679185f898cae9933c6a68650fa26ea4f.
>
> Just as with the Pinebook Pro, there are multiple chipset variants for the Pinephone Pro, and multiple firmware binaries for different distributions. The change causes issues with some of these combinations, and reverting it resolves the issues. See the Closes below for the full report.
>
> Similarly with the Pinebook Pro adjustment, the original commit only indicates "further description" and not indicative of fixing any existing issues, so reverting should not kick any back up.
The commit message did not get any line breaks, 72 character
is the preferred line length.
Also the patch contents itself got mangled.
All the indentation is set as spaces, so that patch won't apply.
Please try using b4 or at least git send-email for sending patches.
Thanks a lot
Heiko
> Fixes: 096bd8c67918 ("arm64: dts: rockchip: Further describe the WiFi for the Pinephone Pro")
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Peter Robinson <pbrobinson@gmail.com>
> Cc: Thorsten Leemhuis <regressions@leemhuis.info>
> Cc: stable@vger.kernel.org
> Closes: https://lore.kernel.org/r/20260607225901.64019-1-oklopfer37@gmail.com/
> Signed-off-by: Oren Klopfer <oklopfer37@gmail.com>
> ---
> .../boot/dts/rockchip/rk3399-pinephone-pro.dts | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 8d26bd9b7500..d46cdfe3f784 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -734,12 +734,6 @@ light_int_l: light-int-l {
> };
> };
>
> - wifi {
> - wifi_host_wake_l: wifi-host-wake-l {
> - rockchip,pins = <4 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
> - };
> - };
> -
> wireless-bluetooth {
> bt_wake_pin: bt-wake-pin {
> rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> @@ -766,19 +760,7 @@ &sdio0 {
> pinctrl-names = "default";
> pinctrl-0 = <&sdio0_bus4 &sdio0_cmd &sdio0_clk>;
> sd-uhs-sdr104;
> - #address-cells = <1>;
> - #size-cells = <0>;
> status = "okay";
> -
> - brcmf: wifi@1 {
> - compatible = "brcm,bcm4329-fmac";
> - reg = <1>;
> - interrupt-parent = <&gpio4>;
> - interrupts = <RK_PD0 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "host-wake";
> - pinctrl-names = "default";
> - pinctrl-0 = <&wifi_host_wake_l>;
> - };
> };
>
> &pwm0 {
>
^ permalink raw reply
* [PATCH 12/28] arm64: Add FEAT_NV2p1 detection
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
Add the necessary NV2p1 probing to the cpufeature infrastructure.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9a22df0c5120f..c9c124b0ccc8e 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2620,6 +2620,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
{ /* Sentinel */ }
},
},
+ {
+ .desc = "FEAT_NV2p1",
+ .capability = ARM64_HAS_NV2P1,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .matches = has_cpuid_feature,
+ ARM64_CPUID_FIELDS(ID_AA64MMFR4_EL1, NV_frac, NV2P1)
+ },
{
.capability = ARM64_HAS_32BIT_EL0_DO_NOT_USE,
.type = ARM64_CPUCAP_SYSTEM_FEATURE,
--
2.47.3
^ permalink raw reply related
* [PATCH 07/28] KVM: arm64: Don't evaluate HCR_EL2.NV on ERET fast path
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
We currently avoid using the ERET fast path if the guest has HCR_EL2.NV
set. This is an odd check, as NV doesn't mean much if HCR_EL2.TGE==1.
Replace this bizarre check with is_nested_ctxt() which makes a lot
more sense: if we are running an L2, the ERET trap must go to L1.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/hyp/vhe/switch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index bbe9cebd3d9d5..3b76e0468317b 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -349,8 +349,8 @@ static bool kvm_hyp_handle_eret(struct kvm_vcpu *vcpu, u64 *exit_code)
* Unless the trap has to be forwarded further down the line,
* of course...
*/
- if ((__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_NV) ||
- (__vcpu_sys_reg(vcpu, HFGITR_EL2) & HFGITR_EL2_ERET))
+ if (is_nested_ctxt(vcpu) ||
+ __vcpu_sys_reg(vcpu, HFGITR_EL2) & HFGITR_EL2_ERET)
return false;
spsr = read_sysreg_el1(SYS_SPSR);
--
2.47.3
^ permalink raw reply related
* [PATCH 13/28] arm64: sysreg: Add NVHCR_EL2 description as a mirror of HCR_EL2
From: Marc Zyngier @ 2026-07-02 16:02 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu
In-Reply-To: <20260702160248.1377250-1-maz@kernel.org>
FEAT_NV3 introduces a new register that contains the HCR_EL2 value
exposed to a NV guest. As such, it has the exact same layout as
HCR_EL2.
Describe NVHCR_EL2 as a mapping to HCR_EL2.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/tools/sysreg | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index c6e8117a6f9cd..31e4ea455a9ce 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -4266,6 +4266,9 @@ Field 1 E2TRE
Field 0 E0HTRE
EndSysreg
+Sysreg NVHCR_EL2 3 4 1 5 0
+Mapping HCR_EL2
+EndSysreg
Sysreg HDFGRTR2_EL2 3 4 3 1 0
Res0 63:25
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 1/8] dt-bindings: rtc: sun6i: no clock-output-names on h616/r329
From: Conor Dooley @ 2026-07-02 18:44 UTC (permalink / raw)
To: Jerome Brunet
Cc: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard, linux-rtc,
devicetree, linux-arm-kernel, linux-sunxi, linux-kernel,
linux-clk, Sashiko
In-Reply-To: <20260702-a733-rtc-v3-1-eb2580374de6@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/8] dt-bindings: rtc: sun6i: add sun60i-a733 support
From: Conor Dooley @ 2026-07-02 18:45 UTC (permalink / raw)
To: Jerome Brunet
Cc: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard, linux-rtc,
devicetree, linux-arm-kernel, linux-sunxi, linux-kernel,
linux-clk
In-Reply-To: <20260702-a733-rtc-v3-2-eb2580374de6@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox