* [RFC PATCH] arm_mpam: remove sanity check of accessibility when error interrupt is SPI on SMT platforms
From: Yadong Qi @ 2026-07-20 7:24 UTC (permalink / raw)
To: james.morse, ben.horgan, reinette.chatre, fenghuay, linux-kernel,
lpieralisi, guohanjun, sudeep.holla, catalin.marinas, will,
rafael, lenb, linux-acpi, linux-arm-kernel, ying.huang
Cc: ptg-linux-kernel, yadong.qi
On SMT platforms, an L2 cache MSC is typically shared between sibling
threads. Per the MPAM ACPI spec (DEN0065B), the MSC's linked device
should be set to the processor container of those siblings, so the
kernel derives msc->accessibility as just the sibling CPUs.
Per the MPAM spec (IHI0099B), when an MSC is not integrated into a PE,
SPI/LPI is the recommended error interrupt. SPIs are not per-CPU, and a
sanity check in mpam_msc_setup_error_irq() requires accessibility ==
cpu_possible_mask — assuming a shared interrupt must be routable to all
CPUs. This creates a conflict on SMT platforms: the MSC has a restricted
accessibility but a shared error interrupt, causing probe to fail with:
msc:N is a private resource with a shared error interrupt
This RFC patch removes the check to allow MSC probe to succeed on SMT
platforms, but this is mainly intended to start a discussion. We would
appreciate advice on the right way to handle this conflict. Is this a
spec issue, or should the driver handle SPI error interrupts differently
when an MSC has a restricted accessibility mask?
Signed-off-by: Yadong Qi <yadong.qi@linux.alibaba.com>
---
drivers/resctrl/mpam_devices.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index b69f99488111..e8b2b5e00d7c 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1964,13 +1964,6 @@ static int mpam_msc_setup_error_irq(struct mpam_msc *msc)
if (irq_is_percpu(irq))
return __setup_ppi(msc);
- /* sanity check: shared interrupts can be routed anywhere? */
- if (!cpumask_equal(&msc->accessibility, cpu_possible_mask)) {
- pr_err_once("msc:%u is a private resource with a shared error interrupt",
- msc->id);
- return -EINVAL;
- }
-
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v8 14/22] RISC-V: perf: Implement supervisor counter delegation support
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-14-7909f863a645@meta.com>
# Add your code comments below. There is no need to trim or delete
# any existing content -- just insert your comments under the relevant
# lines of code. Lines starting with "> " are quoted diff context and
# lines starting with "| " are comments from other reviewers.
# The final email will be reformatted automatically to include only
# the sections that have your comments.
#
> There are few new RISC-V ISA exensions (ssccfg, sscsrind, smcntrpmf) which
> allows the hpmcounter/hpmevents to be programmed directly from S-mode. The
> implementation detects the ISA extension at runtime and uses them if
> available instead of SBI PMU extension. SBI PMU extension will still be
> used for firmware counters if the user requests it.
>
> The current linux driver relies on event encoding defined by SBI PMU
> specification for standard perf events. However, there are no standard
> event encoding available in the ISA. In the future, we may want to
> decouple the counter delegation and SBI PMU completely. In that case,
> counter delegation supported platforms must rely on the event encoding
> defined in the perf json file or in the pmu driver.
>
> For firmware events, it will continue to use the SBI PMU encoding as
> one can not support firmware event without SBI PMU.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index a3b24b88e401..cd22b5168689 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -258,6 +258,7 @@
> #endif
>
> #define SISELECT_SSCCFG_BASE 0x40
> +#define HPMEVENT_MASK GENMASK_ULL(63, 56)
>
> /* mseccfg bits */
> #define MSECCFG_PMM ENVCFG_PMM
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 2568c6808f5d..7995da4a98a1 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -28,6 +28,8 @@
> #include <asm/cpufeature.h>
> #include <asm/vendor_extensions.h>
> #include <asm/vendor_extensions/andes.h>
> +#include <asm/hwcap.h>
> +#include <asm/csr_ind.h>
>
> #define ALT_SBI_PMU_OVERFLOW(__ovl) \
> asm volatile(ALTERNATIVE_2( \
> @@ -60,7 +62,20 @@ asm volatile(ALTERNATIVE( \
> #define PERF_EVENT_FLAG_USER_ACCESS BIT(SYSCTL_USER_ACCESS)
> #define PERF_EVENT_FLAG_LEGACY BIT(SYSCTL_LEGACY)
>
> -PMU_FORMAT_ATTR(event, "config:0-55");
> +#define RVPMU_SBI_PMU_FORMAT_ATTR "config:0-47"
> +#define RVPMU_CDELEG_PMU_FORMAT_ATTR "config:0-55"
> +
> +static ssize_t __maybe_unused rvpmu_format_show(struct device *dev, struct device_attribute *attr,
> + char *buf);
> +
> +#define RVPMU_ATTR_ENTRY(_name, _func, _config) ( \
> + &((struct dev_ext_attribute[]) { \
> + { __ATTR(_name, 0444, _func, NULL), (void *)_config } \
> + })[0].attr.attr)
> +
> +#define RVPMU_FORMAT_ATTR_ENTRY(_name, _config) \
> + RVPMU_ATTR_ENTRY(_name, rvpmu_format_show, (char *)_config)
> +
> PMU_FORMAT_ATTR(firmware, "config:62-63");
>
> static bool sbi_v2_available;
> @@ -68,7 +83,11 @@ static bool sbi_v3_available;
> static DEFINE_STATIC_KEY_FALSE(sbi_pmu_snapshot_available);
> #define sbi_pmu_snapshot_available() \
> static_branch_unlikely(&sbi_pmu_snapshot_available)
> +
> static DEFINE_STATIC_KEY_FALSE(riscv_pmu_sbi_available);
> +#define riscv_pmu_sbi_available() \
> + static_branch_likely(&riscv_pmu_sbi_available)
> +
> static DEFINE_STATIC_KEY_FALSE(riscv_pmu_cdeleg_available);
>
> /* Avoid unnecessary code patching in the one time booting path*/
> @@ -83,19 +102,35 @@ static DEFINE_STATIC_KEY_FALSE(riscv_pmu_cdeleg_available);
> #define riscv_pmu_sbi_available() \
> static_branch_likely(&riscv_pmu_sbi_available)
>
> -static struct attribute *riscv_arch_formats_attr[] = {
> - &format_attr_event.attr,
> +static struct attribute *riscv_sbi_pmu_formats_attr[] = {
> + RVPMU_FORMAT_ATTR_ENTRY(event, RVPMU_SBI_PMU_FORMAT_ATTR),
> &format_attr_firmware.attr,
> NULL,
> };
>
> -static struct attribute_group riscv_pmu_format_group = {
> +static struct attribute_group riscv_sbi_pmu_format_group = {
> .name = "format",
> - .attrs = riscv_arch_formats_attr,
> + .attrs = riscv_sbi_pmu_formats_attr,
> };
>
> -static const struct attribute_group *riscv_pmu_attr_groups[] = {
> - &riscv_pmu_format_group,
> +static const struct attribute_group *riscv_sbi_pmu_attr_groups[] = {
> + &riscv_sbi_pmu_format_group,
> + NULL,
> +};
> +
> +static struct attribute *riscv_cdeleg_pmu_formats_attr[] = {
> + RVPMU_FORMAT_ATTR_ENTRY(event, RVPMU_CDELEG_PMU_FORMAT_ATTR),
> + &format_attr_firmware.attr,
> + NULL,
> +};
> +
> +static struct attribute_group riscv_cdeleg_pmu_format_group = {
> + .name = "format",
> + .attrs = riscv_cdeleg_pmu_formats_attr,
> +};
> +
> +static const struct attribute_group *riscv_cdeleg_pmu_attr_groups[] = {
> + &riscv_cdeleg_pmu_format_group,
> NULL,
> };
>
> @@ -482,6 +517,14 @@ static void rvpmu_sbi_check_std_events(struct work_struct *work)
>
> static DECLARE_WORK(check_std_events_work, rvpmu_sbi_check_std_events);
>
> +static ssize_t rvpmu_format_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dev_ext_attribute *eattr = container_of(attr,
> + struct dev_ext_attribute, attr);
> + return sysfs_emit(buf, "%s\n", (char *)eattr->var);
> +}
> +
> static int rvpmu_ctr_get_width(int idx)
> {
> return pmu_ctr_list[idx].width;
> @@ -599,6 +642,38 @@ static uint8_t rvpmu_csr_index(struct perf_event *event)
> return pmu_ctr_list[event->hw.idx].csr - CSR_CYCLE;
> }
>
> +static uint64_t get_deleg_priv_filter_bits(struct perf_event *event)
> +{
> + u64 priv_filter_bits = 0;
> + bool guest_events = false;
> +
> + if (event->attr.config1 & RISCV_PMU_CONFIG1_GUEST_EVENTS)
> + guest_events = true;
> + if (event->attr.exclude_kernel)
> + priv_filter_bits |= guest_events ? HPMEVENT_VSINH : HPMEVENT_SINH;
> + if (event->attr.exclude_user)
> + priv_filter_bits |= guest_events ? HPMEVENT_VUINH : HPMEVENT_UINH;
> + if (guest_events && event->attr.exclude_hv)
> + priv_filter_bits |= HPMEVENT_SINH;
> + if (event->attr.exclude_host)
> + priv_filter_bits |= HPMEVENT_UINH | HPMEVENT_SINH;
> + if (event->attr.exclude_guest)
> + priv_filter_bits |= HPMEVENT_VSINH | HPMEVENT_VUINH;
> +
> + return priv_filter_bits;
> +}
> +
> +static bool pmu_sbi_is_fw_event(struct perf_event *event)
> +{
> + u32 type = event->attr.type;
> + u64 config = event->attr.config;
> +
> + if (type == PERF_TYPE_RAW && ((config >> 63) == 1))
> + return true;
> + else
> + return false;
> +}
> +
> static unsigned long rvpmu_sbi_get_filter_flags(struct perf_event *event)
> {
> unsigned long cflags = 0;
> @@ -627,7 +702,8 @@ static int rvpmu_sbi_ctr_get_idx(struct perf_event *event)
> struct cpu_hw_events *cpuc = this_cpu_ptr(rvpmu->hw_events);
> struct sbiret ret;
> int idx;
> - uint64_t cbase = 0, cmask = rvpmu->cmask;
> + u64 cbase = 0;
> + unsigned long ctr_mask = rvpmu->cmask;
> unsigned long cflags = 0;
>
> cflags = rvpmu_sbi_get_filter_flags(event);
> @@ -640,21 +716,23 @@ static int rvpmu_sbi_ctr_get_idx(struct perf_event *event)
> if ((hwc->flags & PERF_EVENT_FLAG_LEGACY) && (event->attr.type == PERF_TYPE_HARDWARE)) {
> if (event->attr.config == PERF_COUNT_HW_CPU_CYCLES) {
> cflags |= SBI_PMU_CFG_FLAG_SKIP_MATCH;
> - cmask = 1;
> + ctr_mask = 1;
> } else if (event->attr.config == PERF_COUNT_HW_INSTRUCTIONS) {
> cflags |= SBI_PMU_CFG_FLAG_SKIP_MATCH;
> - cmask = BIT(CSR_INSTRET - CSR_CYCLE);
> + ctr_mask = BIT(CSR_INSTRET - CSR_CYCLE);
> }
> + } else if (pmu_sbi_is_fw_event(event)) {
> + ctr_mask = firmware_cmask;
> }
>
> /* retrieve the available counter index */
> #if defined(CONFIG_32BIT)
> ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
> - cmask, cflags, hwc->event_base, hwc->config,
> + ctr_mask, cflags, hwc->event_base, hwc->config,
> hwc->config >> 32);
> #else
> ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
> - cmask, cflags, hwc->event_base, hwc->config, 0);
> + ctr_mask, cflags, hwc->event_base, hwc->config, 0);
> #endif
> if (ret.error) {
> pr_debug("Not able to find a counter for event %lx config %llx\n",
> @@ -663,7 +741,7 @@ static int rvpmu_sbi_ctr_get_idx(struct perf_event *event)
> }
>
> idx = ret.value;
> - if (!test_bit(idx, &rvpmu->cmask) || !pmu_ctr_list[idx].value)
> + if (!test_bit(idx, &ctr_mask) || !pmu_ctr_list[idx].value)
> return -ENOENT;
>
> /* Additional sanity check for the counter id */
> @@ -713,29 +791,98 @@ static int sbi_pmu_event_find_cache(u64 config)
> return ret;
> }
>
> -static bool pmu_sbi_is_fw_event(struct perf_event *event)
> +static int rvpmu_sbi_event_map(struct perf_event *event, u64 *econfig)
> {
> u32 type = event->attr.type;
> u64 config = event->attr.config;
>
> - if ((type == PERF_TYPE_RAW) && ((config >> 63) == 1))
> - return true;
> - else
> - return false;
> + /*
> + * Ensure we are finished checking standard hardware events for
> + * validity before allowing userspace to configure any events.
> + */
> + flush_work(&check_std_events_work);
> +
> + return riscv_pmu_get_event_info(type, config, econfig);
> }
>
> -static int rvpmu_sbi_event_map(struct perf_event *event, u64 *econfig)
> +static int cdeleg_pmu_event_find_cache(u64 config, u64 *eventid, uint32_t *counter_mask)
> +{
> + unsigned int cache_type, cache_op, cache_result;
> +
> + if (!current_pmu_cache_event_map)
> + return -ENOENT;
> +
> + cache_type = (config >> 0) & 0xff;
> + if (cache_type >= PERF_COUNT_HW_CACHE_MAX)
> + return -EINVAL;
> +
> + cache_op = (config >> 8) & 0xff;
> + if (cache_op >= PERF_COUNT_HW_CACHE_OP_MAX)
> + return -EINVAL;
> +
> + cache_result = (config >> 16) & 0xff;
> + if (cache_result >= PERF_COUNT_HW_CACHE_RESULT_MAX)
> + return -EINVAL;
> +
> + if (eventid)
> + *eventid = current_pmu_cache_event_map[cache_type][cache_op]
> + [cache_result].event_id;
> + if (counter_mask)
> + *counter_mask = current_pmu_cache_event_map[cache_type][cache_op]
> + [cache_result].counter_mask;
> +
> + return 0;
> +}
> +
> +static int rvpmu_cdeleg_event_map(struct perf_event *event, u64 *econfig)
> {
> u32 type = event->attr.type;
> u64 config = event->attr.config;
> + int ret = 0;
>
> /*
> - * Ensure we are finished checking standard hardware events for
> - * validity before allowing userspace to configure any events.
> + * There are two ways standard perf events can be mapped to platform specific
> + * encoding.
> + * 1. The vendor may specify the encodings in the driver.
> + * 2. The Perf tool for RISC-V may remap the standard perf event to platform
> + * specific encoding.
> + *
> + * As RISC-V ISA doesn't define any standard event encoding. Thus, perf tool allows
> + * vendor to define it via json file. The encoding defined in the json will override
> + * the perf legacy encoding. However, some user may want to run performance
> + * monitoring without perf tool as well. That's why, vendors may specify the event
> + * encoding in the driver as well if they want to support that use case too.
> + * If an encoding is defined in the json, it will be encoded as a raw event.
> */
> - flush_work(&check_std_events_work);
>
> - return riscv_pmu_get_event_info(type, config, econfig);
> + switch (type) {
> + case PERF_TYPE_HARDWARE:
> + if (config >= PERF_COUNT_HW_MAX)
> + return -EINVAL;
> + if (!current_pmu_hw_event_map)
> + return -ENOENT;
> +
> + *econfig = current_pmu_hw_event_map[config].event_id;
> + if (*econfig == HW_OP_UNSUPPORTED)
> + ret = -ENOENT;
> + break;
> + case PERF_TYPE_HW_CACHE:
> + ret = cdeleg_pmu_event_find_cache(config, econfig, NULL);
> + if (ret)
> + break;
> + if (*econfig == CACHE_OP_UNSUPPORTED)
> + ret = -ENOENT;
> + break;
> + case PERF_TYPE_RAW:
> + *econfig = config & RISCV_PMU_DELEG_RAW_EVENT_MASK;
> + break;
> + default:
> + ret = -ENOENT;
> + break;
> + }
> +
> + /* event_base is not used for counter delegation */
> + return ret;
> }
>
> static void pmu_sbi_snapshot_free(struct riscv_pmu *pmu)
> @@ -821,7 +968,7 @@ static int pmu_sbi_snapshot_setup(struct riscv_pmu *pmu, int cpu)
> return 0;
> }
>
> -static u64 rvpmu_sbi_ctr_read(struct perf_event *event)
> +static u64 rvpmu_ctr_read(struct perf_event *event)
> {
> struct hw_perf_event *hwc = &event->hw;
> int idx = hwc->idx;
> @@ -898,10 +1045,6 @@ static void rvpmu_sbi_ctr_start(struct perf_event *event, u64 ival)
> if (ret.error && (ret.error != SBI_ERR_ALREADY_STARTED))
> pr_err("Starting counter idx %d failed with error %d\n",
> hwc->idx, sbi_err_map_linux_errno(ret.error));
> -
> - if ((hwc->flags & PERF_EVENT_FLAG_USER_ACCESS) &&
> - (hwc->flags & PERF_EVENT_FLAG_USER_READ_CNT))
> - rvpmu_set_scounteren((void *)event);
> }
>
> static void rvpmu_sbi_ctr_stop(struct perf_event *event, unsigned long flag)
> @@ -912,10 +1055,6 @@ static void rvpmu_sbi_ctr_stop(struct perf_event *event, unsigned long flag)
> struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
> struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
>
> - if ((hwc->flags & PERF_EVENT_FLAG_USER_ACCESS) &&
> - (hwc->flags & PERF_EVENT_FLAG_USER_READ_CNT))
> - rvpmu_reset_scounteren((void *)event);
> -
> if (sbi_pmu_snapshot_available())
> flag |= SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT;
>
> @@ -951,12 +1090,6 @@ static int rvpmu_sbi_find_num_ctrs(void)
> return sbi_err_map_linux_errno(ret.error);
> }
>
> -static u32 rvpmu_deleg_find_ctrs(void)
> -{
> - /* TODO */
> - return 0;
> -}
> -
> static int rvpmu_sbi_get_ctrinfo(u32 nsbi_ctr, u32 *num_fw_ctr, u32 *num_hw_ctr)
> {
> struct sbiret ret;
> @@ -1034,55 +1167,75 @@ static inline void rvpmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> }
> }
>
> -/*
> - * This function starts all the used counters in two step approach.
> - * Any counter that did not overflow can be start in a single step
> - * while the overflowed counters need to be started with updated initialization
> - * value.
> - */
> -static inline void rvpmu_sbi_start_ovf_ctrs_sbi(struct cpu_hw_events *cpu_hw_evt,
> - u64 ctr_ovf_mask)
> +static void rvpmu_deleg_ctr_start_mask(unsigned long mask)
> {
> - int idx = 0, i;
> - struct perf_event *event;
> - unsigned long flag = SBI_PMU_START_FLAG_SET_INIT_VALUE;
> - unsigned long ctr_start_mask = 0;
> - uint64_t max_period;
> - struct hw_perf_event *hwc;
> - u64 init_val = 0;
> + unsigned long scountinhibit_val = 0;
>
> - for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
> - ctr_start_mask = cpu_hw_evt->used_hw_ctrs[i] & ~ctr_ovf_mask;
> - /* Start all the counters that did not overflow in a single shot */
> - if (ctr_start_mask) {
> - sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, i * BITS_PER_LONG,
> - ctr_start_mask, 0, 0, 0, 0);
> - }
> - }
> + scountinhibit_val = csr_read(CSR_SCOUNTINHIBIT);
> + scountinhibit_val &= ~mask;
> +
> + csr_write(CSR_SCOUNTINHIBIT, scountinhibit_val);
> +}
> +
> +static void rvpmu_deleg_ctr_enable_irq(struct perf_event *event)
> +{
> + unsigned long hpmevent_curr;
> + unsigned long of_mask;
> + struct hw_perf_event *hwc = &event->hw;
> + int counter_idx = hwc->idx;
> + unsigned long sip_val = csr_read(CSR_SIP);
> +
> + if (!is_sampling_event(event) || (sip_val & SIP_LCOFIP))
> + return;
>
> - /* Reinitialize and start all the counter that overflowed */
> - while (ctr_ovf_mask) {
> - if (ctr_ovf_mask & 0x01) {
> - event = cpu_hw_evt->events[idx];
> - hwc = &event->hw;
> - max_period = riscv_pmu_ctr_get_width_mask(event);
> - init_val = local64_read(&hwc->prev_count) & max_period;
> #if defined(CONFIG_32BIT)
> - sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx, 1,
> - flag, init_val, init_val >> 32, 0);
> + hpmevent_curr = csr_ind_read(CSR_SIREG5, SISELECT_SSCCFG_BASE, counter_idx);
> + of_mask = (u32)~HPMEVENTH_OF;
> #else
> - sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx, 1,
> - flag, init_val, 0, 0);
> + hpmevent_curr = csr_ind_read(CSR_SIREG2, SISELECT_SSCCFG_BASE, counter_idx);
> + of_mask = ~HPMEVENT_OF;
> #endif
> - perf_event_update_userpage(event);
> - }
> - ctr_ovf_mask = ctr_ovf_mask >> 1;
> - idx++;
> - }
> +
> + hpmevent_curr &= of_mask;
> +#if defined(CONFIG_32BIT)
> + csr_ind_write(CSR_SIREG5, SISELECT_SSCCFG_BASE, counter_idx, hpmevent_curr);
> +#else
> + csr_ind_write(CSR_SIREG2, SISELECT_SSCCFG_BASE, counter_idx, hpmevent_curr);
> +#endif
> +}
> +
> +static void rvpmu_deleg_ctr_start(struct perf_event *event, u64 ival)
> +{
> + unsigned long scountinhibit_val = 0;
> + struct hw_perf_event *hwc = &event->hw;
> +
> +#if defined(CONFIG_32BIT)
> + csr_ind_write(CSR_SIREG, SISELECT_SSCCFG_BASE, hwc->idx, ival & 0xFFFFFFFF);
> + csr_ind_write(CSR_SIREG4, SISELECT_SSCCFG_BASE, hwc->idx, ival >> BITS_PER_LONG);
> +#else
> + csr_ind_write(CSR_SIREG, SISELECT_SSCCFG_BASE, hwc->idx, ival);
> +#endif
> +
> + rvpmu_deleg_ctr_enable_irq(event);
> +
> + scountinhibit_val = csr_read(CSR_SCOUNTINHIBIT);
> + scountinhibit_val &= ~BIT(hwc->idx);
> +
> + csr_write(CSR_SCOUNTINHIBIT, scountinhibit_val);
> +}
> +
> +static void rvpmu_deleg_ctr_stop_mask(unsigned long mask)
> +{
> + unsigned long scountinhibit_val = 0;
> +
> + scountinhibit_val = csr_read(CSR_SCOUNTINHIBIT);
> + scountinhibit_val |= mask;
> +
> + csr_write(CSR_SCOUNTINHIBIT, scountinhibit_val);
> }
>
> -static inline void rvpmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_evt,
> - u64 ctr_ovf_mask)
> +static void rvpmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_evt,
> + u64 ctr_ovf_mask)
> {
> int i, idx = 0;
> struct perf_event *event;
> @@ -1116,15 +1269,53 @@ static inline void rvpmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_h
> }
> }
>
> -static void rvpmu_sbi_start_overflow_mask(struct riscv_pmu *pmu,
> - u64 ctr_ovf_mask)
> +/*
> + * This function starts all the used counters in two step approach.
> + * Any counter that did not overflow can be start in a single step
> + * while the overflowed counters need to be started with updated initialization
> + * value.
> + */
> +static void rvpmu_start_overflow_mask(struct riscv_pmu *pmu, u64 ctr_ovf_mask)
> {
> + int idx = 0, i;
> + struct perf_event *event;
> + unsigned long ctr_start_mask = 0;
> + u64 max_period, init_val = 0;
> + struct hw_perf_event *hwc;
> struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
>
> if (sbi_pmu_snapshot_available())
> - rvpmu_sbi_start_ovf_ctrs_snapshot(cpu_hw_evt, ctr_ovf_mask);
> - else
> - rvpmu_sbi_start_ovf_ctrs_sbi(cpu_hw_evt, ctr_ovf_mask);
> + return rvpmu_sbi_start_ovf_ctrs_snapshot(cpu_hw_evt, ctr_ovf_mask);
> +
> + /* Start all the counters that did not overflow */
> + if (riscv_pmu_cdeleg_available()) {
> + ctr_start_mask = cpu_hw_evt->used_hw_ctrs[0] & ~ctr_ovf_mask;
> + rvpmu_deleg_ctr_start_mask(ctr_start_mask);
> + } else {
> + for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
> + ctr_start_mask = cpu_hw_evt->used_hw_ctrs[i] & ~ctr_ovf_mask;
> + /* Start all the counters that did not overflow in a single shot */
> + sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, i * BITS_PER_LONG,
> + ctr_start_mask, 0, 0, 0, 0);
> + }
> + }
> +
> + /* Reinitialize and start all the counter that overflowed */
> + while (ctr_ovf_mask) {
> + if (ctr_ovf_mask & 0x01) {
> + event = cpu_hw_evt->events[idx];
> + hwc = &event->hw;
> + max_period = riscv_pmu_ctr_get_width_mask(event);
> + init_val = local64_read(&hwc->prev_count) & max_period;
> + if (riscv_pmu_cdeleg_available())
> + rvpmu_deleg_ctr_start(event, init_val);
> + else
> + rvpmu_sbi_ctr_start(event, init_val);
> + perf_event_update_userpage(event);
> + }
> + ctr_ovf_mask = ctr_ovf_mask >> 1;
> + idx++;
> + }
> }
>
> static irqreturn_t rvpmu_ovf_handler(int irq, void *dev)
> @@ -1159,10 +1350,18 @@ static irqreturn_t rvpmu_ovf_handler(int irq, void *dev)
> }
>
> pmu = to_riscv_pmu(event->pmu);
> - rvpmu_sbi_stop_hw_ctrs(pmu);
> + if (riscv_pmu_cdeleg_available())
> + rvpmu_deleg_ctr_stop_mask(cpu_hw_evt->used_hw_ctrs[0]);
> + else
> + rvpmu_sbi_stop_hw_ctrs(pmu);
>
> - /* Overflow status register should only be read after counter are stopped */
> - if (sbi_pmu_snapshot_available())
> + /*
> + * Overflow status register should only be read after counter are stopped.
> + * In counter delegation mode the overflows are reported in scountovf, not
> + * in the SBI snapshot area, so read the CSR directly even when an SBI PMU
> + * snapshot is also available.
> + */
> + if (sbi_pmu_snapshot_available() && !riscv_pmu_cdeleg_available())
> overflow = sdata->ctr_overflow_mask;
> else
> ALT_SBI_PMU_OVERFLOW(overflow);
> @@ -1228,22 +1427,183 @@ static irqreturn_t rvpmu_ovf_handler(int irq, void *dev)
> hw_evt->state = 0;
> }
>
> - rvpmu_sbi_start_overflow_mask(pmu, overflowed_ctrs);
> + rvpmu_start_overflow_mask(pmu, overflowed_ctrs);
> perf_sample_event_took(sched_clock() - start_clock);
>
> return IRQ_HANDLED;
> }
>
> +static int get_deleg_hw_ctr_width(int counter_offset)
> +{
> + unsigned long hpm_warl;
> + int num_bits;
> +
> + if (counter_offset < 3 || counter_offset > 31)
> + return 0;
> +
> + hpm_warl = csr_ind_warl(CSR_SIREG, SISELECT_SSCCFG_BASE, counter_offset, -1);
> + if (!hpm_warl)
> + return 0;
> + num_bits = __fls(hpm_warl);
> +
> +#if defined(CONFIG_32BIT)
> + /*
> + * The low half contributes a full BITS_PER_LONG bits when the counter is
> + * wider than 32 bits; the high half's __fls() gives the remaining width.
> + */
> + hpm_warl = csr_ind_warl(CSR_SIREG4, SISELECT_SSCCFG_BASE, counter_offset, -1);
> + if (hpm_warl)
> + num_bits = BITS_PER_LONG + __fls(hpm_warl);
> +#endif
> + return num_bits;
> +}
> +
> +static int rvpmu_deleg_find_ctrs(void)
> +{
> + int i, num_hw_ctr = 0;
> + union sbi_pmu_ctr_info cinfo;
> + unsigned long scountinhibit_old = 0;
> +
> + /* Do a WARL write/read to detect which hpmcounters have been delegated */
> + scountinhibit_old = csr_read(CSR_SCOUNTINHIBIT);
> + csr_write(CSR_SCOUNTINHIBIT, -1);
> + cmask = csr_read(CSR_SCOUNTINHIBIT);
> +
> + csr_write(CSR_SCOUNTINHIBIT, scountinhibit_old);
> +
> + for_each_set_bit(i, &cmask, RISCV_MAX_HW_COUNTERS) {
> + if (unlikely(i == 1))
> + continue; /* This should never happen as TM is read only */
> + cinfo.value = 0;
> + cinfo.type = SBI_PMU_CTR_TYPE_HW;
> + /*
> + * If counter delegation is enabled, the csr stored to the cinfo will
> + * be a virtual counter that the delegation attempts to read.
> + */
> + cinfo.csr = CSR_CYCLE + i;
> + if (i == 0 || i == 2)
> + cinfo.width = 63;
> + else
> + cinfo.width = get_deleg_hw_ctr_width(i);
> +
> + num_hw_ctr++;
> + pmu_ctr_list[i].value = cinfo.value;
> + }
> +
> + return num_hw_ctr;
> +}
> +
> +static int get_deleg_fixed_hw_idx(struct cpu_hw_events *cpuc, struct perf_event *event)
> +{
> + return -EINVAL;
> +}
> +
> +static int get_deleg_next_hpm_hw_idx(struct cpu_hw_events *cpuc, struct perf_event *event)
> +{
> + unsigned long hw_ctr_mask = 0;
> +
> + /*
> + * TODO: Treat every hpmcounter can monitor every event for now.
> + * The event to counter mapping should come from the json file.
> + * The mapping should also tell if sampling is supported or not.
> + */
> +
> + /* Select only hpmcounters */
> + hw_ctr_mask = cmask & (~0x7);
> + hw_ctr_mask &= ~(cpuc->used_hw_ctrs[0]);
> + return __ffs(hw_ctr_mask);
> +}
> +
> +static void update_deleg_hpmevent(int counter_idx, uint64_t event_value, uint64_t filter_bits)
> +{
> + u64 hpmevent_value = 0;
> +
> + /* OF bit should be enable during the start if sampling is requested */
> + hpmevent_value = (event_value & ~HPMEVENT_MASK) | filter_bits | HPMEVENT_OF;
> +#if defined(CONFIG_32BIT)
> + csr_ind_write(CSR_SIREG2, SISELECT_SSCCFG_BASE, counter_idx, hpmevent_value & 0xFFFFFFFF);
> + if (riscv_isa_extension_available(NULL, SSCOFPMF))
> + csr_ind_write(CSR_SIREG5, SISELECT_SSCCFG_BASE, counter_idx,
> + hpmevent_value >> BITS_PER_LONG);
> +#else
> + csr_ind_write(CSR_SIREG2, SISELECT_SSCCFG_BASE, counter_idx, hpmevent_value);
> +#endif
> +}
> +
> +static int rvpmu_deleg_ctr_get_idx(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct riscv_pmu *rvpmu = to_riscv_pmu(event->pmu);
> + struct cpu_hw_events *cpuc = this_cpu_ptr(rvpmu->hw_events);
> + unsigned long hw_ctr_max_id;
> + u64 priv_filter;
> + int idx;
> +
> + /*
> + * TODO: We should not rely on SBI Perf encoding to check if the event
> + * is a fixed one or not.
> + */
> + if (!is_sampling_event(event)) {
> + idx = get_deleg_fixed_hw_idx(cpuc, event);
> + if (idx == 0 || idx == 2) {
> + /* Priv mode filter bits are only available if smcntrpmf is present */
> + if (riscv_isa_extension_available(NULL, SMCNTRPMF))
> + goto found_idx;
> + else
> + goto skip_update;
> + }
> + }
> +
> + if (!cmask)
> + goto out_err;
> + hw_ctr_max_id = __fls(cmask);
> + idx = get_deleg_next_hpm_hw_idx(cpuc, event);
> + if (idx < 3 || idx > hw_ctr_max_id)
> + goto out_err;
> +found_idx:
> + priv_filter = get_deleg_priv_filter_bits(event);
> + update_deleg_hpmevent(idx, hwc->config, priv_filter);
> +skip_update:
> + if (!test_and_set_bit(idx, cpuc->used_hw_ctrs))
> + return idx;
> +out_err:
> + return -ENOENT;
> +}
> +
> static void rvpmu_ctr_start(struct perf_event *event, u64 ival)
> {
> - rvpmu_sbi_ctr_start(event, ival);
> - /* TODO: Counter delegation implementation */
> + struct hw_perf_event *hwc = &event->hw;
> +
> + if (riscv_pmu_cdeleg_available() && !pmu_sbi_is_fw_event(event))
> + rvpmu_deleg_ctr_start(event, ival);
> + else
> + rvpmu_sbi_ctr_start(event, ival);
> +
> + if ((hwc->flags & PERF_EVENT_FLAG_USER_ACCESS) &&
> + (hwc->flags & PERF_EVENT_FLAG_USER_READ_CNT))
> + rvpmu_set_scounteren((void *)event);
> }
>
> static void rvpmu_ctr_stop(struct perf_event *event, unsigned long flag)
> {
> - rvpmu_sbi_ctr_stop(event, flag);
> - /* TODO: Counter delegation implementation */
> + struct hw_perf_event *hwc = &event->hw;
> +
> + if ((hwc->flags & PERF_EVENT_FLAG_USER_ACCESS) &&
> + (hwc->flags & PERF_EVENT_FLAG_USER_READ_CNT))
> + rvpmu_reset_scounteren((void *)event);
> +
> + if (riscv_pmu_cdeleg_available() && !pmu_sbi_is_fw_event(event)) {
> + /*
> + * The counter is already stopped. No need to stop again. Counter
> + * mapping will be reset in clear_idx function.
> + */
> + if (flag != RISCV_PMU_STOP_FLAG_RESET)
> + rvpmu_deleg_ctr_stop_mask(BIT(hwc->idx));
> + else
> + update_deleg_hpmevent(hwc->idx, 0, 0);
> + } else {
> + rvpmu_sbi_ctr_stop(event, flag);
> + }
> }
>
> static int rvpmu_find_ctrs(void)
> @@ -1292,20 +1652,18 @@ static int rvpmu_find_ctrs(void)
>
> static int rvpmu_event_map(struct perf_event *event, u64 *econfig)
> {
> - return rvpmu_sbi_event_map(event, econfig);
> - /* TODO: Counter delegation implementation */
> + if (riscv_pmu_cdeleg_available() && !pmu_sbi_is_fw_event(event))
> + return rvpmu_cdeleg_event_map(event, econfig);
> + else
> + return rvpmu_sbi_event_map(event, econfig);
> }
>
> static int rvpmu_ctr_get_idx(struct perf_event *event)
> {
> - return rvpmu_sbi_ctr_get_idx(event);
> - /* TODO: Counter delegation implementation */
> -}
> -
> -static u64 rvpmu_ctr_read(struct perf_event *event)
> -{
> - return rvpmu_sbi_ctr_read(event);
> - /* TODO: Counter delegation implementation */
> + if (riscv_pmu_cdeleg_available() && !pmu_sbi_is_fw_event(event))
> + return rvpmu_deleg_ctr_get_idx(event);
> + else
> + return rvpmu_sbi_ctr_get_idx(event);
> }
>
> static int rvpmu_starting_cpu(unsigned int cpu, struct hlist_node *node)
> @@ -1323,7 +1681,16 @@ static int rvpmu_starting_cpu(unsigned int cpu, struct hlist_node *node)
> csr_write(CSR_SCOUNTEREN, 0x2);
>
> /* Stop all the counters so that they can be enabled from perf */
> - rvpmu_sbi_stop_all(pmu);
> + if (riscv_pmu_cdeleg_available()) {
> + rvpmu_deleg_ctr_stop_mask(cmask);
> + if (riscv_pmu_sbi_available()) {
> + /* Stop the firmware counters as well */
> + sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP, 0, firmware_cmask,
> + 0, 0, 0, 0);
> + }
> + } else {
> + rvpmu_sbi_stop_all(pmu);
> + }
>
> if (riscv_pmu_use_irq) {
> cpu_hw_evt->irq = riscv_pmu_irq;
> @@ -1632,8 +1999,11 @@ static int rvpmu_device_probe(struct platform_device *pdev)
> }
> irq_requested = (ret == 0);
>
> - pmu->pmu.attr_groups = riscv_pmu_attr_groups;
> pmu->pmu.parent = &pdev->dev;
> + if (riscv_pmu_cdeleg_available_boot())
> + pmu->pmu.attr_groups = riscv_cdeleg_pmu_attr_groups;
> + else
> + pmu->pmu.attr_groups = riscv_sbi_pmu_attr_groups;
> pmu->cmask = cmask;
> pmu->ctr_start = rvpmu_ctr_start;
> pmu->ctr_stop = rvpmu_ctr_stop;
> diff --git a/include/linux/perf/riscv_pmu.h b/include/linux/perf/riscv_pmu.h
> index f82a28040594..3c64151cb038 100644
> --- a/include/linux/perf/riscv_pmu.h
> +++ b/include/linux/perf/riscv_pmu.h
> @@ -20,6 +20,7 @@
> */
>
> #define RISCV_MAX_COUNTERS 64
> +#define RISCV_MAX_HW_COUNTERS 32
> #define RISCV_OP_UNSUPP (-EOPNOTSUPP)
> #define RISCV_PMU_SBI_PDEV_NAME "riscv-pmu-sbi"
> #define RISCV_PMU_LEGACY_PDEV_NAME "riscv-pmu-legacy"
> @@ -28,6 +29,8 @@
>
> #define RISCV_PMU_CONFIG1_GUEST_EVENTS 0x1
>
> +#define RISCV_PMU_DELEG_RAW_EVENT_MASK GENMASK_ULL(55, 0)
> +
> struct cpu_hw_events {
> /* currently enabled events */
> int n_events;
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* RE: [PATCH v12 3/3] media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver
From: G.N. Zhou (OSS) @ 2026-07-20 7:32 UTC (permalink / raw)
To: Loic Poulain, G.N. Zhou (OSS)
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
Michael Turquette, Stephen Boyd, imx@lists.linux.dev,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
G.N. Zhou
In-Reply-To: <CAFEp6-3sxn8pPBLC4uNnyD5YSZ+Fn8Zg8Bcm52ituCH4y9QcWA@mail.gmail.com>
Hi Loic,
Thanks for the review.
> -----Original Message-----
> From: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Sent: Friday, July 17, 2026 4:19 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Laurent
> Pinchart <laurent.pinchart@ideasonboard.com>; Frank Li <frank.li@nxp.com>;
> Abel Vesa <abelvesa@kernel.org>; Peng Fan <peng.fan@nxp.com>; Michael
> Turquette <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>;
> imx@lists.linux.dev; linux-media@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> clk@vger.kernel.org; G.N. Zhou <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v12 3/3] media: nxp: Add i.MX95 CSI pixel formatter v4l2
> driver
>
> [You don't often get email from loic.poulain@oss.qualcomm.com. Learn why
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi Guoniu,
>
> On Thu, Jul 16, 2026 at 9:11 AM <guoniu.zhou@oss.nxp.com> wrote:
> >
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > The CSI pixel formatter is a module found on i.MX95 used to reformat
> > packet info, pixel and non-pixel data from CSI-2 host controller to
> > match Pixel Link(PL) definition.
> >
> > Add data formatting support.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> > Changes in v12:
> > - Fix stream ID handling: iterate routing table instead of assuming
> > stream ID equals loop index (0-7)
> > - Remove stream_to_vc[] array: derive VC from routing table and frame
> > descriptor on each start/stop operation
> > - Remove V4L2_SUBDEV_FL_HAS_EVENTS flag since driver does not generate
> > events
> > - Support stream IDs 0-63 by using BIT_ULL() for stream masks
> > - Add get_frame_desc call in stop_stream with proper error handling
> > - Add csi_formatter_read() helper function for register reads
> > - Use read-modify-write for CSI_VC_PIXEL_DATA_TYPE register to support
> > multiplexed streams sharing the same virtual channel
> > - Use route->sink_pad instead of hardcoded CSI_FORMATTER_PAD_SINK
> > - Write back coerced format in set_fmt before propagating to source
> > stream
> > - Drop Frank's Reviewed-by tag due to significant changes, requesting
> > re-review
> >
> > Changes in v10:
> > - Use u8 for vc in csi_formatter_get_vc() and drop vc < 0 check
> > - Add MFD_SYSCON dependency to Kconfig
> > - Fix stream/VC mapping potential mismatch in start/stop_stream
> > functions
> >
> > Changes in v8:
> > - Remove fmt field and look up format from subdev state instead
> > - Unify function and structure naming to use csi_formatter_ prefix
> > - Remove misleading alignment comment from set_fmt function
> > - Optimize get_frame_desc to call once per start_stream
> > - Replace V4L2_FRAME_DESC_ENTRY_MAX with CSI_FORMATTER_VC_NUM
> in loops
> > - Remove redundant debug message in enable_streams
> > - Use MEDIA_PAD_FL_MUST_CONNECT flag instead of manual link check
> > - Fix typo: Formater -> Formatter in Kconfig help text
> > - Improve grammar in data type index mapping comment
> >
> > Changes in v7:
> > - Update references from imx9 to imx95 for consistency with
> > dt-bindings
> > - Enable PM runtime before async registration
> >
> > Changes in v6:
> > - Remove unused header includes
> > - Unify macro naming: VCx/VCX -> VC and parameter x -> vc
> > - Remove unused format field from csi_formatter struct
> > - Use compact initialization for formats array
> > - Make find_csi_format() return NULL instead of default format
> > - Use unsigned int for array index in find_csi_format()
> > - Add err_ prefix to error handling labels
> > - Add v4l2_subdev_cleanup() and reorder cleanup sequence
> > - Update enable_streams debug output format
> > - Rename VC_MAX to VC_NUM and fix boundary check
> > - Update CSI formatter Kconfig description
> > - Use v4l2_subdev_get_frame_desc_passthrough() helper
> > - Fix error paths in async registration and probe
> > - Add mutex to protect enabled_streams
> > - Switch to devm_pm_runtime_enable()
> > - Remove redundant num_routes check in set_routing
> > - Optimize get_index_by_dt() and add warning for unsupported type
> > - csi_formatter_start/stop_stream: Process all streams in mask
> > ---
> > MAINTAINERS | 8 +
> > drivers/media/platform/nxp/Kconfig | 15 +
> > drivers/media/platform/nxp/Makefile | 1 +
> > drivers/media/platform/nxp/imx95-csi-formatter.c | 808
> > +++++++++++++++++++++++
> > 4 files changed, 832 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > efbf808063e5..05009228b162 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19275,6 +19275,14 @@ S: Maintained
> > F: Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > F: drivers/media/platform/nxp/imx-jpeg
> >
> > +NXP i.MX 95 CSI PIXEL FORMATTER V4L2 DRIVER
> > +M: Guoniu Zhou <guoniu.zhou@nxp.com>
> > +L: imx@lists.linux.dev
> > +L: linux-media@vger.kernel.org
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/media/fsl,imx95-csi-
> formatter.yaml
> > +F: drivers/media/platform/nxp/imx95-csi-formatter.c
> > +
> > NXP i.MX CLOCK DRIVERS
> > M: Abel Vesa <abelvesa@kernel.org>
> > R: Peng Fan <peng.fan@nxp.com>
> > diff --git a/drivers/media/platform/nxp/Kconfig
> > b/drivers/media/platform/nxp/Kconfig
> > index 40e3436669e2..8f49908b0022 100644
> > --- a/drivers/media/platform/nxp/Kconfig
> > +++ b/drivers/media/platform/nxp/Kconfig
> > @@ -28,6 +28,21 @@ config VIDEO_IMX8MQ_MIPI_CSI2
> > Video4Linux2 driver for the MIPI CSI-2 receiver found on the i.MX8MQ
> > SoC.
> >
> > +config VIDEO_IMX95_CSI_FORMATTER
> > + tristate "NXP i.MX95 CSI Pixel Formatter driver"
> > + depends on ARCH_MXC || COMPILE_TEST
> > + depends on MFD_SYSCON
> > + depends on VIDEO_DEV
> > + select MEDIA_CONTROLLER
> > + select V4L2_FWNODE
> > + select VIDEO_V4L2_SUBDEV_API
> > + help
> > + This driver provides support for the CSI Pixel Formatter found on
> > + i.MX95 series SoCs. This module unpacks the pixels received from the
> > + CSI-2 interface and reformats them to meet pixel link requirements.
> > +
> > + Say Y here to enable CSI Pixel Formatter module for i.MX95 SoC.
> > +
> > config VIDEO_IMX_MIPI_CSIS
> > tristate "NXP MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
> > depends on ARCH_MXC || COMPILE_TEST diff --git
> > a/drivers/media/platform/nxp/Makefile
> > b/drivers/media/platform/nxp/Makefile
> > index 4d90eb713652..6410115d870e 100644
> > --- a/drivers/media/platform/nxp/Makefile
> > +++ b/drivers/media/platform/nxp/Makefile
> > @@ -6,6 +6,7 @@ obj-y += imx8-isi/
> >
> > obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> > obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
> > +obj-$(CONFIG_VIDEO_IMX95_CSI_FORMATTER) += imx95-csi-formatter.o
> > obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> > obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
> > obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o diff --git
> > a/drivers/media/platform/nxp/imx95-csi-formatter.c
> > b/drivers/media/platform/nxp/imx95-csi-formatter.c
> > new file mode 100644
> > index 000000000000..cea60327c972
> > --- /dev/null
> > +++ b/drivers/media/platform/nxp/imx95-csi-formatter.c
> > @@ -0,0 +1,808 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2025 NXP
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/clk.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +
> > +#include <media/mipi-csi2.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +/* CSI Pixel Formatter registers map */
> > +
> > +#define CSI_VC_INTERLACED_LINE_CNT(vc) (0x00 + (vc) * 0x04)
> > +#define INTERLACED_ODD_LINE_CNT_SET(x) FIELD_PREP(GENMASK(13,
> 0), (x))
> > +#define INTERLACED_EVEN_LINE_CNT_SET(x)
> FIELD_PREP(GENMASK(29, 16), (x))
> > +
> > +#define CSI_VC_INTERLACED_CTRL 0x20
> > +
> > +#define CSI_VC_INTERLACED_ERR 0x24
> > +#define CSI_VC_ERR_MASK GENMASK(7, 0)
> > +#define CSI_VC_ERR(vc) BIT((vc))
> > +
> > +#define CSI_VC_YUV420_FIRST_LINE_EVEN 0x28
> > +#define YUV420_FIRST_LINE_EVEN(vc) BIT((vc))
> > +
> > +#define CSI_RAW32_CTRL 0x30
> > +#define CSI_VC_RAW32_MODE(vc) BIT((vc))
> > +#define CSI_VC_RAW32_SWAP_MODE(vc) BIT((vc) + 8)
> > +
> > +#define CSI_STREAM_FENCING_CTRL 0x34
> > +#define CSI_VC_STREAM_FENCING(vc) BIT((vc))
> > +#define CSI_VC_STREAM_FENCING_RST(vc) BIT((vc) + 8)
> > +
> > +#define CSI_STREAM_FENCING_STS 0x38
> > +#define CSI_STREAM_FENCING_STS_MASK GENMASK(7, 0)
> > +
> > +#define CSI_VC_NON_PIXEL_DATA_TYPE(vc) (0x40 + (vc) * 0x04)
> > +
> > +#define CSI_VC_PIXEL_DATA_CTRL(vc) (0x60 + (vc) * 0x04)
> > +#define NEW_VC(vc) FIELD_PREP(GENMASK(3, 1), vc)
> > +#define REROUTE_VC_ENABLE BIT(0)
> > +
> > +#define CSI_VC_ROUTE_PIXEL_DATA_TYPE(vc) (0x80 + (vc) * 0x04)
> > +
> > +#define CSI_VC_NON_PIXEL_DATA_CTRL(vc) (0xa0 + (vc) * 0x04)
> > +
> > +#define CSI_VC_PIXEL_DATA_TYPE(vc) (0xc0 + (vc) * 0x04)
> > +
> > +#define CSI_VC_PIXEL_DATA_TYPE_ERR(vc) (0xe0 + (vc) * 0x04)
> > +
> > +#define CSI_FORMATTER_PAD_SINK 0
> > +#define CSI_FORMATTER_PAD_SOURCE 1
> > +#define CSI_FORMATTER_PAD_NUM 2
> > +
> > +#define CSI_FORMATTER_VC_NUM 8 /* Number of virtual
> channels */
> > +
> > +struct csi_formatter_pix_format {
> > + u32 code;
> > + u32 data_type;
> > +};
> > +
> > +struct csi_formatter {
> > + struct device *dev;
> > + struct regmap *regs;
> > + struct clk *clk;
> > +
> > + struct v4l2_subdev sd;
> > + struct v4l2_subdev *csi_sd;
> > + struct v4l2_async_notifier notifier;
> > + struct media_pad pads[CSI_FORMATTER_PAD_NUM];
> > +
> > + u32 remote_pad;
> > + u32 reg_offset;
> > +
> > + /* Protects enabled_streams */
> > + struct mutex lock;
> > + u64 enabled_streams;
> > +};
> > +
> > +struct csi_formatter_dt_index {
> > + u8 dtype;
> > + u8 index;
> > +};
> > +
> > +/*
> > + * The index corresponds to the bit index in the register that
> > +enables
> > + * the data type of pixel data transported by the Formatter.
> > + */
> > +static const struct csi_formatter_dt_index formatter_dt_to_index_map[] = {
> > + { .dtype = MIPI_CSI2_DT_YUV420_8B, .index = 0 },
> > + { .dtype = MIPI_CSI2_DT_YUV420_8B_LEGACY, .index = 2 },
> > + { .dtype = MIPI_CSI2_DT_YUV422_8B, .index = 6 },
> > + { .dtype = MIPI_CSI2_DT_RGB444, .index = 8 },
> > + { .dtype = MIPI_CSI2_DT_RGB555, .index = 9 },
> > + { .dtype = MIPI_CSI2_DT_RGB565, .index = 10 },
> > + { .dtype = MIPI_CSI2_DT_RGB666, .index = 11 },
> > + { .dtype = MIPI_CSI2_DT_RGB888, .index = 12 },
> > + { .dtype = MIPI_CSI2_DT_RAW6, .index = 16 },
> > + { .dtype = MIPI_CSI2_DT_RAW7, .index = 17 },
> > + { .dtype = MIPI_CSI2_DT_RAW8, .index = 18 },
> > + { .dtype = MIPI_CSI2_DT_RAW10, .index = 19 },
> > + { .dtype = MIPI_CSI2_DT_RAW12, .index = 20 },
> > + { .dtype = MIPI_CSI2_DT_RAW14, .index = 21 },
> > + { .dtype = MIPI_CSI2_DT_RAW16, .index = 22 },
> > +};
> > +
> > +static const struct csi_formatter_pix_format formats[] = {
> > + /* YUV formats */
> > + { MEDIA_BUS_FMT_UYVY8_1X16, MIPI_CSI2_DT_YUV422_8B },
> > + /* RGB formats */
> > + { MEDIA_BUS_FMT_RGB565_1X16, MIPI_CSI2_DT_RGB565 },
> > + { MEDIA_BUS_FMT_RGB888_1X24, MIPI_CSI2_DT_RGB888 },
> > + /* RAW (Bayer and greyscale) formats */
> > + { MEDIA_BUS_FMT_SBGGR8_1X8, MIPI_CSI2_DT_RAW8 },
> > + { MEDIA_BUS_FMT_SGBRG8_1X8, MIPI_CSI2_DT_RAW8 },
> > + { MEDIA_BUS_FMT_SGRBG8_1X8, MIPI_CSI2_DT_RAW8 },
> > + { MEDIA_BUS_FMT_SRGGB8_1X8, MIPI_CSI2_DT_RAW8 },
> > + { MEDIA_BUS_FMT_Y8_1X8, MIPI_CSI2_DT_RAW8 },
> > + { MEDIA_BUS_FMT_SBGGR10_1X10, MIPI_CSI2_DT_RAW10 },
> > + { MEDIA_BUS_FMT_SGBRG10_1X10, MIPI_CSI2_DT_RAW10 },
> > + { MEDIA_BUS_FMT_SGRBG10_1X10, MIPI_CSI2_DT_RAW10 },
> > + { MEDIA_BUS_FMT_SRGGB10_1X10, MIPI_CSI2_DT_RAW10 },
> > + { MEDIA_BUS_FMT_Y10_1X10, MIPI_CSI2_DT_RAW10 },
> > + { MEDIA_BUS_FMT_SBGGR12_1X12, MIPI_CSI2_DT_RAW12 },
> > + { MEDIA_BUS_FMT_SGBRG12_1X12, MIPI_CSI2_DT_RAW12 },
> > + { MEDIA_BUS_FMT_SGRBG12_1X12, MIPI_CSI2_DT_RAW12 },
> > + { MEDIA_BUS_FMT_SRGGB12_1X12, MIPI_CSI2_DT_RAW12 },
> > + { MEDIA_BUS_FMT_Y12_1X12, MIPI_CSI2_DT_RAW12 },
> > + { MEDIA_BUS_FMT_SBGGR14_1X14, MIPI_CSI2_DT_RAW14 },
> > + { MEDIA_BUS_FMT_SGBRG14_1X14, MIPI_CSI2_DT_RAW14 },
> > + { MEDIA_BUS_FMT_SGRBG14_1X14, MIPI_CSI2_DT_RAW14 },
> > + { MEDIA_BUS_FMT_SRGGB14_1X14, MIPI_CSI2_DT_RAW14 },
> > + { MEDIA_BUS_FMT_SBGGR16_1X16, MIPI_CSI2_DT_RAW16 },
> > + { MEDIA_BUS_FMT_SGBRG16_1X16, MIPI_CSI2_DT_RAW16 },
> > + { MEDIA_BUS_FMT_SGRBG16_1X16, MIPI_CSI2_DT_RAW16 },
> > + { MEDIA_BUS_FMT_SRGGB16_1X16, MIPI_CSI2_DT_RAW16 },
> > +};
> > +
> > +static const struct v4l2_mbus_framefmt formatter_default_fmt = {
> > + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> > + .width = 1920U,
> > + .height = 1080U,
> > + .field = V4L2_FIELD_NONE,
> > + .colorspace = V4L2_COLORSPACE_SMPTE170M,
> > + .xfer_func =
> V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> > + .ycbcr_enc =
> V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> > + .quantization = V4L2_QUANTIZATION_LIM_RANGE, };
> > +
> > +static const struct csi_formatter_pix_format
> > +*csi_formatter_find_format(u32 code) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(formats); i++)
> > + if (code == formats[i].code)
> > + return &formats[i];
> > +
> > + return NULL;
> > +}
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * V4L2 subdev operations
> > + */
> > +
> > +static inline struct csi_formatter *sd_to_formatter(struct
> > +v4l2_subdev *sdev) {
> > + return container_of(sdev, struct csi_formatter, sd); }
> > +
> > +static int __csi_formatter_subdev_set_routing(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + struct
> > +v4l2_subdev_krouting *routing) {
> > + int ret;
> > +
> > + ret = v4l2_subdev_routing_validate(sd, routing,
> > + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> > + if (ret)
> > + return ret;
> > +
> > + return v4l2_subdev_set_routing_with_fmt(sd, state, routing,
> > +
> > +&formatter_default_fmt); }
> > +
> > +static int csi_formatter_subdev_init_state(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state
> > +*sd_state) {
> > + struct v4l2_subdev_route routes[] = {
> > + {
> > + .sink_pad = CSI_FORMATTER_PAD_SINK,
> > + .sink_stream = 0,
> > + .source_pad = CSI_FORMATTER_PAD_SOURCE,
> > + .source_stream = 0,
> > + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> > + },
> > + };
> > +
> > + struct v4l2_subdev_krouting routing = {
> > + .num_routes = ARRAY_SIZE(routes),
> > + .routes = routes,
> > + };
> > +
> > + return __csi_formatter_subdev_set_routing(sd, sd_state,
> > +&routing); }
> > +
> > +static int csi_formatter_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state,
> > + struct
> > +v4l2_subdev_mbus_code_enum *code) {
> > + if (code->pad == CSI_FORMATTER_PAD_SOURCE) {
> > + struct v4l2_mbus_framefmt *fmt;
> > +
> > + if (code->index > 0)
> > + return -EINVAL;
> > +
> > + fmt = v4l2_subdev_state_get_format(sd_state, code->pad,
> > + code->stream);
> > + code->code = fmt->code;
> > + return 0;
> > + }
> > +
> > + if (code->index >= ARRAY_SIZE(formats))
> > + return -EINVAL;
> > +
> > + code->code = formats[code->index].code;
> > +
> > + return 0;
> > +}
> > +
> > +static int csi_formatter_subdev_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state,
> > + struct v4l2_subdev_format
> > +*sdformat) {
> > + struct csi_formatter_pix_format const *format;
> > + struct v4l2_mbus_framefmt *fmt;
> > +
> > + if (sdformat->pad == CSI_FORMATTER_PAD_SOURCE)
> > + return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> > +
> > + format = csi_formatter_find_format(sdformat->format.code);
> > + if (!format)
> > + format = &formats[0];
> > +
> > + v4l_bound_align_image(&sdformat->format.width, 1, 0xffff, 2,
> > + &sdformat->format.height, 1, 0xffff, 0,
> > + 0);
> > +
> > + fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad,
> > + sdformat->stream);
> > + *fmt = sdformat->format;
> > +
> > + /* Set default code if user set an invalid value */
> > + fmt->code = format->code;
> > + sdformat->format = *fmt;
> > +
> > + /* Propagate the format from sink stream to source stream */
> > + fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state,
> sdformat->pad,
> > + sdformat->stream);
> > + if (!fmt)
> > + return -EINVAL;
> > +
> > + *fmt = sdformat->format;
> > +
> > + return 0;
> > +}
> > +
> > +static int csi_formatter_subdev_set_routing(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + enum v4l2_subdev_format_whence which,
> > + struct
> > +v4l2_subdev_krouting *routing) {
> > + if (which == V4L2_SUBDEV_FORMAT_ACTIVE &&
> > + media_entity_is_streaming(&sd->entity))
> > + return -EBUSY;
> > +
> > + return __csi_formatter_subdev_set_routing(sd, state, routing);
> > +}
> > +
> > +static inline void csi_formatter_write(struct csi_formatter *formatter,
> > + unsigned int reg, unsigned int
> > +value) {
> > + u32 offset = formatter->reg_offset;
> > +
> > + regmap_write(formatter->regs, reg + offset, value); }
> > +
> > +static inline void csi_formatter_read(struct csi_formatter *formatter,
> > + unsigned int reg, unsigned int
> > +*value) {
> > + u32 offset = formatter->reg_offset;
> > +
> > + regmap_read(formatter->regs, reg + offset, value); }
> > +
> > +static u8 csi_formatter_get_index_by_dt(u8 data_type) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(formatter_dt_to_index_map); ++i) {
> > + const struct csi_formatter_dt_index *entry =
> > + &formatter_dt_to_index_map[i];
> > +
> > + if (data_type == entry->dtype)
> > + return entry->index;
> > + }
> > +
> > + pr_warn_once("Unsupported data type 0x%x, using default\n",
> > + data_type);
>
> Why use the _once variant here? This doesn't look like a hot path, nor does it
> seem like a condition we should silently ignore after the first occurrence.
> Also, if dev_warn_.. is not used here, pr_warn_once() should at least give some
> context.
>
You're right. I've replaced pr_warn_once() with dev_warn() to provide
device context and warn on every occurrence. Will be addressed in v13.
> > +
> > + return formatter_dt_to_index_map[0].index;
> > +}
> > +
> > +static int csi_formatter_get_vc(struct csi_formatter *formatter,
> > + struct v4l2_mbus_frame_desc *fd,
> > + unsigned int stream) {
> > + struct v4l2_mbus_frame_desc_entry *entry = NULL;
> > + unsigned int i;
> > + u8 vc;
> > +
> > + for (i = 0; i < fd->num_entries; ++i) {
> > + if (fd->entry[i].stream == stream) {
> > + entry = &fd->entry[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!entry) {
> > + dev_err(formatter->dev,
> > + "No frame desc entry for stream %u\n", stream);
> > + return -EPIPE;
> > + }
> > +
> > + vc = entry->bus.csi2.vc;
> > +
> > + if (vc >= CSI_FORMATTER_VC_NUM) {
> > + dev_err(formatter->dev, "Invalid virtual channel %u\n", vc);
> > + return -EINVAL;
> > + }
> > +
> > + return vc;
> > +}
> > +
> > +static void csi_formatter_stop_stream(struct csi_formatter *formatter,
> > + struct v4l2_subdev_state *state,
> > + u64 stream_mask) {
> > + const struct csi_formatter_pix_format *pix_fmt;
> > + struct v4l2_mbus_frame_desc fd = {};
> > + struct v4l2_subdev_route *route;
> > + struct v4l2_mbus_framefmt *fmt;
> > + u32 val;
> > + int vc;
> > + int ret;
> > +
> > + ret = v4l2_subdev_call(formatter->csi_sd, pad, get_frame_desc,
> > + formatter->remote_pad, &fd);
> > + if (ret < 0 && ret != -ENOIOCTLCMD) {
> > + dev_err(formatter->dev, "Failed to get frame desc: %d\n", ret);
> > + return;
> > + }
> > +
> > + for_each_active_route(&state->routing, route) {
> > + if (route->source_pad != CSI_FORMATTER_PAD_SOURCE)
> > + continue;
> > +
> > + if (!(stream_mask & BIT_ULL(route->source_stream)))
> > + continue;
> > +
> > + if (ret == -ENOIOCTLCMD) {
> > + /*
> > + * Source doesn't implement get_frame_desc, use
> > + * default VC 0
> > + */
> > + vc = 0;
> > + } else {
> > + vc = csi_formatter_get_vc(formatter, &fd, route->sink_stream);
> > + if (vc < 0)
> > + continue;
> > + }
> > +
> > + fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> > +
> > + route->sink_stream);
> > +
> > + pix_fmt = csi_formatter_find_format(fmt->code);
>
> Do I understand correctly that csi_formatter_find_format() can't return NULL at
> this point? If that's guaranteed by the surrounding logic, it might be worth
> making that assumption explicit with a WARN_ON or BUG_ON, to help catch
> future changes that could violate it.
>
Correct. In practice, csi_formatter_find_format() should not return NULL at this point
because subdev_set_fmt() falls back to formats[0] when the user provides an invalid
format code.
However, I agree that making this assumption explicit is valuable - it helps catch future
changes that could violate this invariant. Will be addressed in v13.
> > +
> > + /* Clear only this stream's data type bit */
> > + csi_formatter_read(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), &val);
> > + val &= ~BIT(csi_formatter_get_index_by_dt(pix_fmt->data_type));
> > + csi_formatter_write(formatter,
> > + CSI_VC_PIXEL_DATA_TYPE(vc), val);
>
> This read/modify/write pattern appears in several places. It might be worth
> introducing a small helper around regmap_update_bits().
Agreed. I've switched to using regmap_update_bits() directly and removed
the unused csi_formatter_read/write helper functions. Will be addressed
in v13.
>
> I haven't looked too deeply at the multi-stream handling, but could you confirm
> that this doesn't break the case where two streams use the same data type? As
> I understand it, when one stream is stopped, the corresponding DT bit is
> cleared unconditionally, which could disable the data type even though it is still
> required by another active stream?
Good question. The formatter hardware is designed such that each virtual channel has
its own independent CSI_VC_PIXEL_DATA_TYPE register. When two streams use the same
data type but on different virtual channels, the DT bit being cleared belongs to a specific
VC's register, not a shared one. So active streams on other VCs with the same data type
will continue to work correctly.
>
> Regards,
> Loic
>
> > + }
> > +}
> > +
> > +static int csi_formatter_start_stream(struct csi_formatter *formatter,
> > + struct v4l2_subdev_state *state,
> > + u64 stream_mask) {
> > + const struct csi_formatter_pix_format *pix_fmt;
> > + struct v4l2_subdev_route *route;
> > + struct v4l2_mbus_framefmt *fmt;
> > + struct v4l2_mbus_frame_desc fd = {};
> > + u64 configured_streams = 0;
> > + u32 val;
> > + int vc;
> > + int ret;
> > +
> > + ret = v4l2_subdev_call(formatter->csi_sd, pad, get_frame_desc,
> > + formatter->remote_pad, &fd);
> > + if (ret < 0 && ret != -ENOIOCTLCMD) {
> > + dev_err(formatter->dev, "Failed to get frame desc: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + for_each_active_route(&state->routing, route) {
> > + if (route->source_pad != CSI_FORMATTER_PAD_SOURCE)
> > + continue;
> > +
> > + if (!(stream_mask & BIT_ULL(route->source_stream)))
> > + continue;
> > +
> > + if (ret == -ENOIOCTLCMD) {
> > + /*
> > + * Source doesn't implement get_frame_desc, use
> > + * default VC 0
> > + */
> > + vc = 0;
> > + } else {
> > + vc = csi_formatter_get_vc(formatter, &fd, route->sink_stream);
> > + if (vc < 0) {
> > + ret = vc;
> > + goto err_cleanup;
> > + }
> > + }
> > +
> > + fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> > +
> > + route->sink_stream);
> > +
> > + pix_fmt = csi_formatter_find_format(fmt->code);
> > +
> > + /* Update VC configuration */
> > + csi_formatter_read(formatter, CSI_VC_PIXEL_DATA_TYPE(vc), &val);
> > + val |= BIT(csi_formatter_get_index_by_dt(pix_fmt->data_type));
> > + csi_formatter_write(formatter,
> > + CSI_VC_PIXEL_DATA_TYPE(vc), val);
> > +
> > + configured_streams |= BIT_ULL(route->source_stream);
> > + }
> > +
> > + return 0;
> > +
> > +err_cleanup:
> > + csi_formatter_stop_stream(formatter, state, configured_streams);
> > + return ret;
> > +}
> > +
> > +static int csi_formatter_subdev_enable_streams(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64
> > +streams_mask) {
> > + struct csi_formatter *formatter = sd_to_formatter(sd);
> > + struct device *dev = formatter->dev;
> > + u64 sink_streams;
> > + int ret;
> > +
> > + sink_streams = v4l2_subdev_state_xlate_streams(state,
> > + CSI_FORMATTER_PAD_SOURCE,
> > + CSI_FORMATTER_PAD_SINK,
> > + &streams_mask);
> > + if (!sink_streams || !streams_mask)
> > + return -EINVAL;
> > +
> > + guard(mutex)(&formatter->lock);
> > +
> > + if (!formatter->enabled_streams) {
> > + ret = pm_runtime_resume_and_get(formatter->dev);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to resume runtime PM: %d\n", ret);
> > + return ret;
> > + }
> > + }
> > +
> > + ret = csi_formatter_start_stream(formatter, state, streams_mask);
> > + if (ret)
> > + goto err_runtime_put;
> > +
> > + ret = v4l2_subdev_enable_streams(formatter->csi_sd,
> > + formatter->remote_pad,
> > + sink_streams);
> > + if (ret)
> > + goto err_stop_stream;
> > +
> > + formatter->enabled_streams |= streams_mask;
> > +
> > + return 0;
> > +
> > +err_stop_stream:
> > + csi_formatter_stop_stream(formatter, state, streams_mask);
> > +err_runtime_put:
> > + if (!formatter->enabled_streams)
> > + pm_runtime_put(formatter->dev);
> > + return ret;
> > +}
> > +
> > +static int csi_formatter_subdev_disable_streams(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64
> > +streams_mask) {
> > + struct csi_formatter *formatter = sd_to_formatter(sd);
> > + u64 sink_streams;
> > + int ret;
> > +
> > + sink_streams = v4l2_subdev_state_xlate_streams(state,
> > + CSI_FORMATTER_PAD_SOURCE,
> > + CSI_FORMATTER_PAD_SINK,
> > + &streams_mask);
> > + if (!sink_streams || !streams_mask)
> > + return -EINVAL;
> > +
> > + guard(mutex)(&formatter->lock);
> > +
> > + ret = v4l2_subdev_disable_streams(formatter->csi_sd, formatter-
> >remote_pad,
> > + sink_streams);
> > + if (ret)
> > + dev_err(formatter->dev, "Failed to disable streams:
> > + %d\n", ret);
> > +
> > + csi_formatter_stop_stream(formatter, state, streams_mask);
> > +
> > + formatter->enabled_streams &= ~streams_mask;
> > +
> > + if (!formatter->enabled_streams)
> > + pm_runtime_put(formatter->dev);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_pad_ops formatter_subdev_pad_ops = {
> > + .enum_mbus_code = csi_formatter_subdev_enum_mbus_code,
> > + .get_fmt = v4l2_subdev_get_fmt,
> > + .set_fmt = csi_formatter_subdev_set_fmt,
> > + .get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
> > + .set_routing = csi_formatter_subdev_set_routing,
> > + .enable_streams = csi_formatter_subdev_enable_streams,
> > + .disable_streams = csi_formatter_subdev_disable_streams,
> > +};
> > +
> > +static const struct v4l2_subdev_ops formatter_subdev_ops = {
> > + .pad = &formatter_subdev_pad_ops, };
> > +
> > +static const struct v4l2_subdev_internal_ops formatter_internal_ops = {
> > + .init_state = csi_formatter_subdev_init_state, };
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * Media entity operations
> > + */
> > +
> > +static const struct media_entity_operations formatter_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate,
> > + .get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> > +};
> > +
> > +static int csi_formatter_subdev_init(struct csi_formatter *formatter)
> > +{
> > + struct v4l2_subdev *sd = &formatter->sd;
> > + int ret;
> > +
> > + v4l2_subdev_init(sd, &formatter_subdev_ops);
> > +
> > + snprintf(sd->name, sizeof(sd->name), "%s", dev_name(formatter->dev));
> > + sd->internal_ops = &formatter_internal_ops;
> > +
> > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > + V4L2_SUBDEV_FL_STREAMS;
> > + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> > + sd->entity.ops = &formatter_entity_ops;
> > + sd->dev = formatter->dev;
> > +
> > + formatter->pads[CSI_FORMATTER_PAD_SINK].flags =
> MEDIA_PAD_FL_SINK
> > + | MEDIA_PAD_FL_MUST_CONNECT;
> > + formatter->pads[CSI_FORMATTER_PAD_SOURCE].flags =
> > + MEDIA_PAD_FL_SOURCE;
> > +
> > + ret = media_entity_pads_init(&sd->entity, CSI_FORMATTER_PAD_NUM,
> > + formatter->pads);
> > + if (ret) {
> > + dev_err(formatter->dev, "Failed to init pads\n");
> > + return ret;
> > + }
> > +
> > + ret = v4l2_subdev_init_finalize(sd);
> > + if (ret)
> > + media_entity_cleanup(&sd->entity);
> > +
> > + return ret;
> > +}
> > +
> > +static inline struct csi_formatter *
> > +notifier_to_csi_formatter(struct v4l2_async_notifier *n) {
> > + return container_of(n, struct csi_formatter, notifier); }
> > +
> > +static int csi_formatter_notify_bound(struct v4l2_async_notifier *notifier,
> > + struct v4l2_subdev *sd,
> > + struct v4l2_async_connection
> > +*asc) {
> > + const unsigned int link_flags = MEDIA_LNK_FL_IMMUTABLE
> > + | MEDIA_LNK_FL_ENABLED;
> > + struct csi_formatter *formatter = notifier_to_csi_formatter(notifier);
> > + struct v4l2_subdev *sdev = &formatter->sd;
> > + struct media_pad *sink = &sdev-
> >entity.pads[CSI_FORMATTER_PAD_SINK];
> > + struct media_pad *remote_pad;
> > + int ret;
> > +
> > + formatter->csi_sd = sd;
> > +
> > + dev_dbg(formatter->dev, "Bound subdev: %s pad\n", sd->name);
> > +
> > + ret = v4l2_create_fwnode_links_to_pad(sd, sink, link_flags);
> > + if (ret < 0)
> > + return ret;
> > +
> > + remote_pad = media_pad_remote_pad_first(sink);
> > + if (!remote_pad) {
> > + dev_err(formatter->dev, "Pipe not setup correctly\n");
> > + return -EPIPE;
> > + }
> > + formatter->remote_pad = remote_pad->index;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations formatter_notify_ops = {
> > + .bound = csi_formatter_notify_bound, };
> > +
> > +static int csi_formatter_async_register(struct csi_formatter
> > +*formatter) {
> > + struct device *dev = formatter->dev;
> > + struct v4l2_async_connection *asc;
> > + int ret;
> > +
> > + struct fwnode_handle *ep __free(fwnode_handle) =
> > + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
> > + FWNODE_GRAPH_ENDPOINT_NEXT);
> > + if (!ep)
> > + return -ENOTCONN;
> > +
> > + v4l2_async_subdev_nf_init(&formatter->notifier,
> > + &formatter->sd);
> > +
> > + asc = v4l2_async_nf_add_fwnode_remote(&formatter->notifier, ep,
> > + struct v4l2_async_connection);
> > + if (IS_ERR(asc)) {
> > + ret = PTR_ERR(asc);
> > + goto err_cleanup_notifier;
> > + }
> > +
> > + formatter->notifier.ops = &formatter_notify_ops;
> > +
> > + ret = v4l2_async_nf_register(&formatter->notifier);
> > + if (ret)
> > + goto err_cleanup_notifier;
> > +
> > + ret = v4l2_async_register_subdev(&formatter->sd);
> > + if (ret)
> > + goto err_unregister_notifier;
> > +
> > + return 0;
> > +
> > +err_unregister_notifier:
> > + v4l2_async_nf_unregister(&formatter->notifier);
> > +err_cleanup_notifier:
> > + v4l2_async_nf_cleanup(&formatter->notifier);
> > + return ret;
> > +}
> > +
> > +static void csi_formatter_async_unregister(struct csi_formatter
> > +*formatter) {
> > + v4l2_async_unregister_subdev(&formatter->sd);
> > + v4l2_async_nf_unregister(&formatter->notifier);
> > + v4l2_async_nf_cleanup(&formatter->notifier);
> > +}
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * Suspend/resume
> > + */
> > +
> > +static int csi_formatter_runtime_suspend(struct device *dev) {
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct csi_formatter *formatter = sd_to_formatter(sd);
> > +
> > + clk_disable_unprepare(formatter->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int csi_formatter_runtime_resume(struct device *dev) {
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct csi_formatter *formatter = sd_to_formatter(sd);
> > +
> > + return clk_prepare_enable(formatter->clk);
> > +}
> > +
> > +static DEFINE_RUNTIME_DEV_PM_OPS(csi_formatter_pm_ops,
> > + csi_formatter_runtime_suspend,
> > + csi_formatter_runtime_resume, NULL);
> > +
> > +static int csi_formatter_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct csi_formatter *formatter;
> > + u32 val;
> > + int ret;
> > +
> > + formatter = devm_kzalloc(dev, sizeof(*formatter), GFP_KERNEL);
> > + if (!formatter)
> > + return -ENOMEM;
> > +
> > + formatter->dev = dev;
> > +
> > + ret = devm_mutex_init(dev, &formatter->lock);
> > + if (ret)
> > + return ret;
> > +
> > + formatter->regs = syscon_node_to_regmap(dev->parent->of_node);
> > + if (IS_ERR(formatter->regs))
> > + return dev_err_probe(dev, PTR_ERR(formatter->regs),
> > + "Failed to get csi formatter
> > + regmap\n");
> > +
> > + ret = of_property_read_u32(dev->of_node, "reg", &val);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret,
> > + "Failed to get csi formatter reg
> > + property\n");
> > +
> > + formatter->reg_offset = val;
> > +
> > + formatter->clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(formatter->clk))
> > + return dev_err_probe(dev, PTR_ERR(formatter->clk),
> > + "Failed to get pixel clock\n");
> > +
> > + ret = csi_formatter_subdev_init(formatter);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to initialize
> > + formatter subdev\n");
> > +
> > + platform_set_drvdata(pdev, &formatter->sd);
> > +
> > + /* Enable runtime PM. */
> > + ret = devm_pm_runtime_enable(dev);
> > + if (ret)
> > + goto err_cleanup_subdev;
> > +
> > + ret = csi_formatter_async_register(formatter);
> > + if (ret < 0) {
> > + dev_err_probe(dev, ret, "Failed to register async subdevice\n");
> > + goto err_cleanup_subdev;
> > + }
> > +
> > + return 0;
> > +
> > +err_cleanup_subdev:
> > + v4l2_subdev_cleanup(&formatter->sd);
> > + media_entity_cleanup(&formatter->sd.entity);
> > + return ret;
> > +}
> > +
> > +static void csi_formatter_remove(struct platform_device *pdev) {
> > + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> > + struct csi_formatter *formatter = sd_to_formatter(sd);
> > +
> > + csi_formatter_async_unregister(formatter);
> > +
> > + v4l2_subdev_cleanup(&formatter->sd);
> > + media_entity_cleanup(&formatter->sd.entity);
> > +}
> > +
> > +static const struct of_device_id csi_formatter_of_match[] = {
> > + { .compatible = "fsl,imx95-csi-formatter" },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, csi_formatter_of_match);
> > +
> > +static struct platform_driver csi_formatter_device_driver = {
> > + .driver = {
> > + .name = "csi-pixel-formatter",
> > + .of_match_table = csi_formatter_of_match,
> > + .pm = pm_ptr(&csi_formatter_pm_ops),
> > + },
> > + .probe = csi_formatter_probe,
> > + .remove = csi_formatter_remove, };
> > +
> > +module_platform_driver(csi_formatter_device_driver);
> > +
> > +MODULE_AUTHOR("NXP Semiconductor, Inc.");
> MODULE_DESCRIPTION("NXP
> > +i.MX95 CSI Pixel Formatter driver"); MODULE_LICENSE("GPL");
> >
> > --
> > 2.34.1
> >
> >
^ permalink raw reply
* [PATCH v2] cpufreq: apple-soc: Calculate frequency as a 64-bit value
From: Sasha Finkelstein @ 2026-07-20 7:25 UTC (permalink / raw)
To: Sven Peter, Janne Grunau, Neal Gompa, Rafael J. Wysocki,
Viresh Kumar
Cc: asahi, linux-arm-kernel, linux-pm, linux-kernel,
Sasha Finkelstein
The current frequency calculation is done in 32 bit, causing problems
if run on a future SoC that can boost higher than 4.2GHz. Ideally, we
should use a true u64 instead of unsigned long and "knowning" that this
only runs on 64 bit machines, but the core code uses ulong everywhere,
so this should be good enough.
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
Changes in v2:
- Minor style fixes
- Link to v1: https://patch.msgid.link/20260703-cpufreq-64-v1-1-c406c705319a@chaosmail.tech
---
drivers/cpufreq/apple-soc-cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/apple-soc-cpufreq.c b/drivers/cpufreq/apple-soc-cpufreq.c
index 638e5bf72185..5ad274a1e6ae 100644
--- a/drivers/cpufreq/apple-soc-cpufreq.c
+++ b/drivers/cpufreq/apple-soc-cpufreq.c
@@ -288,7 +288,7 @@ static int apple_soc_cpufreq_init(struct cpufreq_policy *policy)
/* Get OPP levels (p-state indexes) and stash them in driver_data */
for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
- unsigned long rate = freq_table[i].frequency * 1000 + 999;
+ unsigned long rate = freq_table[i].frequency * 1000UL + 999;
struct dev_pm_opp *opp = dev_pm_opp_find_freq_floor(cpu_dev, &rate);
if (IS_ERR(opp)) {
---
base-commit: 4a50a141f05a8d1737661b19ee22ff8455b94409
change-id: 20260703-cpufreq-64-2a23d7261e09
Best regards,
--
Sasha Finkelstein <k@chaosmail.tech>
^ permalink raw reply related
* Re: [PATCH v22 06/14] dmaengine: qcom: bam_dma: add support for BAM locking
From: Stephan Gerhold @ 2026-07-20 7:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Bjorn Andersson, Mukesh Kumar Savaliya,
Peter Ujfalusi, Michal Simek, Frank Li, Neil Armstrong,
Vignesh Raghavendra, dmaengine, linux-doc, linux-kernel,
linux-arm-msm, linux-crypto, linux-arm-kernel, brgl,
sashiko-reviews
In-Reply-To: <20260717085136.E0FBC1F000E9@smtp.kernel.org>
On Fri, Jul 17, 2026 at 08:51:36AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] DMA API violation due to mismatched device pointers in dma_map_sg and dma_unmap_sg.
> - [High] The client-provided physical address for the lock scratchpad is silently truncated to 24 bits.
> - [Medium] Transient allocation failure for lock descriptors causes a permanent channel stall.
> - [Medium] Pipe locking is restricted to TX channels instead of RX channels, contradicting intended design.
> - [Medium] The dummy lock descriptor clears the `reconfigure` flag using a hardcoded TX direction, potentially misconfiguring the channel.
> - [Low] Missing integer argument for `%d` in `dev_err_ratelimited` call.
> --
>
> --- Patch [6]: [PATCH v22 06/14] dmaengine: qcom: bam_dma: add support for BAM locking ---
> commit c786d3c51276cef4b55a461357b399c32982316b
> Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Subject: dmaengine: qcom: bam_dma: add support for BAM locking
>
> This commit adds support for BAM pipe locking by prepending and appending
> dummy command descriptors with LOCK and UNLOCK bits set. It aims to prevent
> writes into config registers while the engine is busy.
>
Thanks for all the changes! It looks good to me for the most part, so
I'll just comment on the remaining Sashiko comments below.
> > Add support for BAM pipe locking. To that end: when starting DMA on an RX
> > channel - prepend the existing queue of issued descriptors with an
>
> [Severity: Medium]
> Is this intended to be restricted to TX channels? The commit message mentions
> starting DMA on an RX channel, but the implementation in bam_setup_pipe_lock()
> aborts the lock setup if the direction is not DMA_MEM_TO_DEV.
>
Is this a typo in your commit message? I guess RX vs TX depends on the
perspective (kernel or peripheral), but I think "TX channel" would be
clearer here (or even just put DMA_MEM_TO_DEV).
> > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> > index f3e713a5259c2..edaa694eb9df0 100644
> > --- a/drivers/dma/qcom/bam_dma.c
> > +++ b/drivers/dma/qcom/bam_dma.c
>
> [ ... ]
>
> > @@ -859,6 +887,15 @@ static int bam_resume(struct dma_chan *chan)
> > return 0;
> > }
> >
> > +static void bam_dma_free_lock_desc(struct virt_dma_desc *vd)
> > +{
> > + struct bam_async_desc *async_desc = container_of(vd, struct bam_async_desc, vd);
> > + struct dma_chan *chan = vd->tx.chan;
> > +
> > + dma_unmap_sg(chan->slave, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
>
> [Severity: High]
> Will this cause a DMA API violation?
>
> The scatterlist is unmapped here using the client device (chan->slave), but it
> is mapped in bam_make_lock_desc() using the DMA controller device
> (bchan->bdev->dev).
>
Yep, we need to change all those DMA calls to bchan->bdev->dev.
> > + kfree(async_desc);
> > +}
>
> [ ... ]
>
> > @@ -1046,13 +1090,101 @@ static void bam_apply_new_config(struct bam_chan *bchan,
> > bchan->reconfigure = 0;
> > }
> >
> > +static struct bam_async_desc *
> > +bam_make_lock_desc(struct bam_chan *bchan, unsigned long flag)
> > +{
> > + struct bam_async_desc *async_desc;
>
> [ ... ]
>
> > + bam_prep_ce_le32(&async_desc->lock_ce, bchan->lock_scratchpad_addr, BAM_WRITE_COMMAND, 0);
>
> [Severity: High]
> Can this truncate the physical address and cause memory corruption?
>
> The client-provided lock_scratchpad_addr is a 64-bit phys_addr_t, but
> bam_prep_ce_le32() masks the address to 24 bits:
>
> bam_ce->cmd_and_addr = cpu_to_le32((addr & 0xffffff) | ((cmd & 0xff) << 24));
>
> If the physical address is above 16MB, the BAM engine will perform its dummy
> write to an arbitrary truncated address.
>
This looks correct as well. It seems the address in a BAM command
descriptor is more like a "address within the peripheral" than an actual
physical address, so the same applies to the lock_scratchpad_addr. Can
you clarify this inside the struct bam_config documentation comment?
This also means there is no point to accept a phys_addr_t in
lock_scratchpad_addr, u32 is sufficient.
> > + sg_set_buf(&async_desc->lock_sg, &async_desc->lock_ce, sizeof(async_desc->lock_ce));
> > +
> > + mapped = dma_map_sg(bchan->bdev->dev, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
>
> [ ... ]
>
> > +static int bam_setup_pipe_lock(struct bam_chan *bchan)
> > +{
> > + const struct bam_device_data *bdata = bchan->bdev->dev_data;
> > + struct bam_async_desc *lock_desc, *unlock_desc;
> > +
> > + lockdep_assert_held(&bchan->vc.lock);
> > +
> > + if (!bdata->pipe_lock_supported || !bchan->lock_scratchpad_addr ||
> > + bchan->direction != DMA_MEM_TO_DEV)
> > + return 0;
>
> [Severity: Medium]
> Does this restriction conflict with the commit message?
>
> The condition limits pipe locking to DMA_MEM_TO_DEV (TX), making this feature
> dead code for RX channels (DMA_DEV_TO_MEM).
>
Clarifying the commit message should fix this comment as well.
> > +
> > + /*
>
> [ ... ]
>
> > @@ -1072,6 +1205,22 @@ static void bam_start_dma(struct bam_chan *bchan)
> > return;
> >
> > while (vd && !IS_BUSY(bchan)) {
> > + /*
> > + * Open a LOCK/UNLOCK bracket around each fresh sequence.
> > + * Sentinels inserted by bam_setup_pipe_lock() are skipped: they
> > + * already have bam_locked set and must not trigger a second pair.
> > + */
> > + if (!bchan->bam_locked) {
> > + ret = bam_setup_pipe_lock(bchan);
> > + if (ret) {
> > + dev_err_ratelimited(bdev->dev,
> > + "failed to setup the pipe lock, deferring transfer: %d\n");
>
> [Severity: Low]
> Is this missing the integer argument for the %d format specifier?
>
Yep.
> [Severity: Medium]
> Will this cause a permanent channel stall on memory pressure?
>
> If bam_setup_pipe_lock() fails due to a GFP_NOWAIT allocation failure, the
> loop breaks and leaves the descriptors on the desc_issued list, but no
> mechanism appears to schedule a retry.
>
> > + break;
Could be true, if we break out of the loop here, where will this
deferral happen? Will anything call bam_start_dma() again?
Perhaps we should have queue_work() here?
> > + }
> > + if (bchan->bam_locked)
> > + vd = vchan_next_desc(&bchan->vc);
> > + }
> > +
> > list_del(&vd->node);
> >
> > async_desc = container_of(vd, struct bam_async_desc, vd);
>
> [ ... ]
>
> > /* on first use, initialize the channel hardware */
> > if (!bchan->initialized)
> > bam_chan_init_hw(bchan, async_desc->dir);
> >
> > /* apply new slave config changes, if necessary */
> > if (bchan->reconfigure)
> > bam_apply_new_config(bchan, async_desc->dir);
>
> [Severity: Medium]
> Could the dummy lock descriptor clear the reconfigure flag with the wrong
> direction?
>
> The lock descriptor is created with async_desc->dir hardcoded to
> DMA_MEM_TO_DEV. If pipeline locking is ever enabled for RX channels,
> bam_apply_new_config() will configure the BAM engine using dst_maxburst
> instead of src_maxburst, and clear the bchan->reconfigure flag before the
> real data descriptor is processed.
>
Err, what? I think we can ignore this one, since I don't think there is
a use case for locking on the RX channel.
Although, looking a bit more at your changes, I think you should make
sure the direction is correct already in bam_slave_config():
+ /*
+ * This is required to setup the pipe locking and must be done even
+ * before the first call to bam_start_dma().
+ */
+ if (bdata->pipe_lock_supported && peripheral_cfg) {
+ bchan->lock_scratchpad_addr = peripheral_cfg->lock_scratchpad_addr;
+ bchan->direction = cfg->direction;
+ }
+
I don't think there is much point to store the direction at all if we
just support DMA_MEM_TO_DEV in bam_setup_pipe_lock(). I would just check
the direction inside this if statement and return -EINVAL if some user
tries to setup pipe locking on a TX channel. Then you don't need to
store bchan->direction at all.
> >
>
> [ ... ]
>
> > @@ -1191,8 +1344,11 @@ static void bam_issue_pending(struct dma_chan *chan)
> > */
> > static void bam_dma_free_desc(struct virt_dma_desc *vd)
> > {
> > - struct bam_async_desc *async_desc = container_of(vd,
> > - struct bam_async_desc, vd);
> > + struct bam_async_desc *async_desc = container_of(vd, struct bam_async_desc, vd);
> > + struct dma_chan *chan = vd->tx.chan;
> > +
> > + if (async_desc->is_lock_desc)
> > + dma_unmap_sg(chan->slave, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
>
> [Severity: High]
> Does this share the same DMA API violation mentioned earlier?
>
> The abort/cleanup path also unmaps using the client device instead of the
> DMA controller device.
>
Same thing here as above, yeah. Should be bchan->bdev->dev.
Thanks,
Stephan
^ permalink raw reply
* [PATCH 0/4] ASoC: stm: Drop redundant error message
From: phucduc.bui @ 2026-07-20 7:40 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Olivier Moysan, Arnaud Pouliquen, Maxime Coquelin,
Alexandre Torgue
Cc: linux-stm32, linux-kernel, linux-sound, linux-arm-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Hi all,
This series removes redundant error messages across multiple STM32 ASoC
drivers. Since core functions already log failures internally, dropping
these explicit dev_err() blocks prevents duplicate log messages and
cleans up the code.
Compile-tested only.
Best regards,
Phuc
bui duc phuc (4):
ASoC: stm: stm32_adfsdm: Drop redundant error message
ASoC: stm: stm32_i2s: Drop redundant error messages
ASoC: stm: stm32_sai_sub: Drop redundant error messages
ASoC: stm: stm32_spdifrx: Drop redundant error messages
sound/soc/stm/stm32_adfsdm.c | 5 +----
sound/soc/stm/stm32_i2s.c | 6 ++----
sound/soc/stm/stm32_sai_sub.c | 8 ++------
sound/soc/stm/stm32_spdifrx.c | 6 ++----
4 files changed, 7 insertions(+), 18 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH 1/4] ASoC: stm: stm32_adfsdm: Drop redundant error message
From: phucduc.bui @ 2026-07-20 7:40 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Olivier Moysan, Arnaud Pouliquen, Maxime Coquelin,
Alexandre Torgue
Cc: linux-stm32, linux-kernel, linux-sound, linux-arm-kernel,
bui duc phuc
In-Reply-To: <20260720074044.87528-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
devm_snd_soc_register_component() already logs the failure internally.
Drop the redundant error message and return the original error directly.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
sound/soc/stm/stm32_adfsdm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/sound/soc/stm/stm32_adfsdm.c b/sound/soc/stm/stm32_adfsdm.c
index 66efb9a0acb9..73c6460a6327 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -360,11 +360,8 @@ static int stm32_adfsdm_probe(struct platform_device *pdev)
ret = devm_snd_soc_register_component(&pdev->dev,
&stm32_adfsdm_soc_platform,
NULL, 0);
- if (ret < 0) {
- dev_err(&pdev->dev, "%s: Failed to register PCM platform\n",
- __func__);
+ if (ret < 0)
return ret;
- }
pm_runtime_enable(&pdev->dev);
--
2.43.0
^ permalink raw reply related
* [PATCH 2/4] ASoC: stm: stm32_i2s: Drop redundant error messages
From: phucduc.bui @ 2026-07-20 7:40 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Olivier Moysan, Arnaud Pouliquen, Maxime Coquelin,
Alexandre Torgue
Cc: linux-stm32, linux-kernel, linux-sound, linux-arm-kernel,
bui duc phuc
In-Reply-To: <20260720074044.87528-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Both devm_request_irq() and snd_dmaengine_pcm_register() already log
failures internally. Drop the redundant error messages and return the
original errors directly.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
sound/soc/stm/stm32_i2s.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/stm/stm32_i2s.c b/sound/soc/stm/stm32_i2s.c
index ae9e25657f3f..83b51893b37c 100644
--- a/sound/soc/stm/stm32_i2s.c
+++ b/sound/soc/stm/stm32_i2s.c
@@ -1238,10 +1238,8 @@ static int stm32_i2s_parse_dt(struct platform_device *pdev,
ret = devm_request_irq(&pdev->dev, irq, stm32_i2s_isr, 0,
dev_name(&pdev->dev), i2s);
- if (ret) {
- dev_err(&pdev->dev, "irq request returned %d\n", ret);
+ if (ret)
return ret;
- }
/* Reset */
rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
@@ -1295,7 +1293,7 @@ static int stm32_i2s_probe(struct platform_device *pdev)
ret = snd_dmaengine_pcm_register(&pdev->dev, &stm32_i2s_pcm_config, 0);
if (ret)
- return dev_err_probe(&pdev->dev, ret, "PCM DMA register error\n");
+ return ret;
ret = snd_soc_register_component(&pdev->dev, &stm32_i2s_component,
i2s->dai_drv, 1);
--
2.43.0
^ permalink raw reply related
* [PATCH 3/4] ASoC: stm: stm32_sai_sub: Drop redundant error messages
From: phucduc.bui @ 2026-07-20 7:40 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Olivier Moysan, Arnaud Pouliquen, Maxime Coquelin,
Alexandre Torgue
Cc: linux-stm32, linux-kernel, linux-sound, linux-arm-kernel,
bui duc phuc
In-Reply-To: <20260720074044.87528-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Both devm_request_irq() and snd_dmaengine_pcm_register() already log
failures internally. Drop the redundant error messages and return the
original errors directly.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
sound/soc/stm/stm32_sai_sub.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index ea9e8bddd63f..9a201bdb306f 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -1696,19 +1696,15 @@ static int stm32_sai_sub_probe(struct platform_device *pdev)
ret = devm_request_irq(&pdev->dev, sai->pdata->irq, stm32_sai_isr,
IRQF_SHARED, dev_name(&pdev->dev), sai);
- if (ret) {
- dev_err(&pdev->dev, "IRQ request returned %d\n", ret);
+ if (ret)
goto err_unprepare_pclk;
- }
if (STM_SAI_PROTOCOL_IS_SPDIF(sai))
conf = &stm32_sai_pcm_config_spdif;
ret = snd_dmaengine_pcm_register(&pdev->dev, conf, 0);
- if (ret) {
- ret = dev_err_probe(&pdev->dev, ret, "Could not register pcm dma\n");
+ if (ret)
goto err_unprepare_pclk;
- }
ret = snd_soc_register_component(&pdev->dev, &stm32_component,
&sai->cpu_dai_drv, 1);
--
2.43.0
^ permalink raw reply related
* [PATCH 4/4] ASoC: stm: stm32_spdifrx: Drop redundant error messages
From: phucduc.bui @ 2026-07-20 7:40 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Olivier Moysan, Arnaud Pouliquen, Maxime Coquelin,
Alexandre Torgue
Cc: linux-stm32, linux-kernel, linux-sound, linux-arm-kernel,
bui duc phuc
In-Reply-To: <20260720074044.87528-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Both devm_request_irq() and snd_dmaengine_pcm_register() already log
failures internally. Drop the redundant error messages and return the
original errors directly.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
sound/soc/stm/stm32_spdifrx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/stm/stm32_spdifrx.c b/sound/soc/stm/stm32_spdifrx.c
index 2f83ca989e68..e0fef47a227e 100644
--- a/sound/soc/stm/stm32_spdifrx.c
+++ b/sound/soc/stm/stm32_spdifrx.c
@@ -970,10 +970,8 @@ static int stm32_spdifrx_probe(struct platform_device *pdev)
ret = devm_request_irq(&pdev->dev, spdifrx->irq, stm32_spdifrx_isr, 0,
dev_name(&pdev->dev), spdifrx);
- if (ret) {
- dev_err(&pdev->dev, "IRQ request returned %d\n", ret);
+ if (ret)
return ret;
- }
rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
if (IS_ERR(rst))
@@ -987,7 +985,7 @@ static int stm32_spdifrx_probe(struct platform_device *pdev)
pcm_config = &stm32_spdifrx_pcm_config;
ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, 0);
if (ret)
- return dev_err_probe(&pdev->dev, ret, "PCM DMA register error\n");
+ return ret;
ret = snd_soc_register_component(&pdev->dev,
&stm32_spdifrx_component,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] ARM: enable interrupts when arm_notify_die() is handling user mode errors
From: Sebastian Andrzej Siewior @ 2026-07-20 7:46 UTC (permalink / raw)
To: Xie Yuanbin
Cc: linux, rmk+kernel, arnd, clrkwllms, liaohua4, lilinjie8, linusw,
linux-arm-kernel, linux-kernel, linux-rt-devel, rostedt
In-Reply-To: <20260629094022.Ml4PCqsB@linutronix.de>
On 2026-06-29 11:40:22 [+0200], To Xie Yuanbin wrote:
> On 2026-06-29 16:31:50 [+0800], Xie Yuanbin wrote:
> > Thanks. So, can I take it that you agree with the first one?
I am going to apply the initial patch to my RT tree so it does not get
lost. This needs however to be routed via Russell.
Sebastian
^ permalink raw reply
* Re: [PATCH v7 6/7] mm/vmalloc: map contiguous pages in batches for vmap() if possible
From: Dev Jain @ 2026-07-20 7:50 UTC (permalink / raw)
To: David Hildenbrand (Arm), Wen Jiang, akpm, catalin.marinas,
linux-mm, urezki, will
Cc: Xueyuan.chen21, ajd, anshuman.khandual, linux-arm-kernel,
linux-kernel, rppt, ryan.roberts, baohua, Wen Jiang, Leo Yan
In-Reply-To: <fbea7f50-ee00-4466-8788-1cc78303f5bc@kernel.org>
[------]
>> +
>> + nr_contig = num_pages_contiguous(&pages[idx], max_steps);
>> + if (nr_contig < 2)
>> + return 0;
>> +
>> + order = ilog2(nr_contig);
>> + pfn = page_to_pfn(pages[idx]);
>> +
>> + /* Limit order by pfn alignment */
>> + if (pfn > 0)
>> + order = min_t(int, order, __ffs(pfn));
>
> Wouldn't it make sense to determine that before you call num_pages_contiguous?
> Because then, you can just scan to that maximum instead of the given nr_pages.
Right!
>
>> +
>> + if (vm_shift(prot, PAGE_SIZE << order) == PAGE_SHIFT)
>> + return 0;
>> +
>> + return order;
>> +}
>> +
>> +static int vmap_pages_range_batched(unsigned long addr, unsigned long end,
>> + pgprot_t prot, struct page **pages)
>> +{
>> + unsigned int count = (end - addr) >> PAGE_SHIFT;
>
> "nr_pages" ? Also, can be const.
>
>> + unsigned int prev_shift = 0, idx = 0;
>> + unsigned long map_addr = addr, batch_end = addr;
>> + int err;
>> +
>> + err = kmsan_vmap_pages_range_noflush(addr, end, prot, pages,
>> + PAGE_SHIFT, GFP_KERNEL);
>
> Is the indentation on the second parameter line off?
>
>> + if (err)
>> + goto out;
>> +
>> + for (unsigned int i = 0; i < count; ) {
>> + unsigned int shift = PAGE_SHIFT +
>> + get_vmap_batch_order(pages, prot, count - i, i);
>
> Having two indices, i and idx, is just confusing.
>
> The whole function is a bit overly complicated. Does it really buy us much to
> batch over vmap_pages_range_noflush_walk() calling with the same shift?
It will buy us more in the sense that, vmap() speed is sensitive to the number
of pagetable walks. I think that is reflected by the ioremap(1MB) 1.35x speedup
mentioned in the cover letter.
In terms of callers, not sure. I would expect a caller to get the highest
power of 2 from the total needed nr_pages, then get the next highest power
of 2 from the remaining, and so on.
For the arm64 TRBE usecase, the relevant code is the rb_alloc_aux_page() caller in
kernel/events/ring_buffer.c. We may get multiple same order requests if can't
get the first requested higher order.
Since on the cover letter I see Wen and team have tested on boards, I lean
towards saying it is reasonable to assume a usecase where such order
fallback can happen.
But yeah the code for this currently is not so nice I agree.
>
>> +
>> + if (!i)
>> + prev_shift = shift;
>> +
>> + if (shift != prev_shift) {
>> + err = vmap_pages_range_noflush_walk(map_addr, batch_end,
>> + prot, pages + idx, prev_shift);
>> + if (err)
>> + goto out;
>> + prev_shift = shift;
>> + map_addr = batch_end;
>> + idx = i;
>> + }
>> +
>> + /*
>> + * Once small pages are encountered, the remaining pages
>> + * are likely small as well.
>
> "small pages" is odd. Maybe
>
> "Once we fail to batch pages, we expect to fail batching for all remaining
> pages, so just give up."
>
>> + */
>> + if (shift == PAGE_SHIFT)
>> + break;
>> +
>> + batch_end += 1UL << shift;
>> + i += 1U << (shift - PAGE_SHIFT);
>> + }
>> +
>> + /* Remaining */
>> + if (map_addr < end)
>> + err = vmap_pages_range_noflush_walk(map_addr, end,
>> + prot, pages + idx, prev_shift);
>> +
>> +out:
>> + flush_cache_vmap(addr, end);
>> + return err;
>> +}
>> +
>> /**
>> * vmap - map an array of pages into virtually contiguous space
>> * @pages: array of page pointers
>> @@ -3600,8 +3678,8 @@ void *vmap(struct page **pages, unsigned int count,
>> return NULL;
>>
>> addr = (unsigned long)area->addr;
>> - if (vmap_pages_range(addr, addr + size, pgprot_nx(prot),
>> - pages, PAGE_SHIFT) < 0) {
>> + if (vmap_pages_range_batched(addr, addr + size, pgprot_nx(prot),
>> + pages) < 0) {
>
> Nit: single line would make that nicer to read.
>
^ permalink raw reply
* Re: [PATCH v7 5/7] mm/vmalloc: Extract vm_shift() to consolidate mapping shift selection
From: Dev Jain @ 2026-07-20 7:50 UTC (permalink / raw)
To: Wen Jiang, akpm, catalin.marinas, linux-mm, urezki, will
Cc: Xueyuan.chen21, ajd, anshuman.khandual, david, linux-arm-kernel,
linux-kernel, rppt, ryan.roberts, baohua, Wen Jiang, Leo Yan
In-Reply-To: <20260715120813.3609949-6-jiangwen6@xiaomi.com>
On 15/07/26 5:38 pm, Wen Jiang wrote:
> Extract the vmalloc mapping shift selection from
> __vmalloc_node_range_noprof() into vm_shift(), preparing for reuse by the
> vmap batching path.
>
> No functional change.
>
> Suggested-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> Tested-by: Leo Yan <leo.yan@arm.com>
> ---
Reviewed-by: Dev Jain <dev.jain@arm.com>
> mm/vmalloc.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index afc695d5973e8..a1e025120e9df 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3549,6 +3549,14 @@ void vunmap(const void *addr)
> }
> EXPORT_SYMBOL(vunmap);
>
> +static inline unsigned int vm_shift(pgprot_t prot, unsigned long size)
> +{
> + if (arch_vmap_pmd_supported(prot) && size >= PMD_SIZE)
> + return PMD_SHIFT;
> +
> + return arch_vmap_pte_supported_shift(size);
> +}
> +
> /**
> * vmap - map an array of pages into virtually contiguous space
> * @pages: array of page pointers
> @@ -4047,11 +4055,7 @@ void *__vmalloc_node_range_noprof(unsigned long size, unsigned long align,
> * supporting them.
> */
>
> - if (arch_vmap_pmd_supported(prot) && size >= PMD_SIZE)
> - shift = PMD_SHIFT;
> - else
> - shift = arch_vmap_pte_supported_shift(size);
> -
> + shift = vm_shift(prot, size);
> align = max(original_align, 1UL << shift);
> }
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH] regulator: mt6358: use regmap helper to read fixed LDO calibration
From: Chen-Yu Tsai @ 2026-07-20 7:56 UTC (permalink / raw)
To: Daniel Golle
Cc: Liam Girdwood, Mark Brown, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones, linux-kernel,
linux-arm-kernel, linux-mediatek
In-Reply-To: <dcd98d81dede338c9bbb9700a9613c848b702e49.1784336005.git.daniel@makrotopia.org>
On Sat, Jul 18, 2026 at 9:06 AM Daniel Golle <daniel@makrotopia.org> wrote:
>
> The "fixed" LDOs with output voltage calibration use
> mt6358_get_buck_voltage_sel as their get_voltage_sel op, but the
> MT6358_REG_FIXED and MT6366_REG_FIXED entries do not populate
> da_vsel_reg/da_vsel_mask. The op therefore reads register 0x0 with a
> zero mask and shifts the result by ffs(0) - 1 = -1, which is undefined
> behaviour and gets flagged by UBSAN on every boot on MT6366 boards:
>
> UBSAN: shift-out-of-bounds in drivers/regulator/mt6358-regulator.c:384:38
> shift exponent -1 is negative
> Call trace:
> mt6358_get_buck_voltage_sel+0xc8/0x120
> regulator_get_voltage_rdev+0x70/0x170
> set_machine_constraints+0x504/0xc38
> regulator_register+0x324/0xc68
>
> Besides the undefined shift, the returned selector is always 0, so the
> actual calibration offset programmed in <reg>_ANA_CON0 is never
> reported.
>
> The descriptor already carries the correct vsel_reg/vsel_mask (the
> ANA_CON0 calibration field), matching the regulator_set_voltage_sel_regmap
> op already in use. Read the selector back through
> regulator_get_voltage_sel_regmap instead.
>
> Fixes: cf08fa74c716 ("regulator: mt6358: Add output voltage fine tuning to fixed regulators")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Tested-by: Chen-Yu Tsai <wens@kernel.org>
I guess I wasn't looking hard enough when I did the original patch.
I can confirm that after this patch, the fixed LDOs like vxo22 on
Juniper reads out as 2.24V instead of 2.2V (and then gets corrected
down to 2.2V by the kernel).
> ---
> drivers/regulator/mt6358-regulator.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index f2bb3c1523ca..d6a0ec406b07 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -492,7 +492,7 @@ static const struct regulator_ops mt6358_volt_fixed_ops = {
> .list_voltage = regulator_list_voltage_linear,
> .map_voltage = regulator_map_voltage_linear,
> .set_voltage_sel = regulator_set_voltage_sel_regmap,
> - .get_voltage_sel = mt6358_get_buck_voltage_sel,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> .set_voltage_time_sel = regulator_set_voltage_time_sel,
> .enable = regulator_enable_regmap,
> .disable = regulator_disable_regmap,
> --
> 2.55.0
^ permalink raw reply
* Re: [PATCH v7 1/7] arm64/hugetlb: Extend batching of multiple CONT_PTE in a single PTE setup
From: David Hildenbrand (Arm) @ 2026-07-20 8:16 UTC (permalink / raw)
To: Wen Jiang, akpm, catalin.marinas, linux-mm, urezki, will
Cc: Xueyuan.chen21, ajd, anshuman.khandual, linux-arm-kernel,
linux-kernel, rppt, ryan.roberts, dev.jain, baohua, Wen Jiang,
Leo Yan
In-Reply-To: <20260715120813.3609949-2-jiangwen6@xiaomi.com>
On 7/15/26 13:08, Wen Jiang wrote:
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
>
> For sizes aligned to CONT_PTE_SIZE and smaller than PMD_SIZE,
> we can handle CONT_PTE_SIZE groups together.
>
> These additional sizes are mapping spans used by non-hugetlbfs(vmalloc)
> mm code, not new HugeTLB hstate sizes.
>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> Tested-by: Leo Yan <leo.yan@arm.com>
> Reviewed-by: Dev Jain <dev.jain@arm.com>
> ---
> arch/arm64/mm/hugetlbpage.c | 15 +++++++++++++++
Huh, why are we touching hugetlbpage.c, which is HUGETLB-only material:
$ cat arch/arm64/mm/Makefile | grep hugetlbpage
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
Something is off here, if vmalloc ends up reusing some of these helpers, they
should probably get moved out here into common arm64 code.
--
Cheers,
David
^ permalink raw reply
* [PATCH v2 0/4] arm: ras: Add DT frontend support for ARM RAS
From: Umang Chheda @ 2026-07-20 8:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Ruidong Tian, Tony Luck, Borislav Petkov,
Umang Chheda
Cc: devicetree, linux-kernel, linux-arm-msm, linux-acpi,
linux-arm-kernel, linux-edac, faruque.ansari, avaneesh.dwivedi
This series adds Device Tree support for the ARM64 RAS driver
introduced by Ruidong Tian [1]. The existing driver supports ACPI-based
platforms via the AEST table, this series extends it to DT-based
platforms without any changes to the core driver.
How to test with QEMU
--------------------------
Tian Ruidong's QEMU fork [2] emulates AEST MMIO error records on the
virt machine. To test the DT frontend:
1. Build QEMU:
git clone https://github.com/winterddd/qemu.git
cd qemu
git checkout c5e2d5dec9fd62ba622314c40bff0fbecb4dfb34
./configure --target-list=aarch64-softmmu
make -j$(nproc)
2. Build the kernel with:
CONFIG_ARM64_RAS_EXTN=y
CONFIG_RAS=y
CONFIG_ACPI_AEST=y
CONFIG_ARM64_RAS_DRIVER=y
CONFIG_ARM64_RAS_DT=y
3. Add the following DT node to your virt machine DTB. The QEMU
fork maps GIC error source at 0x017a00000 (SPI 0x0d).
ras-gic-dist@17a00000 {
compatible = "arm,ras-gic";
reg = <0x0 0x17a00000 0x0 0x10000>;
reg-names = "err-group";
arm,group-format = <ARM_RAS_GROUP_4K>;
arm,num-records = <1>;
arm,record-impl = /bits/ 64 <0x1>;
arm,status-reporting = /bits/ 64 <0x0>;
arm,gic-ref = <&gic>;
interrupts = <GIC_SPI 0x0d 0x04>;
interrupt-names = "fhi";
};
4. Boot QEMU with acpi=off:
./qemu-system-aarch64 \
-machine virt,accel=tcg,gic-version=3 \
-cpu cortex-a57 -m 2G -smp 4 \
-kernel Image -dtb virt-aest.dtb \
-append "console=ttyAMA0 acpi=off earlycon" \
-nographic
5. Verify probe:
dmesg | grep "DT RAS"
# Expected: DT RAS: registered 1 RAS error source(s) from DT
ls /sys/kernel/debug/aest/
6. Inject a CE error via the QEMU MMIO fault injection registers.
The QEMU device accepts 64-bit accesses only (use devmem with
the 64-bit width flag):
devmem 0x090e0808 64 0x40000001
This triggers QEMU's error_record_inj_write() which sets
ERR<n>STATUS.V=1 and asserts the IRQ. The kernel driver's
aest_irq_func() fires, reads the status, and logs:
arm64_ras: {2}[Hardware Error]: Hardware error from AEST gic.90e0000
arm64_ras: {2}[Hardware Error]: Error from GIC type 0x0 instance 0x8005
arm64_ras: {2}[Hardware Error]: ERR0FR: 0x48a5
arm64_ras: {2}[Hardware Error]: ERR0CTRL: 0x108
arm64_ras: {2}[Hardware Error]: ERR0STATUS: 0x40000001
Testing
-------
- Validated Processor error nodes/sources for L1-L2 and L3 caches error
on Qualcomm's lemans-evk and monaco-evk boards with DT boot.
- Validated CE and UE injection via debugfs soft_inject.
- Validated GIC and SMMU error sources on QEMU.
[1] https://lore.kernel.org/lkml/20260122094656.73399-1-tianruidong@linux.alibaba.com/
[2] https://github.com/winterddd/qemu/tree/error_record
---
Changes in v2:
- Rebased on top of Ruidong Tian's v7 RAS driver series and reworked
the DT frontend to match the new driver architecture.
- Moved RAS error source nodes to the DT root, removed the arm,aest
container node as suggested by Rob Herring.
- Dropped arm,processor-flags and arm,resource-type properties, these
are now inferred from the interrupt type and cache phandle
respectively.
- Renamed compatible strings from arm,aest-* to arm,ras-* and the
dt-bindings header from aest.h to arm-ras.h to avoid ACPI terminology
in DT bindings.
---
Umang Chheda (4):
dt-bindings: arm: ras: Introduce bindings for ARM RAS error sources
arm64: ras: Add Device Tree frontend
arm64: dts: qcom: monaco: add RAS error source nodes
arm64: dts: qcom: lemans: add RAS error source nodes
.../bindings/arm/arm,ras-error-source.yaml | 330 +++++++++++++++
arch/arm64/boot/dts/qcom/lemans.dtsi | 30 ++
arch/arm64/boot/dts/qcom/monaco.dtsi | 30 ++
drivers/ras/arm64/Kconfig | 25 +-
drivers/ras/arm64/Makefile | 2 +
drivers/ras/arm64/ras-of.c | 383 ++++++++++++++++++
include/dt-bindings/arm/arm-ras.h | 11 +
7 files changed, 806 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/arm,ras-error-source.yaml
create mode 100644 drivers/ras/arm64/ras-of.c
create mode 100644 include/dt-bindings/arm/arm-ras.h
--
2.34.1
^ permalink raw reply
* [PATCH v2 1/4] dt-bindings: arm: ras: Introduce bindings for ARM RAS error sources
From: Umang Chheda @ 2026-07-20 8:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Ruidong Tian, Tony Luck, Borislav Petkov,
Umang Chheda
Cc: devicetree, linux-kernel, linux-arm-msm, linux-acpi,
linux-arm-kernel, linux-edac, faruque.ansari, avaneesh.dwivedi
In-Reply-To: <20260720081954.1858180-1-umang.chheda@oss.qualcomm.com>
ARMv8 and later processors implement the RAS (Reliability,
Availability and Serviceability) extensions, exposing hardware
error records through a standardised register interface.
Add Device Tree bindings to describe RAS error sources.
Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
---
.../bindings/arm/arm,ras-error-source.yaml | 330 ++++++++++++++++++
include/dt-bindings/arm/arm-ras.h | 11 +
2 files changed, 341 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/arm,ras-error-source.yaml
create mode 100644 include/dt-bindings/arm/arm-ras.h
diff --git a/Documentation/devicetree/bindings/arm/arm,ras-error-source.yaml b/Documentation/devicetree/bindings/arm/arm,ras-error-source.yaml
new file mode 100644
index 000000000000..add7063a1a62
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,ras-error-source.yaml
@@ -0,0 +1,330 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,ras-error-source.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM RAS error source
+
+maintainers:
+ - Umang Chheda <umang.chheda@oss.qualcomm.com>
+
+description: |
+ ARMv8 and later processors implement the Reliability, Availability and
+ Serviceability (RAS) extensions. Hardware blocks that support RAS expose
+ one or more error records through a standardised register interface. Each
+ error record captures information about a detected hardware error (cache
+ ECC fault, TLB parity error, interconnect error, etc.) and can optionally
+ signal the OS via an interrupt.
+
+ Each DT node described by this binding represents one RAS error source —
+ a hardware block that exposes a set of error records. Error records are
+ accessed either through system registers (for processor-local resources
+ such as L1/L2 caches and TLBs) or through a memory-mapped register window
+ (for shared or off-core resources such as L3 caches, SMMUs and GICs).
+
+properties:
+ compatible:
+ description:
+ Identifies the class of hardware block this error source belongs to.
+ arm,ras-processor covers processor error sources (cache, TLB, etc.).
+ arm,ras-smmu covers SMMU error sources.
+ arm,ras-gic covers GIC error sources.
+ enum:
+ - arm,ras-processor
+ - arm,ras-smmu
+ - arm,ras-gic
+
+ reg:
+ description:
+ Register windows for this error source. When absent the error records
+ are accessed through system registers (ERRSELR_EL1 + ERX*_EL1).
+ When present, the first range is the primary error-record window;
+ additional named ranges are identified by reg-names.
+ minItems: 1
+ maxItems: 4
+
+ reg-names:
+ description:
+ Names for the optional additional register windows beyond the primary
+ error-record window. err-group is the error group status register
+ window (ERRGSR). fault-inject is the fault injection register window
+ (ERXPFG*). irq-config is the interrupt routing configuration window.
+ minItems: 1
+ maxItems: 3
+ items:
+ enum:
+ - err-group
+ - fault-inject
+ - irq-config
+
+ interrupts:
+ description:
+ Interrupts signalled by this error source. The first interrupt is the
+ Fault Handling Interrupt (FHI), fired when a corrected error counter
+ overflows or a deferred error is detected. The optional second
+ interrupt is the Error Recovery Interrupt (ERI), fired when an
+ uncorrected recoverable error is detected.
+ minItems: 1
+ maxItems: 2
+
+ interrupt-names:
+ description:
+ Names identifying the interrupts. "fhi" is the Fault Handling
+ Interrupt; "eri" is the optional Error Recovery Interrupt.
+ minItems: 1
+ maxItems: 2
+ items:
+ enum:
+ - fhi
+ - eri
+
+ arm,group-format:
+ description:
+ Page granularity of the memory-mapped error record group register
+ window. Determines the ioremap size and the number of error group
+ status registers (ERRGSR) available. Required when reg is present.
+ Use the ARM_RAS_GROUP_* constants from <dt-bindings/arm/arm-ras.h>.
+ 0 (ARM_RAS_GROUP_4K) is a 4 KiB window with 1 ERRGSR supporting up
+ to 64 records. 1 (ARM_RAS_GROUP_16K) is 16 KiB with 4 ERRGSRs and
+ up to 256 records. 2 (ARM_RAS_GROUP_64K) is 64 KiB with 14 ERRGSRs
+ and up to 896 records.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2]
+
+ arm,num-records:
+ description:
+ Total number of error records in this error source, including both
+ implemented and unimplemented slots.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+
+ arm,record-impl:
+ description:
+ Bitmap of implemented error records. Bit N set to 1 means error
+ record N is present and active in this error source. Bit N set to 0
+ means record N is not implemented and must be skipped. The array
+ length must equal the number of ERRGSRs implied by arm,group-format
+ (1 element for 4K, 4 for 16K, 14 for 64K). For system-register
+ nodes (no reg property) a single u64 element is used.
+ $ref: /schemas/types.yaml#/definitions/uint64-array
+ minItems: 1
+ maxItems: 14
+
+ arm,status-reporting:
+ description:
+ Bitmap indicating which implemented error records must be polled
+ directly by the OS. Bit N set to 1 means record N does not report
+ through the ERRGSR and must be polled by reading its ERX_STATUS
+ register directly in the interrupt handler. Bit N set to 0 means
+ record N reports its status through the ERRGSR and will be discovered
+ via the ERRGSR scan path. For system-register nodes (no reg property)
+ there is no ERRGSR, so every implemented record must be polled
+ directly; arm,status-reporting must equal arm,record-impl for all
+ system-register nodes. Array length as for arm,record-impl.
+ $ref: /schemas/types.yaml#/definitions/uint64-array
+ minItems: 1
+ maxItems: 14
+
+ arm,addressing-mode:
+ description:
+ Bitmap indicating the type of address reported in the error address
+ register (ERX_ADDR) for each error record. Bit N set to 0 means
+ record N reports a System Physical Address (SPA) that the OS can use
+ directly. Bit N set to 1 means record N reports a node-specific
+ Logical Address (LA) that requires platform-specific translation to
+ obtain a SPA. Array length as for arm,record-impl.
+ $ref: /schemas/types.yaml#/definitions/uint64-array
+ minItems: 1
+ maxItems: 14
+
+ # Processor error source properties (arm,ras-processor only)
+
+ cache:
+ description:
+ Phandle to the cache node (L1, L2, or L3) that this processor error
+ source monitors. The referenced node must have compatible = "cache"
+ and a cache-level property identifying the level in the hierarchy.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ # SMMU error source properties (arm,ras-smmu only)
+
+ iommus:
+ description:
+ Phandle to the SMMU node that this error source monitors.
+ maxItems: 1
+
+ # GIC error source properties (arm,ras-gic only)
+
+ arm,gic-ref:
+ description:
+ Phandle to the GIC node that this error source monitors.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+ - compatible
+ - arm,num-records
+ - arm,record-impl
+ - arm,status-reporting
+
+allOf:
+ - if:
+ required:
+ - reg
+ then:
+ required:
+ - arm,group-format
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: arm,ras-processor
+ then:
+ required:
+ - cache
+ properties:
+ cache: {}
+ else:
+ properties:
+ cache: false
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: arm,ras-smmu
+ then:
+ required:
+ - iommus
+ properties:
+ iommus: {}
+ else:
+ properties:
+ iommus: false
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: arm,ras-gic
+ then:
+ required:
+ - arm,gic-ref
+ properties:
+ arm,gic-ref: {}
+ else:
+ properties:
+ arm,gic-ref: false
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/arm/arm-ras.h>
+
+ / {
+ compatible = "qcom,sa8775p-ride", "qcom,sa8775p";
+ model = "Qualcomm Technologies, Inc. SA8775P RAS example";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ interrupt-parent = <&intc>;
+
+ intc: interrupt-controller@17100000 {
+ compatible = "arm,gic-v3";
+ reg = <0x0 0x17100000 0x0 0x10000>,
+ <0x0 0x17180000 0x0 0x100000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ };
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,armv8";
+ reg = <0x0 0x0>;
+ next-level-cache = <&l2_0>;
+
+ l2_0: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ cache-unified;
+ next-level-cache = <&l3_0>;
+
+ l3_0: l3-cache {
+ compatible = "cache";
+ cache-level = <3>;
+ cache-unified;
+ };
+ };
+ };
+ };
+
+ /*
+ * Per-PE L1/L2 cache RAS error source. System-register access,
+ * per-CPU PPI. Record 0 is implemented (arm,record-impl bit 0
+ * set). arm,status-reporting equals arm,record-impl because
+ * system-register nodes have no ERRGSR; record 0 must be polled.
+ */
+ ras-l1l2-0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <1>;
+ arm,record-impl = /bits/ 64 <0x1>;
+ arm,status-reporting = /bits/ 64 <0x1>;
+ cache = <&l2_0>;
+ interrupts = <GIC_PPI 0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+
+ ras-l3-cluster0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <2>;
+ arm,record-impl = /bits/ 64 <0x2>;
+ arm,status-reporting = /bits/ 64 <0x2>;
+ cache = <&l3_0>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+ };
+
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/arm/arm-ras.h>
+
+ / {
+ compatible = "qcom,sa8775p-ride", "qcom,sa8775p";
+ model = "Qualcomm Technologies, Inc. SA8775P RAS example";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ interrupt-parent = <&gic>;
+
+ /*
+ * GICv3 interrupt controller with RAS support.
+ */
+ gic: interrupt-controller@17b00000 {
+ compatible = "arm,gic-v3";
+ reg = <0x0 0x17b00000 0x0 0x10000>,
+ <0x0 0x17b60000 0x0 0x100000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ };
+
+ ras-gic-dist@17a00000 {
+ compatible = "arm,ras-gic";
+ reg = <0x0 0x17a00000 0x0 0x10000>;
+ reg-names = "err-group";
+ arm,group-format = <ARM_RAS_GROUP_4K>;
+ arm,num-records = <1>;
+ arm,record-impl = /bits/ 64 <0x1>;
+ arm,status-reporting = /bits/ 64 <0x0>;
+ arm,gic-ref = <&gic>;
+ interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+ };
diff --git a/include/dt-bindings/arm/arm-ras.h b/include/dt-bindings/arm/arm-ras.h
new file mode 100644
index 000000000000..c2f4e1f8243e
--- /dev/null
+++ b/include/dt-bindings/arm/arm-ras.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_ARM_RAS_H
+#define _DT_BINDINGS_ARM_RAS_H
+
+/* arm,group-format - error record group register window page size */
+#define ARM_RAS_GROUP_4K 0 /* 4 KiB, 1 ERRGSR */
+#define ARM_RAS_GROUP_16K 1 /* 16 KiB, 4 ERRGSRs */
+#define ARM_RAS_GROUP_64K 2 /* 64 KiB, 14 ERRGSRs */
+
+#endif /* _DT_BINDINGS_ARM_RAS_H */
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/4] arm64: ras: Add Device Tree frontend
From: Umang Chheda @ 2026-07-20 8:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Ruidong Tian, Tony Luck, Borislav Petkov,
Umang Chheda
Cc: devicetree, linux-kernel, linux-arm-msm, linux-acpi,
linux-arm-kernel, linux-edac, faruque.ansari, avaneesh.dwivedi
In-Reply-To: <20260720081954.1858180-1-umang.chheda@oss.qualcomm.com>
Add a Device Tree frontend for the ARM64 RAS driver, allowing it
to be used on platforms without ACPI firmware.
The error sources defined in DT are registered as arm64_ras platform
devices at boot. The frontend produces the same software fwnode properties
as the ACPI path, so the core driver probes DT and ACPI nodes identically
without any firmware-specific knowledge.
Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
---
drivers/ras/arm64/Kconfig | 25 ++-
drivers/ras/arm64/Makefile | 2 +
drivers/ras/arm64/ras-of.c | 383 +++++++++++++++++++++++++++++++++++++
3 files changed, 405 insertions(+), 5 deletions(-)
create mode 100644 drivers/ras/arm64/ras-of.c
diff --git a/drivers/ras/arm64/Kconfig b/drivers/ras/arm64/Kconfig
index dcdeaa216d67..8bdb219bc90f 100644
--- a/drivers/ras/arm64/Kconfig
+++ b/drivers/ras/arm64/Kconfig
@@ -1,16 +1,31 @@
# SPDX-License-Identifier: GPL-2.0
#
-# ARM Error Source Table Support
+# ARM RAS driver
#
# Copyright (c) 2025, Alibaba Group.
#
+config ARM64_RAS_DT
+ bool "ARM64 RAS Device Tree support"
+ depends on ARM64_RAS_EXTN && OF
+ help
+ Enable Device Tree support for the ARM64 RAS driver.
+
+ When selected, RAS error sources described in the Device Tree
+ (arm,ras-processor, arm,ras-smmu, arm,ras-gic etc) are registered
+ as platform devices at boot, allowing the driver to be used on
+ platforms without ACPI firmware.
+
config ARM64_RAS_DRIVER
tristate "ARM64 RAS Driver"
- depends on ARM64 && ACPI_AEST && RAS
+ depends on ARM64 && (ACPI_AEST || ARM64_RAS_DT) && RAS
help
- This is the RAS driver for the arm64 architecture. It depends on
- the Arm Error Source Table (AEST) to provide basic register and
- interrupt information.
+ This is the RAS driver for the arm64 architecture. It uses the
+ ARMv8 RAS extension register interface to discover, configure,
+ and handle hardware errors from processor caches, SMMUs, and GICs.
+
+ On ACPI systems the error source topology is provided by the AEST
+ ACPI table (requires ACPI_AEST). On Device Tree systems it is
+ provided by "arm,ras-*" nodes in the DT root (requires ARM64_RAS_DT).
If set, the kernel will report and process hardware errors.
diff --git a/drivers/ras/arm64/Makefile b/drivers/ras/arm64/Makefile
index 6897798f7314..aee2f9de37f6 100644
--- a/drivers/ras/arm64/Makefile
+++ b/drivers/ras/arm64/Makefile
@@ -7,3 +7,5 @@ arm64_ras-y += ras-sysfs.o
arm64_ras-y += ras-inject.o
arm64_ras-y += ras-cmn.o
arm64_ras-y += ras-storm.o
+
+obj-$(CONFIG_ARM64_RAS_DT) += ras-of.o
diff --git a/drivers/ras/arm64/ras-of.c b/drivers/ras/arm64/ras-of.c
new file mode 100644
index 000000000000..e1aa8e13c077
--- /dev/null
+++ b/drivers/ras/arm64/ras-of.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#include <linux/acpi_aest.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "DT RAS: " fmt
+
+/* Maximum number of software fwnode properties per RAS node */
+#define RAS_OF_MAX_PROPS 20
+
+static const unsigned long ras_of_type_processor __initconst =
+ ACPI_AEST_PROCESSOR_ERROR_NODE;
+static const unsigned long ras_of_type_smmu __initconst =
+ ACPI_AEST_SMMU_ERROR_NODE;
+static const unsigned long ras_of_type_gic __initconst =
+ ACPI_AEST_GIC_ERROR_NODE;
+
+static const struct of_device_id ras_of_match[] __initconst = {
+ { .compatible = "arm,ras-processor", .data = &ras_of_type_processor },
+ { .compatible = "arm,ras-smmu", .data = &ras_of_type_smmu },
+ { .compatible = "arm,ras-gic", .data = &ras_of_type_gic },
+ { }
+};
+
+static const char * const __initconst ras_of_irq_res_names[] = {
+ [ACPI_AEST_NODE_FAULT_HANDLING] = AEST_FHI_NAME,
+ [ACPI_AEST_NODE_ERROR_RECOVERY] = AEST_ERI_NAME,
+};
+
+static const char * const __initconst ras_of_dt_irq_names[] = {
+ "fhi", "eri"
+};
+
+static int __init ras_of_build_node_data(struct device_node *np,
+ u8 node_type, int index,
+ int fhi_virq,
+ u8 **data_out, u32 *size_out)
+{
+ *data_out = NULL;
+ *size_out = 0;
+
+ switch (node_type) {
+ case ACPI_AEST_PROCESSOR_ERROR_NODE: {
+ /*
+ * Allocate the processor header plus the cache sub-structure.
+ * resource_type is inferred from the presence of the 'cache'
+ * phandle: if present the resource is a cache, otherwise it
+ * is treated as a generic processor resource.
+ * flags (SHARED/GLOBAL) are inferred from the interrupt type:
+ * a PPI is per-PE (global); an SPI is shared across a cluster.
+ */
+ size_t total = sizeof(struct acpi_aest_processor) +
+ sizeof(struct acpi_aest_processor_cache);
+ struct acpi_aest_processor_cache *c;
+ struct acpi_aest_processor *proc;
+ struct device_node *cache_np;
+ u8 pflags;
+
+ proc = kzalloc(total, GFP_KERNEL);
+ if (!proc)
+ return -ENOMEM;
+
+ cache_np = of_parse_phandle(np, "cache", 0);
+ if (cache_np) {
+ proc->resource_type = ACPI_AEST_CACHE_RESOURCE;
+ c = (struct acpi_aest_processor_cache *)(proc + 1);
+ /*
+ * alloc_ras_node_name() reads c->cache_reference as
+ * the name disambiguator for shared/global nodes.
+ */
+ c->cache_reference = cache_np->phandle;
+ of_node_put(cache_np);
+ } else {
+ proc->resource_type = ACPI_AEST_GENERIC_RESOURCE;
+ }
+
+ /*
+ * Infer scope from the FHI interrupt type: a PPI is
+ * per-PE (set GLOBAL); an SPI is cluster-shared (set SHARED).
+ * alloc_ras_node_name() uses flags to choose the name format.
+ */
+ pflags = irq_is_percpu(fhi_virq) ?
+ ACPI_AEST_PROC_FLAG_GLOBAL :
+ ACPI_AEST_PROC_FLAG_SHARED;
+ proc->flags = pflags;
+ proc->processor_id = (u32)index;
+
+ *data_out = (u8 *)proc;
+ *size_out = (u32)total;
+ break;
+ }
+ case ACPI_AEST_SMMU_ERROR_NODE: {
+ struct acpi_aest_smmu *smmu;
+ struct device_node *smmu_np;
+
+ smmu = kzalloc_obj(smmu, GFP_KERNEL);
+ if (!smmu)
+ return -ENOMEM;
+
+ smmu_np = of_parse_phandle(np, "iommus", 0);
+ if (smmu_np) {
+ smmu->iort_node_reference = smmu_np->phandle;
+ of_node_put(smmu_np);
+ }
+
+ *data_out = (u8 *)smmu;
+ *size_out = sizeof(*smmu);
+ break;
+ }
+ case ACPI_AEST_GIC_ERROR_NODE: {
+ struct acpi_aest_gic *gic;
+ struct device_node *gic_np;
+
+ gic = kzalloc_obj(gic, GFP_KERNEL);
+ if (!gic)
+ return -ENOMEM;
+
+ gic_np = of_parse_phandle(np, "arm,gic-ref", 0);
+ if (gic_np) {
+ gic->instance_id = gic_np->phandle;
+ of_node_put(gic_np);
+ }
+ *data_out = (u8 *)gic;
+ *size_out = sizeof(*gic);
+ break;
+ }
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/*
+ * Determine the register access type from the DT node:
+ * no reg -> system-register access (ERRSELR_EL1 + ERX*_EL1)
+ * 1 range -> memory-mapped access
+ * 2+ ranges -> single-record memory-mapped access
+ */
+static u8 __init ras_of_interface_type(struct device_node *np)
+{
+ int addr_cells, size_cells, reg_len;
+
+ if (!of_property_present(np, "reg"))
+ return ACPI_AEST_NODE_SYSTEM_REGISTER;
+
+ addr_cells = of_n_addr_cells(np);
+ size_cells = of_n_size_cells(np);
+ if (addr_cells <= 0 || size_cells <= 0)
+ return ACPI_AEST_NODE_SYSTEM_REGISTER;
+
+ reg_len = of_property_count_elems_of_size(np, "reg", sizeof(u32));
+ if (reg_len <= 0)
+ return ACPI_AEST_NODE_SYSTEM_REGISTER;
+
+ if (reg_len <= addr_cells + size_cells)
+ return ACPI_AEST_NODE_MEMORY_MAPPED;
+
+ return ACPI_AEST_NODE_SINGLE_RECORD_MEMORY_MAPPED;
+}
+
+static resource_size_t __init ras_of_mem_size(u32 gfmt)
+{
+ switch (gfmt) {
+ case ACPI_AEST_NODE_GROUP_FORMAT_16K:
+ return SZ_16K;
+ case ACPI_AEST_NODE_GROUP_FORMAT_64K:
+ return SZ_64K;
+ default:
+ return SZ_4K;
+ }
+}
+
+static u32 __init ras_of_virq_to_gsiv(int virq)
+{
+ struct irq_data *irqd = irq_get_irq_data(virq);
+
+ return irqd ? (u32)irqd->hwirq : 0;
+}
+
+static int __init ras_of_attach_fwnode(struct device_node *np,
+ struct platform_device *pdev,
+ u8 node_type, u8 itype, u32 gfmt,
+ int index, int fhi_virq,
+ u32 fhi_gsiv, u32 eri_gsiv)
+{
+ u64 rec_impl[14] = { };
+ u64 stat_rep[14] = { };
+ u64 addr_mode[14] = { };
+ u64 err_group_base = 0, fault_inject_base = 0, irq_cfg_base = 0;
+ struct property_entry props[RAS_OF_MAX_PROPS] = { };
+ struct resource res;
+ u8 *node_data = NULL;
+ u32 node_data_size = 0;
+ u32 nrec = 1;
+ int group_len, p = 0, i, ret;
+
+ of_property_read_u32(np, "arm,num-records", &nrec);
+
+ switch (gfmt) {
+ case ACPI_AEST_NODE_GROUP_FORMAT_16K:
+ group_len = 4;
+ break;
+ case ACPI_AEST_NODE_GROUP_FORMAT_64K:
+ group_len = 14;
+ break;
+ default:
+ group_len = 1;
+ break;
+ }
+
+ of_property_read_u64_array(np, "arm,record-impl", rec_impl, group_len);
+ of_property_read_u64_array(np, "arm,status-reporting", stat_rep, group_len);
+ of_property_read_u64_array(np, "arm,addressing-mode", addr_mode, group_len);
+
+ /*
+ * DT binding: bit=1 means record IS implemented.
+ * ras-core.c uses for_each_clear_bit() on record_implemented, so
+ * bit=0 means implemented internally. Invert before storing.
+ */
+ for (i = 0; i < group_len; i++)
+ rec_impl[i] = ~rec_impl[i];
+
+ /* Named MMIO windows — only present on memory-mapped nodes */
+ if (itype != ACPI_AEST_NODE_SYSTEM_REGISTER) {
+ int idx;
+
+ idx = of_property_match_string(np, "reg-names", "err-group");
+ if (idx >= 0 && !of_address_to_resource(np, idx, &res))
+ err_group_base = res.start;
+
+ idx = of_property_match_string(np, "reg-names", "fault-inject");
+ if (idx >= 0 && !of_address_to_resource(np, idx, &res))
+ fault_inject_base = res.start;
+
+ idx = of_property_match_string(np, "reg-names", "irq-config");
+ if (idx >= 0 && !of_address_to_resource(np, idx, &res))
+ irq_cfg_base = res.start;
+ }
+
+ ret = ras_of_build_node_data(np, node_type, index, fhi_virq,
+ &node_data, &node_data_size);
+ if (ret)
+ return ret;
+
+ props[p++] = PROPERTY_ENTRY_U8("arm,node-type", node_type);
+ props[p++] = PROPERTY_ENTRY_U8("arm,interface-type", itype);
+ props[p++] = PROPERTY_ENTRY_U8("arm,group-format", (u8)gfmt);
+ props[p++] = PROPERTY_ENTRY_U32("arm,error-records-count", nrec);
+ props[p++] = PROPERTY_ENTRY_U32("arm,error-records-index", 0);
+ props[p++] = PROPERTY_ENTRY_U32("arm,interface-flags", 0);
+ props[p++] = PROPERTY_ENTRY_U64_ARRAY_LEN("arm,record-implemented",
+ rec_impl, group_len);
+ props[p++] = PROPERTY_ENTRY_U64_ARRAY_LEN("arm,status-reporting",
+ stat_rep, group_len);
+ props[p++] = PROPERTY_ENTRY_U64_ARRAY_LEN("arm,addressing-mode",
+ addr_mode, group_len);
+ props[p++] = PROPERTY_ENTRY_U64("arm,error-group-base", err_group_base);
+ props[p++] = PROPERTY_ENTRY_U64("arm,fault-inject-base", fault_inject_base);
+ props[p++] = PROPERTY_ENTRY_U64("arm,interrupt-config-base", irq_cfg_base);
+ props[p++] = PROPERTY_ENTRY_U32("arm,fhi-gsiv", fhi_gsiv);
+ props[p++] = PROPERTY_ENTRY_U32("arm,eri-gsiv", eri_gsiv);
+
+ if (node_data && node_data_size)
+ props[p++] = PROPERTY_ENTRY_U8_ARRAY_LEN("arm,node-specific-data",
+ node_data, node_data_size);
+
+ ret = device_create_managed_software_node(&pdev->dev, props, NULL);
+
+ kfree(node_data);
+ return ret;
+}
+
+static int __init ras_of_init_one_node(struct device_node *np, u8 node_type,
+ int index)
+{
+ struct resource res[AEST_MAX_INTERRUPT_PER_NODE + 1] = { };
+ struct platform_device *pdev;
+ u32 gfmt = ACPI_AEST_NODE_GROUP_FORMAT_4K;
+ u32 gsiv[AEST_MAX_INTERRUPT_PER_NODE] = { };
+ int virq[AEST_MAX_INTERRUPT_PER_NODE] = { };
+ int nres = 0, ret, i;
+ u8 itype;
+
+ itype = ras_of_interface_type(np);
+ of_property_read_u32(np, "arm,group-format", &gfmt);
+
+ pdev = platform_device_alloc("arm64_ras", PLATFORM_DEVID_AUTO);
+ if (!pdev)
+ return -ENOMEM;
+
+ if (itype != ACPI_AEST_NODE_SYSTEM_REGISTER) {
+ struct resource mmio_res;
+
+ ret = of_address_to_resource(np, 0, &mmio_res);
+ if (ret) {
+ pr_err("node %pOF: missing 'reg' for MMIO interface\n", np);
+ goto err_put;
+ }
+ res[nres].name = AEST_NODE_NAME;
+ res[nres].start = mmio_res.start;
+ res[nres].end = mmio_res.start + ras_of_mem_size(gfmt) - 1;
+ res[nres].flags = IORESOURCE_MEM;
+ nres++;
+ }
+
+ for (i = 0; i < AEST_MAX_INTERRUPT_PER_NODE; i++) {
+ int irq_num = of_irq_get_byname(np, ras_of_dt_irq_names[i]);
+
+ if (irq_num <= 0)
+ continue;
+
+ gsiv[i] = ras_of_virq_to_gsiv(irq_num);
+ virq[i] = irq_num;
+
+ res[nres].name = ras_of_irq_res_names[i];
+ res[nres].start = irq_num;
+ res[nres].end = irq_num;
+ res[nres].flags = IORESOURCE_IRQ;
+ nres++;
+ }
+
+ ret = platform_device_add_resources(pdev, res, nres);
+ if (ret)
+ goto err_put;
+
+ ret = ras_of_attach_fwnode(np, pdev, node_type, itype, gfmt, index,
+ virq[ACPI_AEST_NODE_FAULT_HANDLING],
+ gsiv[ACPI_AEST_NODE_FAULT_HANDLING],
+ gsiv[ACPI_AEST_NODE_ERROR_RECOVERY]);
+ if (ret)
+ goto err_put;
+
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto err_put;
+
+ pr_debug("registered RAS node %pOF as arm64_ras.%d\n", np, pdev->id);
+ return 0;
+
+err_put:
+ platform_device_put(pdev);
+ return ret;
+}
+
+static int __init ras_of_init(void)
+{
+ const struct of_device_id *match;
+ struct device_node *np;
+ int index = 0, ret;
+
+ if (!acpi_disabled)
+ return 0;
+
+ for_each_matching_node_and_match(np, ras_of_match, &match) {
+ u8 node_type = *(const unsigned long *)match->data;
+
+ ret = ras_of_init_one_node(np, node_type, index++);
+ if (ret) {
+ pr_err("failed to register RAS node %pOF: %d\n",
+ np, ret);
+ of_node_put(np);
+ return ret;
+ }
+ }
+
+ if (index)
+ pr_info("registered %d RAS error source(s) from DT\n", index);
+
+ return 0;
+}
+subsys_initcall_sync(ras_of_init);
--
2.34.1
^ permalink raw reply related
* [PATCH v2 4/4] arm64: dts: qcom: lemans: add RAS error source nodes
From: Umang Chheda @ 2026-07-20 8:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Ruidong Tian, Tony Luck, Borislav Petkov,
Umang Chheda
Cc: devicetree, linux-kernel, linux-arm-msm, linux-acpi,
linux-arm-kernel, linux-edac, faruque.ansari, avaneesh.dwivedi
In-Reply-To: <20260720081954.1858180-1-umang.chheda@oss.qualcomm.com>
Add RAS error source nodes for the Lemans SoC.
Two processor error sources are described: a per-PE node covering
the L1/L2 cache hierarchy using a PPI, and two shared L3 cache
nodes for the two CPU clusters using SPIs.
Co-developed-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
Signed-off-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/lemans.dtsi | 30 ++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
index 3b0539e27b51..64d3370cba82 100644
--- a/arch/arm64/boot/dts/qcom/lemans.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
@@ -619,6 +619,36 @@ system_pd: power-domain-system {
};
};
+ ras-l1l2-0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <1>;
+ arm,record-impl = /bits/ 64 <0x1>;
+ arm,status-reporting = /bits/ 64 <0x1>;
+ cache = <&l2_0>;
+ interrupts = <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "fhi";
+ };
+
+ ras-l3-cluster0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <2>;
+ arm,record-impl = /bits/ 64 <0x2>;
+ arm,status-reporting = /bits/ 64 <0x2>;
+ cache = <&l3_0>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+
+ ras-l3-cluster1 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <2>;
+ arm,record-impl = /bits/ 64 <0x2>;
+ arm,status-reporting = /bits/ 64 <0x2>;
+ cache = <&l3_1>;
+ interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/4] arm64: dts: qcom: monaco: add RAS error source nodes
From: Umang Chheda @ 2026-07-20 8:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Ruidong Tian, Tony Luck, Borislav Petkov,
Umang Chheda
Cc: devicetree, linux-kernel, linux-arm-msm, linux-acpi,
linux-arm-kernel, linux-edac, faruque.ansari, avaneesh.dwivedi
In-Reply-To: <20260720081954.1858180-1-umang.chheda@oss.qualcomm.com>
Add RAS error source nodes for the Monaco SoC.
Two processor error sources are described: a per-PE node covering the
L1/L2 cache hierarchy using a PPI, and two shared L3 cache nodes for
the two CPU clusters using SPIs.
Co-developed-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
Signed-off-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/monaco.dtsi | 30 ++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
index 64fc0d592282..4fbd1dc9a147 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -724,6 +724,36 @@ system_pd: power-domain-system {
};
};
+ ras-l1l2-0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <1>;
+ arm,record-impl = /bits/ 64 <0x1>;
+ arm,status-reporting = /bits/ 64 <0x1>;
+ cache = <&l2_0>;
+ interrupts = <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "fhi";
+ };
+
+ ras-l3-cluster0 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <2>;
+ arm,record-impl = /bits/ 64 <0x2>;
+ arm,status-reporting = /bits/ 64 <0x2>;
+ cache = <&l3_0>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+
+ ras-l3-cluster1 {
+ compatible = "arm,ras-processor";
+ arm,num-records = <2>;
+ arm,record-impl = /bits/ 64 <0x2>;
+ arm,status-reporting = /bits/ 64 <0x2>;
+ cache = <&l3_1>;
+ interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fhi";
+ };
+
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] cpufreq: apple-soc: Calculate frequency as a 64-bit value
From: Zhongqiu Han @ 2026-07-20 8:23 UTC (permalink / raw)
To: Sasha Finkelstein, Sven Peter, Janne Grunau, Neal Gompa,
Rafael J. Wysocki, Viresh Kumar
Cc: asahi, linux-arm-kernel, linux-pm, linux-kernel, zhongqiu.han
In-Reply-To: <20260720-cpufreq-64-v2-1-72bd9b4e5ca0@chaosmail.tech>
On 7/20/2026 3:25 PM, Sasha Finkelstein wrote:
> The current frequency calculation is done in 32 bit, causing problems
> if run on a future SoC that can boost higher than 4.2GHz. Ideally, we
> should use a true u64 instead of unsigned long and "knowning" that this
> only runs on 64 bit machines, but the core code uses ulong everywhere,
> so this should be good enough.
>
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---
> Changes in v2:
> - Minor style fixes
> - Link to v1: https://patch.msgid.link/20260703-cpufreq-64-v1-1-c406c705319a@chaosmail.tech
Looks good to me.
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> ---
> drivers/cpufreq/apple-soc-cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/apple-soc-cpufreq.c b/drivers/cpufreq/apple-soc-cpufreq.c
> index 638e5bf72185..5ad274a1e6ae 100644
> --- a/drivers/cpufreq/apple-soc-cpufreq.c
> +++ b/drivers/cpufreq/apple-soc-cpufreq.c
> @@ -288,7 +288,7 @@ static int apple_soc_cpufreq_init(struct cpufreq_policy *policy)
>
> /* Get OPP levels (p-state indexes) and stash them in driver_data */
> for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
> - unsigned long rate = freq_table[i].frequency * 1000 + 999;
> + unsigned long rate = freq_table[i].frequency * 1000UL + 999;
> struct dev_pm_opp *opp = dev_pm_opp_find_freq_floor(cpu_dev, &rate);
>
> if (IS_ERR(opp)) {
>
> ---
> base-commit: 4a50a141f05a8d1737661b19ee22ff8455b94409
> change-id: 20260703-cpufreq-64-2a23d7261e09
>
> Best regards,
> --
> Sasha Finkelstein <k@chaosmail.tech>
>
>
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply
* Re: [PATCH] nvme-apple: Remove redundant dev_err_probe()
From: Christoph Hellwig @ 2026-07-20 8:26 UTC (permalink / raw)
To: Pan Chuang
Cc: Sven Peter, Janne Grunau, Neal Gompa, Keith Busch, Jens Axboe,
Christoph Hellwig, Sagi Grimberg,
open list:ARM/APPLE MACHINE SUPPORT,
moderated list:ARM/APPLE MACHINE SUPPORT,
open list:NVM EXPRESS DRIVER, open list
In-Reply-To: <20260716033301.212797-1-panchuang@vivo.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v7 00/16] Support Armv8 RAS Extensions for Kernel-first error handling
From: Umang Chheda @ 2026-07-20 8:26 UTC (permalink / raw)
To: Catalin Marinas, Borislav Petkov
Cc: Ruidong Tian, Will Deacon, Lorenzo Pieralisi, Hanjun Guo,
Sudeep Holla, Rafael J . Wysocki, Len Brown, Tony Luck,
Thomas Gleixner, Peter Zijlstra, Robin Murphy, linux-acpi,
linux-arm-kernel, linux-kernel, linux-edac, zhuo.song,
oliver.yang, James Morse, Rob Herring
In-Reply-To: <ak6HPmosSUleJFYU@arm.com>
Hello All/Catalin,
On 7/8/2026 10:52 PM, Catalin Marinas wrote:
> On Tue, Jul 07, 2026 at 07:41:31PM -0700, Borislav Petkov wrote:
>> On Tue, Jun 02, 2026 at 03:15:23PM +0800, Ruidong Tian wrote:
>>> Documentation/ABI/testing/debugfs-arm64-ras | 87 ++
>>> MAINTAINERS | 11 +
>>> arch/arm64/include/asm/ras.h | 99 ++
>>> drivers/acpi/arm64/Kconfig | 10 +
>>> drivers/acpi/arm64/Makefile | 1 +
>>> drivers/acpi/arm64/aest.c | 423 ++++++++
>>> drivers/ras/Kconfig | 1 +
>>> drivers/ras/Makefile | 1 +
>>> drivers/ras/arm64/Kconfig | 16 +
>>> drivers/ras/arm64/Makefile | 9 +
>>> drivers/ras/arm64/ras-cmn.c | 479 +++++++++
>>> drivers/ras/arm64/ras-core.c | 1026 +++++++++++++++++++
>>> drivers/ras/arm64/ras-inject.c | 130 +++
>>> drivers/ras/arm64/ras-storm.c | 198 ++++
>>> drivers/ras/arm64/ras-sysfs.c | 319 ++++++
>>> drivers/ras/arm64/ras.h | 374 +++++++
>>> drivers/ras/debugfs.c | 3 +-
>>> drivers/ras/ras.c | 3 +
>>> include/linux/acpi_aest.h | 36 +
>>> include/linux/cpuhotplug.h | 1 +
>>> include/linux/ras.h | 10 +
>>> include/ras/ras_event.h | 79 ++
>>> 22 files changed, 3315 insertions(+), 1 deletion(-)
>>> create mode 100644 Documentation/ABI/testing/debugfs-arm64-ras
>>> create mode 100644 arch/arm64/include/asm/ras.h
>>> create mode 100644 drivers/acpi/arm64/aest.c
>>> create mode 100644 drivers/ras/arm64/Kconfig
>>> create mode 100644 drivers/ras/arm64/Makefile
>>> create mode 100644 drivers/ras/arm64/ras-cmn.c
>>> create mode 100644 drivers/ras/arm64/ras-core.c
>>> create mode 100644 drivers/ras/arm64/ras-inject.c
>>> create mode 100644 drivers/ras/arm64/ras-storm.c
>>> create mode 100644 drivers/ras/arm64/ras-sysfs.c
> [...]
>> But that's still not that important - what is even more important is who from
>> ARM is going to review and maintain this? I can take a look at the generic
>> bits so that the RAS stuff coexists fine in drivers/ras/ but I'd need an ARM
>> person to take care of this.
>>
>> There are enough on Cc so let's see if anyone moves.
>
> I will volunteer James Morse (he doesn't know yet) as the Arm RAS
> expert. He's away this week, hopefully can have a look when he gets
> back. For the ACPI bits, we have Lorenzo, Hanjun, Sudeep who should be
> able to review.
>
> Adding Rob Herring as well who's been giving some feedback on the DT
> counterpart:
>
> https://lore.kernel.org/all/20260505-aest-devicetree-support-v1-0-d5d6ffacf0a5@oss.qualcomm.com/
>
> I have some vague recollection James wanting to start with something
> simpler like EDAC and do DT first but I may be wrong or things could
> have changed since. So, let's wait for James to comment.
>
JYFI, I have v2 [1] of DT front-end support for ARM RAS.
https://lore.kernel.org/lkml/20260720081954.1858180-1-umang.chheda@oss.qualcomm.com/
Thanks,
Umang
^ permalink raw reply
* Re: [PATCH v8 3/3] thermal/drivers/imx: Add calibration offset support
From: Frieder Schrempf @ 2026-07-20 8:27 UTC (permalink / raw)
To: CHENG Haoning (BCSC/ENG1)
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <PAWPR10MB7415DE998EA3CCA639F14EF2C2C32@PAWPR10MB7415.EURPRD10.PROD.OUTLOOK.COM>
On 20.07.26 04:10, CHENG Haoning (BCSC/ENG1) wrote:
> [Sie erhalten nicht h?ufig E-Mails von haoning.cheng@cn.bosch.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
>
> On 2026-07-17 06:23:44+00:00, CHENG Haoning (BCSC/ENG1) wrote:
>> On Wed, Jul 15, 2026 at 11:06:07AM +0200, Frieder Schrempf wrote:
>>
>>> On 14.07.26 12:28, Haoning CHENG via B4 Relay wrote:
>>>
>>> Sorry to chime in so late. I just want to understand what this
>>> calibration offset is about. Why would there be a need of a
>>> board-specific offset? The sensor is in the SoC and if you add a
>>> board-specific offset, you no longer measure the SoC core temperature,
>>> right?
>>>
>>> How would you determine the offset in the first place? How would I know
>>> what fsl,temp-calibration-offset-millicelsius should be set to? I could
>>> put a sensor on the SoC case and use the delta as offset, but then I
>>> would just account for the thermal resistance of the casing and not
>>> measure the SoC core temperature anymore, right?
>>
>> Hi Frieder,
>>
>> Thanks for pointing this out. Your understanding is correct: after
>> applying the offset, the reported value no longer represents the raw
>> SoC die or junction temperature.
>>
>> For this board, the required "SoC temperature" is the package-surface
>> temperature. The offset was derived by comparing the TEMPMON reading
>> against a calibrated external sensor placed near the SoC package
>> surface under steady-state thermal conditions, and is used to
>> approximate the package-surface temperature from the internal TEMPMON
>> reading.
>>
>> The Linux thermal framework does not require a thermal zone to use a
>> specific temperature reference. It only requires the reported
>> temperature and trip points to use the same temperature domain.
>>
>> In this driver, the offset is added in get_temp() and subtracted in
>> set_alarm() and set_panic() when programming the hardware thresholds.
>> This keeps the reported temperature and trip points in the same
>> package-surface temperature domain. Doing this in the driver is
>> necessary because the hardware alarm thresholds must also be offset-
>> corrected; applying the offset only in userspace would cause the
>> TEMPMON IRQ to fire at the wrong die temperature.
>>
>> I agree that "calibration offset" is misleading, since this does not
>> calibrate TEMPMON to a more accurate junction temperature. In the next
>> revision, the commit messages and DT binding have been updated to
>> describe this as a board-specific conversion offset from the internal
>> sensor reading to an estimated package-surface temperature.
>>
>> Thanks,
>> Haoning
> Hi Frieder,
>
> I need to correct my previous reply. After further discussion with our hardware team, the offset does NOT convert die temperature to package-surface temperature.
>
> Their thermal characterization shows that the raw TEMPMON sensor reading itself deviates from the theoretical junction temperature - this deviation exists even at the die level, before any board or package influence. The offset corrects the sensor reading toward the actual junction temperature, so it is a genuine sensor calibration.
>
> Board-level effects may contribute additionally, but they are not the primary reason for this feature. I apologize for the confusion - my earlier description was inaccurate. The v10 series reflects this corrected understanding.
Thanks for the update. If that is the case, then I would still be
interested to know:
1. How is the value for fsl,temp-calibration-offset-millicelsius
determined, if it is not provided by the SoC vendor.
2. Why do we need an additional offset, if there is already a
calibration using the value that NXP stores in the OTPs.
3. Who is supposed to set this property and where does it belong? The
SoC dtsi, the board dts?
Thanks
Frieder
^ permalink raw reply
* RE: [PATCH v2] PCI: imx6: Fix i.MX6Q/DL boot hang by separating PHY power and reference clock control
From: Hongxing Zhu (OSS) @ 2026-07-20 8:32 UTC (permalink / raw)
To: Hongxing Zhu (OSS), Manivannan Sadhasivam, Hongxing Zhu (OSS)
Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, robh@kernel.org, bhelgaas@google.com,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
imx@lists.linux.dev, linux-kernel@vger.kernel.org, Hongxing Zhu
In-Reply-To: <GV2PR04MB1201914A80BB2BF78A10465748CC62@GV2PR04MB12019.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Sent: Friday, July 17, 2026 4:57 PM
> To: Manivannan Sadhasivam <mani@kernel.org>; Hongxing Zhu (OSS)
> <hongxing.zhu@oss.nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; robh@kernel.org; bhelgaas@google.com;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org; Hongxing Zhu <hongxing.zhu@nxp.com>
> Subject: RE: [PATCH v2] PCI: imx6: Fix i.MX6Q/DL boot hang by separating PHY
> power and reference clock control
>
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@kernel.org>
> > Sent: Friday, July 17, 2026 12:35 AM
> > To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kwilczynski@kernel.org; robh@kernel.org;
> > bhelgaas@google.com; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux- pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org; Hongxing Zhu <hongxing.zhu@nxp.com>
> > Subject: Re: [PATCH v2] PCI: imx6: Fix i.MX6Q/DL boot hang by
> > separating PHY power and reference clock control
> >
> > On Wed, Jul 08, 2026 at 11:59:27AM +0800, hongxing.zhu@oss.nxp.com wrote:
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > >
> > > Commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling
> > > regulators") introduced a boot hang on i.MX6Q/DL variants by
> > > changing the initialization sequence.
> > >
> > > The issue stems from coupling PHY power (TEST_PD) and reference
> > > clock
> > > (REF_CLK_EN) control in imx6q_pcie_enable_ref_clk(). When these are
> > > managed together, the timing between PHY power-up and reference
> > > clock enablement cannot be properly controlled, leading to
> > > initialization failures.
> > >
> >
> > What is the timing requirement here?
> The timing requirement is that TEST_PD must be deasserted (cleared) before link
> training starts.
>
> Before commit 610fa91d9863:
> - imx_pcie_assert_core_reset(): Assert TEST_PD and REF_CLK_EN
> - imx_pcie_clk_enable(): Deassert TEST_PD and assert REF_CLK_EN
> - Link training starts with TEST_PD properly cleared
>
> After commit 610fa91d9863:
> - imx_pcie_clk_enable(): Deassert TEST_PD and assert REF_CLK_EN
> - imx_pcie_assert_core_reset(): Assert TEST_PD and assert REF_CLK_EN again
> - Link training starts with TEST_PD still asserted (never cleared again)
>
> This commit corrects the sequence, and makes sure the TEST_PD is cleared
> before link training starts.
> >
> > > Fix this by separating the two concerns:
> > >
> > > - Move PHY power control (TEST_PD) to imx6q_pcie_core_reset() where it
> > > logically belongs with reset operations. This ensures PHY power state
> > > is managed as part of the core reset sequence.
> > >
> > > - Update imx6qp_pcie_core_reset() to call imx6q_pcie_core_reset() for
> > > shared PHY power management, avoiding code duplication.
> > >
> > > - Make imx6q_pcie_enable_ref_clk() responsible only for reference clock
> > > (REF_CLK_EN) control, simplifying its purpose.
> > >
> > > - Remove the 10us delay workaround from imx6q_pcie_enable_ref_clk() as
> > > proper sequencing is now handled by the core_reset functions.
> > >
> > > This refactoring ensures PHY power is controlled during reset
> > > operations, fixing the boot hang while improving code maintainability.
> > >
> >
> > This patch does too many things at once. Can't you split it and keep
> > the minimal fix in one patch?
> Okay, I'll split this into a patch series in v3.
> Thanks.
Hi Mani:
I've attempted to split the changes as below:
1. Set/Clear TEST_PD in assert_core_reset()/deassert_core_reset()
2. Clean up imx6q_pcie_enable_ref_clk() to only manipulate the REF_CLK_EN bit
However, I found that patch 1 alone doesn't work correctly. Here's what happens:
With only patch 1 applied:
- The board boots successfully, but fails to detect the remote endpoint device
- Problem sequence:
Begin (TEST_PD asserted by default)
→ TEST_PD cleared + REF_CLK_EN asserted in clk_enable()
→ TEST_PD asserted again in assert_core_reset()
→ TEST_PD cleared in deassert_core_reset()
With both patches applied:
- The board boots and detects the remote endpoint device successfully
- Correct sequence:
Begin (TEST_PD asserted by default)
→ REF_CLK_EN asserted in clk_enable() (TEST_PD remains untouched)
→ TEST_PD asserted in assert_core_reset()
→ TEST_PD cleared in deassert_core_reset()
The issue is that patch 1 relies on patch 2 to avoid prematurely clearing TEST_PD
in clk_enable(). Both changes are mandatory for the fix to work.
Given this dependency, would you prefer:
- A two-patch series with the dependency clearly documented, or
- A single combined patch since they cannot function independently.
Thanks.
Best Regards
Richard Zhu
>
> Best Regards
> Richard Zhu
> >
> > - Mani
> >
> > > Fixes: 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling
> > > regulators")
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > > Changes in v2:
> > > Regarding sashiko's reivew, invoke imx_pcie_assert_core_reset()
> > > explicitly in error path of imx_pcie_host_init() and imx_pcie_host_exit().
> > > ---
> > > drivers/pci/controller/dwc/pci-imx6.c | 45
> > > ++++++++++++---------------
> > > 1 file changed, 20 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 9406bba36953f..53f3da6ab30d5 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -680,21 +680,12 @@ static int imx_pcie_attach_pd(struct device
> > > *dev)
> > >
> > > static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie,
> > > bool
> > > enable) {
> > > - if (enable) {
> > > - /* power up core phy and enable ref clock */
> > > - regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_TEST_PD);
> > > - /*
> > > - * The async reset input need ref clock to sync internally,
> > > - * when the ref clock comes after reset, internal synced
> > > - * reset time is too short, cannot meet the requirement.
> > > - * Add a ~10us delay here.
> > > - */
> > > - usleep_range(10, 100);
> > > - regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_REF_CLK_EN);
> > > - } else {
> > > - regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_REF_CLK_EN);
> > > - regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_TEST_PD);
> > > - }
> > > + if (enable)
> > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > + IMX6Q_GPR1_PCIE_REF_CLK_EN);
> > > + else
> > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > + IMX6Q_GPR1_PCIE_REF_CLK_EN);
> > >
> > > return 0;
> > > }
> > > @@ -823,23 +814,25 @@ static int imx6sx_pcie_core_reset(struct
> > > imx_pcie
> > *imx_pcie, bool assert)
> > > return 0;
> > > }
> > >
> > > -static int imx6qp_pcie_core_reset(struct imx_pcie *imx_pcie, bool
> > > assert)
> > > +static int imx6q_pcie_core_reset(struct imx_pcie *imx_pcie, bool
> > > +assert)
> > > {
> > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_SW_RST,
> > > - assert ? IMX6Q_GPR1_PCIE_SW_RST : 0);
> > > - if (!assert)
> > > - usleep_range(200, 500);
> > > + if (assert)
> > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > + IMX6Q_GPR1_PCIE_TEST_PD);
> > > + else
> > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > + IMX6Q_GPR1_PCIE_TEST_PD);
> > >
> > > return 0;
> > > }
> > >
> > > -static int imx6q_pcie_core_reset(struct imx_pcie *imx_pcie, bool
> > > assert)
> > > +static int imx6qp_pcie_core_reset(struct imx_pcie *imx_pcie, bool
> > > +assert)
> > > {
> > > + imx6q_pcie_core_reset(imx_pcie, assert);
> > > + regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_SW_RST,
> > > + assert ? IMX6Q_GPR1_PCIE_SW_RST : 0);
> > > if (!assert)
> > > - return 0;
> > > -
> > > - regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_TEST_PD);
> > > - regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > IMX6Q_GPR1_PCIE_REF_CLK_EN);
> > > + usleep_range(200, 500);
> > >
> > > return 0;
> > > }
> > > @@ -1445,6 +1438,7 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> *pp)
> > > return 0;
> > >
> > > err_phy_off:
> > > + imx_pcie_assert_core_reset(imx_pcie);
> > > phy_power_off(imx_pcie->phy);
> > > err_phy_exit:
> > > phy_exit(imx_pcie->phy);
> > > @@ -1471,6 +1465,7 @@ static void imx_pcie_host_exit(struct
> > > dw_pcie_rp
> > *pp)
> > > dev_err(pci->dev, "unable to power off PHY\n");
> > > phy_exit(imx_pcie->phy);
> > > }
> > > + imx_pcie_assert_core_reset(imx_pcie);
> > > imx_pcie_clk_disable(imx_pcie);
> > >
> > > pci_pwrctrl_power_off_devices(pci->dev);
> > > --
> > > 2.34.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
^ 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