* Re: [PATCH v2 1/3] dt-bindings: phy: sc8280xp-qmp-pcie: Add vdda-refgen supply for Glymur
From: Krzysztof Kozlowski @ 2026-06-24 7:34 UTC (permalink / raw)
To: Qiang Yu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, linux-arm-msm,
linux-phy, devicetree, linux-kernel
In-Reply-To: <20260623-phy_refgen-v2-1-4d15983bf91d@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 06:05:17AM -0700, Qiang Yu wrote:
> The PCIe QMP PHYs require a stable reference voltage provided by REFGEN,
> which in turn requires two separate LDOs to operate.
>
> Add vdda-refgen0p9-supply and vdda-refgen1p2-supply properties. Mark them
> as required for the Glymur PCIe QMP PHYs for now; other platforms having
> the same requirement and can be added later.
>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
> .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: arm: qcom,ids: Add SoC ID for Snapdragon SDA 850
From: Konrad Dybcio @ 2026-06-24 7:35 UTC (permalink / raw)
To: Krzysztof Kozlowski, david, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <6ddd0652-d18e-4e23-a230-5f5c8a9756f8@kernel.org>
On 6/24/26 8:26 AM, Krzysztof Kozlowski wrote:
> On 23/06/2026 20:41, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> Add SoC ID for Qualcomm Snapdragon SDA850.
>>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>> Will be used by Comma three.
>
> Can you provide a link?
Previous gen of https://comma.ai/shop/comma-four, they don't seem
to have the old one listed on their website anymore
The other computer tells me SDA850 doesn't exist and 348 is SDM850
(for which we have a number of DTs in the kernel today), so let's
rename it and add a second patch to add it to socinfo
Konrad
^ permalink raw reply
* Re: [PATCH v6 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device
From: sashiko-bot @ 2026-06-24 7:36 UTC (permalink / raw)
To: Nas Chung; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260624072043.238-8-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Ignored error return in set_cur_state callback causes thermal framework desynchronization.
- [Medium] Initial thermal state changes during registration are silently dropped due to a check for `!thermal->cooling`.
--
commit f2e1f0ee6d5cf6a10bb958433178a8d9e3143413
Author: Nas Chung <nas.chung@chipsnmedia.com>
media: chips-media: wave6: Add Wave6 thermal cooling device
Add a thermal cooling device for the Wave6 VPU. The device operates within
the Linux thermal framework, adjusting the VPU performance state based on
thermal conditions.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
> new file mode 100644
> index 0000000000000..91cd826e81195
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
[ ... ]
> +static int wave6_vpu_thermal_cooling_update(struct vpu_thermal_cooling *thermal,
> + int state)
> +{
> + unsigned long new_clock_rate;
> + int ret;
> +
> + if (state > thermal->thermal_max || !thermal->cooling)
> + return 0;
[Severity: Medium]
Will this silently drop initial thermal state changes during registration?
When devm_thermal_of_cooling_device_register() is called, it can synchronously
trigger a state update before the registration function returns and assigns the
pointer to thermal->cooling. Because thermal->cooling is not yet assigned,
this check evaluates to true and skips the hardware update.
[ ... ]
> +static int wave6_vpu_cooling_set_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long state)
> +{
> + struct vpu_thermal_cooling *thermal = cdev->devdata;
> +
> + thermal->thermal_event = state;
> + wave6_vpu_thermal_cooling_update(thermal, state);
> +
> + return 0;
> +}
[Severity: Medium]
Should the return value of wave6_vpu_thermal_cooling_update() be checked before
returning success to the thermal framework?
If wave6_vpu_thermal_cooling_update() fails, this function still unconditionally
returns 0 and updates the local thermal->thermal_event state. This could leave
the thermal framework thinking the hardware state has changed when the hardware
is actually stuck in its previous state.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=7
^ permalink raw reply
* Re: [PATCH v6 6/9] media: chips-media: wave6: Improve debugging capabilities
From: sashiko-bot @ 2026-06-24 7:36 UTC (permalink / raw)
To: Nas Chung; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260624072043.238-7-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Kernel panic due to division by zero in debugfs FPS calculation.
- [High] Lockless access to `vb2_queue` internals causes Use-After-Free during concurrent `REQBUFS(0)`.
- [Medium] Checking `IS_ERR_OR_NULL` on an optional debugfs dentry inappropriately triggers `WARN_ON` when debugfs is disabled.
- [Medium] 32-bit integer multiplication overflow when calculating FPS statistics.
- [Medium] Data truncation of 64-bit DMA addresses in trace events.
- [Low] Use of 64-bit division and modulo operators breaks compilation on 32-bit architectures.
- [Low] Incorrect chaining of assignments with comma operators in `TP_fast_assign`.
--
commit 59a3991812bdf6b7f45f197b804c2beea6a0e9aa
Author: Nas Chung <nas.chung@chipsnmedia.com>
Subject: media: chips-media: wave6: Improve debugging capabilities
Add debugfs entries and trace events to provide detailed
debugging information.
These enhancements help diagnose issues and improve debugging
capabilities for the Wave6 core driver.
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Tested-by: Ming Qian <ming.qian@oss.nxp.com>
Tested-by: Marek Vasut <marek.vasut@mailbox.org>
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-trace.h b/drivers/media/platform/chips-media/wave6/wave6-trace.h
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-trace.h
[ ... ]
> +TRACE_EVENT(wave6_vpu_source_change,
> + TP_PROTO(struct vpu_instance *inst, struct dec_seq_info *info),
> + TP_ARGS(inst, info),
[ ... ]
> + TP_fast_assign(__assign_str(name);
> + __entry->id = inst->id;
> + __entry->width = info->pic_width,
> + __entry->height = info->pic_height,
[Severity: Low]
Is there a typo in these assignments? The width and height fields are
terminated with commas instead of semicolons inside TP_fast_assign.
> + __entry->profile = info->profile,
> + __entry->level = info->level;
[ ... ]
> +TRACE_EVENT(wave6_vpu_enc_pic,
> + TP_PROTO(struct vpu_instance *inst, struct enc_param *param),
> + TP_ARGS(inst, param),
> + TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
> + __field(u32, id)
> + __field(u32, srcidx)
> + __field(u32, buf_y)
[Severity: Medium]
Does this truncate 64-bit physical addresses?
Variables like buf_y and buf_cb are dma_addr_t, which can be 64-bit on
platforms like arm64. Declaring these fields as u32 will lose the upper
32 bits of the address in the trace output.
> + __field(u32, buf_cb)
> + __field(u32, buf_cr)
> + __field(u32, stride)
[ ... ]
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
[ ... ]
> +static int wave6_vpu_dbg_instance(struct seq_file *s, void *data)
> +{
> + struct vpu_instance *inst = s->private;
> + struct vpu_performance_info *perf = &inst->performance;
> + struct vb2_queue *vq;
[ ... ]
> + vq = v4l2_m2m_get_src_vq(inst->v4l2_fh.m2m_ctx);
> + seq_printf(s, "output (%2d, %2d): fmt = %c%c%c%c %d x %d, %d;\n",
> + vb2_is_streaming(vq),
> + vb2_get_num_buffers(vq),
[Severity: High]
Can this lead to a use-after-free?
This accesses the queue state locklessly in wave6_vpu_dbg_instance(). If a
concurrent userspace thread calls VIDIOC_REQBUFS with count=0, the buffers
bitmap is freed.
If the lockless read evaluates vb2_get_num_buffers() at the same time, it
can race against the release and cause bitmap_weight() to read uninitialized
or freed memory.
> + inst->src_fmt.pixelformat,
[ ... ]
> + seq_puts(s, "fps");
> + tmp = MSEC_PER_SEC * inst->processed_buf_num;
[Severity: Medium]
Can this multiplication overflow on 32-bit architectures?
MSEC_PER_SEC and inst->processed_buf_num are both 32-bit integers. The result
will wrap around after about 4.29 million frames before the implicit cast to
the 64-bit tmp variable.
> + if (perf->ts_last > perf->ts_first + NSEC_PER_MSEC) {
> + fps = DIV_ROUND_CLOSEST(tmp, (perf->ts_last - perf->ts_first) / NSEC_PER_MSEC);
[Severity: Low]
Will this break the build on 32-bit platforms?
The calculation performs direct division using the / operator on 64-bit types,
which results in undefined references to __divdi3. Should this use a kernel
math helper like div64_s64() instead?
> + seq_printf(s, " actual: %lld;", fps);
> + }
> + if (perf->total_sw_time) {
> + fps = DIV_ROUND_CLOSEST(tmp, perf->total_sw_time / NSEC_PER_MSEC);
[Severity: High]
What happens if total_sw_time or total_hw_time is non-zero but less than
1,000,000 nanoseconds?
The division perf->total_sw_time / NSEC_PER_MSEC will evaluate to zero.
Passing zero as the divisor to DIV_ROUND_CLOSEST causes an immediate
division by zero exception and a kernel panic.
> + seq_printf(s, " sw: %lld;", fps);
> + }
> + if (perf->total_hw_time) {
> + fps = DIV_ROUND_CLOSEST(tmp, perf->total_hw_time / NSEC_PER_MSEC);
> + seq_printf(s, " hw: %lld", fps);
> + }
[ ... ]
> +int wave6_vpu_create_dbgfs_file(struct vpu_instance *inst)
> +{
> + char name[64];
> +
> + if (WARN_ON(!inst || !inst->dev || IS_ERR_OR_NULL(inst->dev->debugfs)))
> + return -EINVAL;
[Severity: Medium]
Is it intended to trigger a warning when debugfs is disabled?
If CONFIG_DEBUG_FS is disabled, debugfs_create_dir() returns ERR_PTR(-ENODEV).
Using IS_ERR_OR_NULL() here will trigger the WARN_ON() every time a new
encoder or decoder instance is created, dumping a stack trace into the kernel
log.
> +
> + scnprintf(name, sizeof(name), "instance.%d", inst->id);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=6
^ permalink raw reply
* Re: [PATCH v6 07/21] RISC-V: Add Sscfg extension CSR definition
From: Atish Patra @ 2026-06-24 7:36 UTC (permalink / raw)
To: Charlie Jenkins
Cc: James Clark, Rob Herring, Arnaldo Carvalho de Melo, Jiri Olsa,
Will Deacon, Mark Rutland, Anup Patel, Namhyung Kim,
Paul Walmsley, Krzysztof Kozlowski, Ian Rogers, linux-riscv,
linux-kernel, linux-perf-users, Conor Dooley, devicetree,
linux-arm-kernel
In-Reply-To: <ajjZkRxoh2F3vUAS@blinky>
On 6/21/26 11:43 PM, Charlie Jenkins wrote:
> On Mon, Jun 08, 2026 at 11:01:21PM -0700, Atish Patra wrote:
>> From: Kaiwen Xue <kaiwenx@rivosinc.com>
>>
>> This adds the scountinhibit CSR definition and S-mode accessible hpmevent
>> bits defined by smcdeleg/ssccfg. scountinhibit allows S-mode to start/stop
>> counters directly from S-mode without invoking SBI calls to M-mode. It is
>> also used to figure out the counters delegated to S-mode by the M-mode as
>> well.
>>
>> Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
>> Reviewed-by: Clément Léger <cleger@rivosinc.com>
>> ---
>> arch/riscv/include/asm/csr.h | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> index b4551a6cf7cb..26cb78dee2fd 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -241,6 +241,31 @@
>> #define SMSTATEEN0_HSENVCFG (_ULL(1) << SMSTATEEN0_HSENVCFG_SHIFT)
>> #define SMSTATEEN0_SSTATEEN0_SHIFT 63
>> #define SMSTATEEN0_SSTATEEN0 (_ULL(1) << SMSTATEEN0_SSTATEEN0_SHIFT)
>> +/* HPMEVENT bits. These are accessible in S-mode via Smcdeleg/Ssccfg */
>> +#ifdef CONFIG_64BIT
>> +#define HPMEVENT_OF (BIT_ULL(63))
>> +#define HPMEVENT_MINH (BIT_ULL(62))
>> +#define HPMEVENT_SINH (BIT_ULL(61))
>> +#define HPMEVENT_UINH (BIT_ULL(60))
>> +#define HPMEVENT_VSINH (BIT_ULL(59))
>> +#define HPMEVENT_VUINH (BIT_ULL(58))
>> +#else
>> +#define HPMEVENTH_OF (BIT_ULL(31))
>> +#define HPMEVENTH_MINH (BIT_ULL(30))
>> +#define HPMEVENTH_SINH (BIT_ULL(29))
>> +#define HPMEVENTH_UINH (BIT_ULL(28))
>> +#define HPMEVENTH_VSINH (BIT_ULL(27))
>> +#define HPMEVENTH_VUINH (BIT_ULL(26))
> Since these are rv32 bits for a 32-bit register, I think these should be
> BIT() instead of BIT_ULL()
>
>> +
>> +#define HPMEVENT_OF (HPMEVENTH_OF << 32)
>> +#define HPMEVENT_MINH (HPMEVENTH_MINH << 32)
>> +#define HPMEVENT_SINH (HPMEVENTH_SINH << 32)
>> +#define HPMEVENT_UINH (HPMEVENTH_UINH << 32)
>> +#define HPMEVENT_VSINH (HPMEVENTH_VSINH << 32)
>> +#define HPMEVENT_VUINH (HPMEVENTH_VUINH << 32)
> These definitions are identical to the rv64 ones, can these be removed
> and can you move the rv64 definitions to be global?
Good catch. Will fix this and the above in v8.
> - Charlie
>
>> +#endif
>> +
>> +#define SISELECT_SSCCFG_BASE 0x40
>>
>> /* mseccfg bits */
>> #define MSECCFG_PMM ENVCFG_PMM
>> @@ -322,6 +347,7 @@
>> #define CSR_SCOUNTEREN 0x106
>> #define CSR_SENVCFG 0x10a
>> #define CSR_SSTATEEN0 0x10c
>> +#define CSR_SCOUNTINHIBIT 0x120
>> #define CSR_SSCRATCH 0x140
>> #define CSR_SEPC 0x141
>> #define CSR_SCAUSE 0x142
>>
>> --
>> 2.53.0-Meta
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply
* Re: [PATCH v5 09/16] media: iris: Add framework support for AR50_LITE video core
From: Vikash Garodia @ 2026-06-24 7:36 UTC (permalink / raw)
To: Dmitry Baryshkov, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vishnu Reddy
Cc: linux-media, linux-arm-msm, linux-kernel, devicetree,
Dikshita Agarwal
In-Reply-To: <20260616-iris-ar50lt-v5-9-583b42770b6a@oss.qualcomm.com>
On 6/16/2026 5:34 AM, Dmitry Baryshkov wrote:
> From: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>
> Add power sequence for ar5lt core.
> Add register handling for ar50lt by hooking up vpu op with ar50lt
> specific implemtation or resue from earlier generation wherever
> feasible.
>
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/Makefile | 1 +
> .../platform/qcom/iris/iris_platform_common.h | 2 +
> drivers/media/platform/qcom/iris/iris_vpu2.c | 28 +----
> drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c | 130 +++++++++++++++++++++
> drivers/media/platform/qcom/iris/iris_vpu_common.c | 29 ++++-
> drivers/media/platform/qcom/iris/iris_vpu_common.h | 2 +
> 6 files changed, 164 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
> index 48e415cbc439..f1b204b95694 100644
> --- a/drivers/media/platform/qcom/iris/Makefile
> +++ b/drivers/media/platform/qcom/iris/Makefile
> @@ -26,6 +26,7 @@ qcom-iris-objs += iris_buffer.o \
> iris_vpu2.o \
> iris_vpu3x.o \
> iris_vpu4x.o \
> + iris_vpu_ar50lt.o \
> iris_vpu_buffer.o \
> iris_vpu_common.o \
>
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index accc1627defd..6a189489369f 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -74,6 +74,7 @@ enum platform_clk_type {
> IRIS_VPP0_HW_CLK,
> IRIS_VPP1_HW_CLK,
> IRIS_APV_HW_CLK,
> + IRIS_THROTTLE_CLK,
As we extend the common enum for (one off probably) SOC specific
entries, should we bring in the changes to migrate this to per block
clock data, something which was done here [1] ?
[1]
https://lore.kernel.org/linux-media/20260610-glymur-v8-6-1c79b9d51fc0@oss.qualcomm.com/
> };
>
> struct platform_clk_data {
> @@ -315,6 +316,7 @@ struct iris_platform_data {
> u32 tz_cp_config_data_size;
> u32 num_vpp_pipe;
> bool no_aon;
> + bool no_rpmh;
> u32 wd_intr_mask;
> u32 icc_ib_multiplier;
> u32 max_session_count;
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
> index dd2eeae0d9eb..5419a5096b00 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
> @@ -12,38 +12,12 @@
>
> #include "iris_vpu_register_defines.h"
>
> -static u64 iris_vpu2_calc_freq(struct iris_inst *inst, size_t data_size)
> -{
> - struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
> - struct v4l2_format *inp_f = inst->fmt_src;
> - u32 mbs_per_second, mbpf, height, width;
> - unsigned long vpp_freq, vsp_freq;
> - u32 fps = inst->frame_rate;
> -
> - width = max(inp_f->fmt.pix_mp.width, inst->crop.width);
> - height = max(inp_f->fmt.pix_mp.height, inst->crop.height);
> -
> - mbpf = NUM_MBS_PER_FRAME(height, width);
> - mbs_per_second = mbpf * fps;
> -
> - vpp_freq = mbs_per_second * caps->mb_cycles_vpp;
> -
> - /* 21 / 20 is overhead factor */
> - vpp_freq += vpp_freq / 20;
> - vsp_freq = mbs_per_second * caps->mb_cycles_vsp;
> -
> - /* 10 / 7 is overhead factor */
> - vsp_freq += ((fps * data_size * 8) * 10) / 7;
> -
> - return max(vpp_freq, vsp_freq);
> -}
> -
> const struct vpu_ops iris_vpu2_ops = {
> .power_off_hw = iris_vpu_power_off_hw,
> .power_on_hw = iris_vpu_power_on_hw,
> .power_off_controller = iris_vpu_power_off_controller,
> .power_on_controller = iris_vpu_power_on_controller,
> - .calc_freq = iris_vpu2_calc_freq,
> + .calc_freq = iris_vpu2_calculate_frequency,
> .set_hwmode = iris_vpu_set_hwmode,
> .set_preset_registers = iris_vpu_set_preset_registers,
> .interrupt_init = iris_vpu_interrupt_init,
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
> new file mode 100644
> index 000000000000..e084a5b49f2e
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2026 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/iopoll.h>
> +#include <linux/reset.h>
> +
> +#include "iris_instance.h"
> +#include "iris_vpu_common.h"
> +
> +#include "iris_vpu_register_defines.h"
> +
> +#define WRAPPER_INTR_MASK_A2HVCODEC_BMSK_AR50LT BIT(3)
> +
> +#define WRAPPER_VCODEC0_CLOCK_CONFIG_AR50LT 0xb0080
> +
> +#define CPU_CS_VCICMD 0xa0020
> +#define CPU_CS_VCICMD_ARP_OFF 0x1
> +
> +static void iris_vpu_ar50lt_set_preset_registers(struct iris_core *core)
> +{
> + writel(0x0, core->reg_base + WRAPPER_VCODEC0_CLOCK_CONFIG_AR50LT);
> +}
> +
> +static void iris_vpu_ar50lt_interrupt_init(struct iris_core *core)
> +{
> + writel(WRAPPER_INTR_MASK_A2HVCODEC_BMSK_AR50LT, core->reg_base + WRAPPER_INTR_MASK);
> +}
> +
> +static void iris_vpu_ar50lt_disable_arp(struct iris_core *core)
> +{
> + writel(CPU_CS_VCICMD_ARP_OFF, core->reg_base + CPU_CS_VCICMD);
> +}
> +
> +static int iris_vpu_ar50lt_power_off_controller(struct iris_core *core)
> +{
> + iris_disable_unprepare_clock(core, IRIS_AHB_CLK);
> + iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
> + iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
> + iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +
> + return 0;
> +}
> +
> +static void iris_vpu_ar50lt_power_off_hw(struct iris_core *core)
> +{
> + dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], false);
> + iris_disable_unprepare_clock(core, IRIS_THROTTLE_CLK);
> + iris_disable_unprepare_clock(core, IRIS_HW_AHB_CLK);
> + iris_disable_unprepare_clock(core, IRIS_HW_CLK);
> + iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +}
> +
> +static int iris_vpu_ar50lt_power_on_controller(struct iris_core *core)
> +{
> + int ret;
> +
> + ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> + if (ret)
> + return ret;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_CTRL_CLK);
> + if (ret)
> + goto err_disable_power;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
> + if (ret && ret != -ENOENT)
> + goto err_disable_ctrl_clock;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_AHB_CLK);
> + if (ret)
> + goto err_disable_axi_clock;
> +
> + return 0;
> +
> +err_disable_axi_clock:
> + iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
> +err_disable_ctrl_clock:
> + iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
> +err_disable_power:
> + iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +
> + return ret;
> +}
> +
> +static int iris_vpu_ar50lt_power_on_hw(struct iris_core *core)
> +{
> + int ret;
> +
> + ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> + if (ret)
> + return ret;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
> + if (ret)
> + goto err_disable_power;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_HW_AHB_CLK);
> + if (ret)
> + goto err_disable_hw_clock;
> +
> + ret = iris_prepare_enable_clock(core, IRIS_THROTTLE_CLK);
> + if (ret)
> + goto err_disable_hw_ahb_clock;
> +
> + return 0;
...lot of these per soc specific clock handling would be removed.
Regards,
Vikash
^ permalink raw reply
* Re: [PATCH] dt-bindings: arm: qcom,ids: Add SoC ID for Snapdragon SDA 850
From: Krzysztof Kozlowski @ 2026-06-24 7:37 UTC (permalink / raw)
To: Konrad Dybcio, david, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <90f9f8a8-971d-4cb7-b2a0-248349b329d4@oss.qualcomm.com>
On 24/06/2026 09:35, Konrad Dybcio wrote:
> On 6/24/26 8:26 AM, Krzysztof Kozlowski wrote:
>> On 23/06/2026 20:41, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> Add SoC ID for Qualcomm Snapdragon SDA850.
>>>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>> Will be used by Comma three.
>>
>> Can you provide a link?
>
> Previous gen of https://comma.ai/shop/comma-four, they don't seem
> to have the old one listed on their website anymore
I meant, link to the upstream user. Something which will convince why do
we want this patch.
>
> The other computer tells me SDA850 doesn't exist and 348 is SDM850
> (for which we have a number of DTs in the kernel today), so let's
> rename it and add a second patch to add it to socinfo
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 5/9] media: chips-media: wave6: Add Wave6 core driver
From: sashiko-bot @ 2026-06-24 7:38 UTC (permalink / raw)
To: Nas Chung; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260624072043.238-6-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [High] Allocating `struct video_device` with `devm_kzalloc()` and an empty release callback causes a Use-After-Free when the driver is unbound while a file descriptor is open.
- [High] Freeing `core->irq_fifo` in `remove()` while the devm-managed IRQ handler is still active creates a use-after-free window.
- [High] Returning early for `W6_INT_BIT_REQ_WORK_BUF` in `wave6_vpu_core_irq` drops other concurrent hardware interrupts.
- [High] The interrupt thread drops completion events because `wave6_vpu_core_get_instance()` only returns the first matching instance.
- [High] Writing 0 to `W6_RET_INT_INSTANCE_INFO` creates a race condition that can drop newly raised interrupts.
- [High] The hardware interrupt handler is registered before the kfifo is allocated, leading to a potential NULL pointer dereference on early interrupts.
- [High] Hardware initialization failures during PM runtime resume are silently ignored, falsely reporting a successful resume.
- [High] Video devices are unregistered after PM runtime is disabled, allowing active file operations to access hardware without power management.
- [High] The IRQ handler unconditionally accesses hardware registers without checking if the device is active.
- [High] `wave6_vpu_disable_instance()` ignores timeouts while waiting for refcount to drop, potentially leading to Use-After-Free and ABA problems with stale events.
- [Medium] The driver creates a debugfs directory but never removes it, causing a memory leak on module unload.
- [Medium] Clocks are left permanently enabled if the driver is removed while the device is runtime-active.
- [Medium] Ignoring the return value of `kfifo_in()` allows interrupts to be silently dropped if the queue fills up.
--
commit dac8dbfd6064814484fc40f423abb75bd9d68a7c
Author: Nas Chung <nas.chung@chipsnmedia.com>
media: chips-media: wave6: Add Wave6 core driver
Add the core driver for the Chips&Media Wave6 video codec IP.
It configures the V4L2 mem2mem devices and communicates with the Wave6
hardware to perform video processing tasks.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
> new file mode 100644
> index 0000000000000..e54a2a39ff9a3
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
[ ... ]
> +static irqreturn_t wave6_vpu_core_irq(int irq, void *dev_id)
> +{
> + struct vpu_core_device *core = dev_id;
> + struct vpu_irq irq_info;
> +
> + if (!vpu_read_reg(core, W6_VPU_VPU_INT_STS))
> + return IRQ_NONE;
[Severity: High]
Does this unconditionally access hardware registers without checking if the
device is active?
If a spurious interrupt occurs while the device is runtime-suspended and its
clocks are disabled, this unclocked register read could cause an asynchronous
bus fault on platforms like i.MX95. Should the handler use
pm_runtime_get_if_active() before reading MMIO registers?
> +
> + irq_info.status = vpu_read_reg(core, W6_VPU_VINT_REASON);
> + irq_info.inst_idc = vpu_read_reg(core, W6_RET_INT_INSTANCE_INFO);
> +
> + vpu_write_reg(core, W6_RET_INT_INSTANCE_INFO, INT_INSTANCE_INFO_CLEAR);
[Severity: High]
Does unconditionally writing 0 to W6_RET_INT_INSTANCE_INFO create a race
condition?
If the hardware finishes another operation and sets a new bit in this register
between the read and the write, the write of 0 will clear the newly set bit
before it can be read, permanently dropping the interrupt for that instance.
> + vpu_write_reg(core, W6_VPU_VINT_REASON_CLEAR, irq_info.status);
> + vpu_write_reg(core, W6_VPU_VINT_CLEAR, VINT_CLEAR);
> +
> + trace_wave6_vpu_irq(core, irq_info.status, irq_info.inst_idc);
> +
> + if (irq_info.status & BIT(W6_INT_BIT_REQ_WORK_BUF)) {
> + if (core->vpu)
> + core->vpu->req_work_buffer(core->vpu, core);
> +
> + return IRQ_HANDLED;
> + }
[Severity: High]
Will returning early for W6_INT_BIT_REQ_WORK_BUF drop other concurrent
hardware interrupts?
Because the entire irq_info.status mask was already cleared from the hardware,
returning here bypasses the kfifo_in() call. Any other interrupt events (like
DEC_PIC or BSBUF_ERROR) present in the status are never queued to the
interrupt thread and will be permanently lost, potentially hanging the
affected instances.
> +
> + kfifo_in(&core->irq_fifo, &irq_info, sizeof(struct vpu_irq));
[Severity: Medium]
What happens if the FIFO fills up?
kfifo_in() returns the number of bytes queued (or 0 if full). Ignoring the
return value allows interrupts to be silently dropped if a burst occurs before
the threaded handler can process them, leaving VPU instances waiting
indefinitely. Should this at least log an error on overflow?
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static struct vpu_instance *wave6_vpu_core_get_instance(struct vpu_core_device *core,
> + u32 inst_idc)
> +{
> + struct vpu_instance *inst;
> +
> + guard(spinlock)(&core->inst_lock);
> +
> + list_for_each_entry(inst, &core->instances, list) {
> + if ((BIT(inst->id) & inst_idc) && inst->enable) {
> + atomic_inc(&inst->refcount);
> + return inst;
> + }
> + }
> +
> + return NULL;
> +}
[Severity: High]
Does this only return the first matching instance?
The hardware provides a bitmask of interrupting instances in inst_idc. By
iterating and returning only the first matched instance, any other instances
that had their bits set in the same inst_idc mask are completely ignored,
causing them to miss their interrupts and hang indefinitely.
[ ... ]
> +void wave6_vpu_disable_instance(struct vpu_instance *inst)
> +{
> + int count;
> +
> + scoped_guard(spinlock, &inst->dev->inst_lock)
> + inst->enable = false;
> +
> + if (read_poll_timeout(atomic_read, count, !count,
> + W6_VPU_POLL_DELAY_US,
> + W6_VPU_POLL_TIMEOUT,
> + true, &inst->refcount))
> + dev_dbg(inst->dev->dev, "[%d] disable timeout\n", inst->id);
> +}
[Severity: High]
Does ignoring the timeout here lead to a potential use-after-free and ABA
problem?
If read_poll_timeout() times out, the function merely logs a debug message and
returns, allowing the caller to proceed to destroy the instance. If the
delayed IRQ thread is still processing the instance, it will access freed
memory.
Additionally, refcount only tracks events actively being processed by the
thread, not un-popped events still in the kfifo. Thus, disable_instance() can
return while events are still in the FIFO. If a new instance is created
reusing the same ID, the IRQ thread could pop the stale event and incorrectly
match it to the new instance.
[ ... ]
> +static void wave6_vpu_core_check_state(struct vpu_core_device *core)
> +{
> + u32 val;
> + int ret;
> +
> + guard(mutex)(&core->hw_lock);
> +
> + ret = read_poll_timeout(vpu_read_reg, val, val != 0,
> + W6_VPU_POLL_DELAY_US, W6_VPU_POLL_TIMEOUT,
> + false, core, W6_VPU_VCPU_CUR_PC);
> + if (ret)
> + return;
[Severity: High]
Are hardware initialization failures swallowed here?
This function returns void and drops the error code if read_poll_timeout()
or wave6_vpu_get_version() fails.
[ ... ]
> +static int wave6_vpu_core_probe(struct platform_device *pdev)
> +{
[ ... ]
> + ret = devm_request_threaded_irq(&pdev->dev, irq,
> + wave6_vpu_core_irq,
> + wave6_vpu_core_irq_thread,
> + 0, "vpu_irq", core);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request IRQ: %d\n", ret);
> + return ret;
> + }
> +
> + ret = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register v4l2_dev: %d\n", ret);
> + return ret;
> + }
> +
> + ret = wave6_vpu_init_m2m_dev(core);
> + if (ret)
> + goto err_v4l2_unregister;
> +
> + ret = kfifo_alloc(&core->irq_fifo,
> + MAX_NUM_INSTANCE * sizeof(struct vpu_irq),
> + GFP_KERNEL);
[Severity: High]
Could an early interrupt cause a NULL pointer dereference?
devm_request_threaded_irq() is called before kfifo_alloc(). If an interrupt
fires immediately upon registration, wave6_vpu_core_irq() will execute and
attempt to push the event into the unallocated FIFO using kfifo_in(),
leading to a crash.
[ ... ]
> + core->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
> + if (!IS_ERR_OR_NULL(core->debugfs))
> + dput(core->debugfs);
> + else
> + core->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR, NULL);
[Severity: Medium]
Is there a missing debugfs_remove() call for this directory in
wave6_vpu_core_remove()? This leaks the directory on module unload.
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + if (core->res->codec_types & WAVE6_IS_DEC) {
> + ret = wave6_vpu_dec_register_device(core);
[Severity: High]
Does allocating the video_device with devm_kzalloc() inside
wave6_vpu_dec_register_device() cause a use-after-free?
wave6_vpu_dec_register_device() and wave6_vpu_enc_register_device() allocate
the video device via devm_kzalloc() and assign an empty release callback. If
a user holds a file descriptor open and the driver is unbound, devres frees
the video_device memory. When the user later closes the file descriptor, the
V4L2 core accesses the now-freed video_device struct.
[ ... ]
> +static void wave6_vpu_core_remove(struct platform_device *pdev)
> +{
> + struct vpu_core_device *core = dev_get_drvdata(&pdev->dev);
> +
> + pm_runtime_disable(&pdev->dev);
> +
> + wave6_vpu_enc_unregister_device(core);
> + wave6_vpu_dec_unregister_device(core);
[Severity: High]
Is PM runtime disabled before the user-facing video devices are unregistered?
Unregistering video devices after pm_runtime_disable() allows active file
operations to access hardware without power management. If a user application
issues an ioctl during this window, the driver may attempt to wake the device,
failing and leading to unexpected errors or unpowered hardware access.
[Severity: Medium]
Are clocks left permanently enabled if the driver is removed while the device
is active?
Since this function does not explicitly suspend the device or disable clocks,
and devm_clk_bulk_get_all() only releases the clock references without
disabling them, the device clocks will be left enabled if the driver is
unbound while active (e.g., streaming).
> + wave6_vdi_free_dma(&core->temp_vbuf);
> + kfifo_free(&core->irq_fifo);
[Severity: High]
Does manually freeing core->irq_fifo create a use-after-free window?
The IRQ was requested using devm_request_threaded_irq(), meaning it will be
freed during the devres cleanup phase after remove() returns. Manually calling
kfifo_free() here creates a window where the devm-managed IRQ remains active
but the kfifo and DMA memory are already freed. A late or spurious interrupt
during this window will trigger kfifo_in() on freed memory.
> + wave6_vpu_release_m2m_dev(core);
> + v4l2_device_unregister(&core->v4l2_dev);
> +}
[ ... ]
> +static int wave6_vpu_core_runtime_resume(struct device *dev)
> +{
[ ... ]
> + /*
> + * Only call parent VPU get_vpu if the core has a parent and is active.
> + * - core->vpu: prevent access in core without parent VPU.
> + * - core->active: execute boot only after m2m streaming is started.
> + */
> + if (core->vpu && core->active)
> + ret = core->vpu->get_vpu(core->vpu, core);
> +
> + if (!ret)
> + wave6_vpu_core_wait_activated(core);
> + else if (core->num_clks)
> + clk_bulk_disable_unprepare(core->num_clks, core->clks);
> +
> + return ret;
> +}
[Severity: High]
Because wave6_vpu_core_check_state() is void and cannot propagate failures,
wave6_vpu_core_wait_activated() will return without an error if it times out
or fails to get the version.
This means wave6_vpu_core_runtime_resume() will return 0 (success) to the PM
framework even if hardware initialization failed, leaving the device unable to
generate interrupts and hanging subsequent V4L2 operations.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=5
^ permalink raw reply
* Re: [PATCH v6 4/9] media: chips-media: wave6: Add v4l2 m2m driver support
From: sashiko-bot @ 2026-06-24 7:40 UTC (permalink / raw)
To: Nas Chung; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260624072043.238-5-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] On encoder/decoder start failure, the driver removes the wrong source buffer from the M2M queue, leading to premature buffer completion and Use-After-Free.
- [High] The decoder `VIDIOC_S_FMT` handlers lack `vb2_is_busy()` checks, allowing userspace to dynamically change the pixel format during streaming.
- [High] Missing `vb2_is_busy()` check in encoder's `s_selection` allows changing crop dimensions during streaming, leading to hardware DMA out-of-bounds writes.
- [High] The `video_device` structure is allocated with `devm_kzalloc`, leading to a Use-After-Free on driver unbind.
- [High] A race condition in the global `task_timer` timeout handler leads to Use-After-Free and M2M state corruption.
- [High] Unprotected traversal of `v4l2_m2m` ready queues causes linked list corruption.
- [High] Out-of-bounds array reads and writes due to unvalidated frame buffer and motion vector counts returned directly from hardware registers.
- [High] Missing hardware reset and completion reinitialization on VPU timeout leaves a stray completion that corrupts future hardware syncs.
- [Medium] Conditionally dropping the PM usage counter based on `pm_runtime_suspended()` leaks the usage counter.
- [Medium] Decoder permanently enables the hardware scaler once scaling is requested.
--
commit 85193b153b6b5079480d70d95e569973a1a2732e
Author: Nas Chung <nas.chung@chipsnmedia.com>
media: chips-media: wave6: Add v4l2 m2m driver support
This commit adds V4L2 M2M stateful driver support for the Chips&Media
Wave6 video codec IP, covering both encoder and decoder. It includes
multiplanar format support, scaling, and extensive V4L2 encoder controls
for AVC and HEVC formats.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-dec.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-dec.c
> index 0000000000000..9545264c19b7f 100644
> --- a/drivers/media/platform/chips-media/wave6/wave6-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-dec.c
[ ... ]
> @@ -152,6 +152,8 @@ static void wave6_vpu_dec_destroy_instance(struct vpu_instance *inst, bool remov
> wave6_vpu_dec_release_fb(inst);
> wave6_vpu_set_instance_state(inst, VPU_INST_STATE_NONE);
>
> if (!pm_runtime_suspended(inst->dev->dev))
> pm_runtime_put_sync(inst->dev->dev);
> }
[Severity: Medium]
Will this skip dropping the PM usage counter if the device is runtime suspended?
If the device enters runtime suspend (e.g., via system suspend) and
pm_runtime_suspended() returns true, pm_runtime_put_sync() is skipped. This
seems to leave the usage counter elevated, which could permanently prevent the
VPU from entering runtime suspend again.
[ ... ]
> @@ -219,6 +219,8 @@ static void wave6_vpu_dec_handle_dst_buffer(struct vpu_instance *inst)
> int consumed_num = wave6_vpu_get_consumed_fb_num(inst);
> int ret;
>
> v4l2_m2m_for_each_dst_buf(inst->v4l2_fh.m2m_ctx, v4l2_m2m_buf) {
> dst_buf = &v4l2_m2m_buf->vb;
> vpu_buf = wave6_to_vpu_buf(dst_buf);
[Severity: High]
Does traversing the V4L2 M2M ready queue here require holding the rdy_spinlock?
The v4l2_m2m_for_each_dst_buf() macro iterates over the rdy_queue, which can be
modified concurrently by VIDIOC_QBUF in process context. Traversing this queue
without the rdy_spinlock might result in following stale list pointers and
causing linked list corruption.
[ ... ]
> @@ -598,6 +598,8 @@ static int wave6_vpu_dec_start_decode(struct vpu_instance *inst)
> dev_err(inst->dev->dev, "[%d] %s: fail %d\n", inst->id, __func__, ret);
> wave6_vpu_set_instance_state(inst, VPU_INST_STATE_STOP);
>
> src_buf = v4l2_m2m_src_buf_remove(inst->v4l2_fh.m2m_ctx);
> if (src_buf) {
> v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
[Severity: High]
If start_one_frame fails, could removing the first buffer from the source queue
complete the wrong buffer?
Since the VPU driver allows multiple source buffers to be in flight (such as
for B-frame reordering), the first buffer in the M2M rdy_queue might be
actively processing. Unconditionally popping it via v4l2_m2m_src_buf_remove()
and completing it with VB2_BUF_STATE_ERROR would return an active buffer to
userspace. If userspace then frees or unmaps it, this could lead to a
Use-After-Free or DMA memory corruption.
[ ... ]
> @@ -682,6 +682,8 @@ static int wave6_vpu_dec_s_fmt_cap(struct file *file, void *fh, struct v4l2_form
> __func__, pix_mp->pixelformat, pix_mp->width, pix_mp->height,
> pix_mp->num_planes, pix_mp->colorspace);
>
> ret = wave6_vpu_dec_try_fmt_cap(file, fh, f);
> if (ret)
> return ret;
>
> vpu_fmt = wave6_find_vpu_fmt(pix_mp->pixelformat, VPU_FMT_TYPE_RAW);
[Severity: High]
Should there be a vb2_is_busy() check here to prevent userspace from changing
the format mid-stream?
Without this check, the format and inst->dst_fmt properties can be modified
during streaming without reallocating buffers. This could cause the driver to
calculate out-of-bounds DMA addresses and corrupt memory.
[ ... ]
> @@ -766,6 +766,8 @@ static int wave6_vpu_dec_s_fmt_out(struct file *file, void *fh, struct v4l2_form
> __func__, pix_mp->pixelformat, pix_mp->width, pix_mp->height,
> pix_mp->num_planes, pix_mp->colorspace);
>
> ret = wave6_vpu_dec_try_fmt_out(file, fh, f);
> if (ret)
> return ret;
>
> pix_mp->colorspace = in_pix_mp.colorspace;
[Severity: High]
Does this also need a vb2_is_busy() check to prevent mid-stream format changes
on the output queue?
Altering inst->src_fmt while streaming could lead to similar out-of-bounds
DMA operations.
[ ... ]
> @@ -954,6 +954,8 @@ static int wave6_vpu_dec_s_selection(struct file *file, void *fh, struct v4l2_se
> scale_height = round_down(scale_height, step);
> }
>
> if (scale_width < inst->src_fmt.width ||
> scale_height < inst->src_fmt.height)
> inst->scaler_info.enable = true;
>
> if (inst->scaler_info.enable) {
[Severity: Medium]
If a subsequent call requests a 1:1 scale, does inst->scaler_info.enable ever
get reset to false?
It appears the scaler is permanently enabled once requested. If userspace
reverts to a 1:1 scale later, the hardware scaler remains active, which might
introduce unnecessary overhead or logic errors.
[ ... ]
> @@ -1323,6 +1323,8 @@ static int wave6_vpu_dec_prepare_fb(struct vpu_instance *inst)
> luma_size = fb_stride * fb_height;
> chroma_size = ALIGN(fb_stride / 2, W6_FBC_BUF_ALIGNMENT) * fb_height;
>
> for (i = 0; i < fb_num; i++) {
> struct frame_buffer *frame = &inst->frame_buf[i];
> struct vpu_buf *vframe = &inst->frame_vbuf[i];
>
> vframe->size = luma_size + chroma_size;
[Severity: High]
Can fb_num (derived from the hardware register W6_RET_DEC_MIN_FB_CNT) exceed
the WAVE6_MAX_FBS array bounds?
The values min_frame_buffer_count and req_mv_buffer_count are read directly
from hardware in wave6_get_dec_seq_result() without validation. If the
hardware or firmware returns a value larger than the fixed-size array bounds,
this loop could cause out-of-bounds writes to the inst->frame_buf and
inst->frame_vbuf arrays.
[ ... ]
> @@ -1803,6 +1803,8 @@ int wave6_vpu_dec_register_device(struct vpu_core_device *core)
> struct video_device *vdev_dec;
> int ret;
>
> vdev_dec = devm_kzalloc(core->v4l2_dev.dev, sizeof(*vdev_dec), GFP_KERNEL);
> if (!vdev_dec)
> return -ENOMEM;
>
> core->video_dev_dec = vdev_dec;
[Severity: High]
Is it safe to allocate the video_device with devm_kzalloc() here?
The V4L2 video_device structure must be freed dynamically in its release
callback (currently assigned video_device_release_empty) when the last file
descriptor is closed. If the device is unbound via sysfs while userspace holds
an open file descriptor, devm_kzalloc will free the memory, but the V4L2 core
will still hold pointers to it. This could result in a Use-After-Free during
subsequent ioctl or close operations.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-enc.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-enc.c
> index 0000000000000..825229f6fb051 100644
> --- a/drivers/media/platform/chips-media/wave6/wave6-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-enc.c
[ ... ]
> @@ -422,6 +422,8 @@ static int wave6_allocate_aux_buffer(struct vpu_instance *inst,
> return ret;
> }
>
> for (i = 0; i < num; i++) {
> inst->aux_vbuf[type][i].size = size;
> ret = wave6_vdi_alloc_dma(inst->dev->dev, &inst->aux_vbuf[type][i]);
> if (ret) {
[Severity: High]
Is the num parameter validated against the array size before this loop?
Unlike the decoder's allocate function which clamps num to WAVE6_MAX_FBS,
the encoder seems to use the unvalidated num (derived from hardware registers)
directly. This could lead to out-of-bounds writes into the aux_vbuf array if
the hardware returns an unexpectedly large value.
[ ... ]
> @@ -783,6 +783,8 @@ static int wave6_vpu_enc_s_selection(struct file *file, void *fh, struct v4l2_se
> s->r.width = clamp(s->r.width, W6_MIN_ENC_PIC_WIDTH, max_crop_w);
> s->r.height = clamp(s->r.height, W6_MIN_ENC_PIC_HEIGHT, max_crop_h);
>
> wave6_update_pix_fmt(&inst->dst_fmt, s->r.width, s->r.height);
> wave6_update_crop_info(inst, s->r.left, s->r.top, s->r.width, s->r.height);
>
> dev_dbg(inst->dev->dev, "V4L2_SEL_TGT_CROP %dx%dx%dx%d\n",
[Severity: High]
Should there be a vb2_is_busy() check before updating the crop dimensions?
Without checking if the queues are busy, userspace can change the crop
dimensions during streaming. If a larger crop selection is set,
inst->codec_rect is updated, and the hardware will be programmed with larger
dimensions than the previously allocated internal FBC backing buffers. This
could result in hardware DMA memory corruption.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-v4l2.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-v4l2.c
> index 0000000000000..a9a168b8bf269 100644
> --- a/drivers/media/platform/chips-media/wave6/wave6-vpu-v4l2.c
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-v4l2.c
[ ... ]
> @@ -72,6 +72,8 @@ struct vb2_v4l2_buffer *wave6_get_dst_buf_by_addr(struct vpu_instance *inst,
> struct v4l2_m2m_buffer *v4l2_m2m_buf;
> struct vb2_v4l2_buffer *dst_buf = NULL;
>
> v4l2_m2m_for_each_dst_buf(inst->v4l2_fh.m2m_ctx, v4l2_m2m_buf) {
> vb2_v4l2_buf = &v4l2_m2m_buf->vb;
> if (addr == wave6_get_dma_addr(vb2_v4l2_buf, 0)) {
> dst_buf = vb2_v4l2_buf;
[Severity: High]
Is it safe to iterate over the M2M destination queue without holding the
rdy_spinlock?
The rdy_queue is shared and can be concurrently modified by VIDIOC_QBUF.
Traversing it unprotected could result in following stale list pointers.
[ ... ]
> @@ -312,6 +312,8 @@ int wave6_vpu_wait_interrupt(struct vpu_instance *inst, unsigned int timeout)
>
> ret = wait_for_completion_timeout(&inst->irq_done,
> msecs_to_jiffies(timeout));
> if (!ret)
> return -ETIMEDOUT;
>
> reinit_completion(&inst->irq_done);
[Severity: High]
Does exiting early on a timeout leave a stray completion signal?
If the timeout expires and the driver returns -ETIMEDOUT without resetting
the hardware or calling reinit_completion(), the VPU might finish the command
later and signal the completion. This stray completion would cause a subsequent
wait_for_completion_timeout() to return immediately, potentially reading result
registers while the hardware is still executing a new command.
[ ... ]
> @@ -419,6 +419,8 @@ static void wave6_vpu_device_run_timeout(struct work_struct *work)
> vb2_queue_error(v4l2_m2m_get_src_vq(inst->v4l2_fh.m2m_ctx));
> vb2_queue_error(v4l2_m2m_get_dst_vq(inst->v4l2_fh.m2m_ctx));
>
> v4l2_m2m_job_finish(inst->dev->m2m_dev, inst->v4l2_fh.m2m_ctx);
> }
[Severity: High]
Should the hardware be reset or aborted before finishing the M2M job?
If v4l2_m2m_job_finish() is called while the VPU is actively executing a
timed-out command, an eventual hardware interrupt might corrupt the next job
or trigger a Use-After-Free if the instance has been freed.
[ ... ]
> @@ -442,6 +442,8 @@ static void wave6_vpu_device_run(void *priv)
>
> void wave6_vpu_finish_job(struct vpu_instance *inst)
> {
> cancel_delayed_work(&inst->dev->task_timer);
> v4l2_m2m_job_finish(inst->dev->m2m_dev, inst->v4l2_fh.m2m_ctx);
> }
[Severity: High]
Can cancel_delayed_work() leave the timeout handler running concurrently?
Using cancel_delayed_work() instead of cancel_delayed_work_sync() allows the
timeout handler to potentially race with instance destruction or subsequent
jobs.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=4
^ permalink raw reply
* Re: [PATCH v6 8/9] media: chips-media: wave6: Add Wave6 control driver
From: sashiko-bot @ 2026-06-24 7:41 UTC (permalink / raw)
To: Nas Chung; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260624072043.238-9-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [Critical] Concurrent access to work buffer pool without synchronization causes data races and array out-of-bounds access.
- [Critical] Self-deadlock (ABBA) during VPU core unregistration due to holding vpu->lock.
- [Critical] Missing hardware quiescence during device removal allows the VPU to continue DMAing into freed memory.
- [High] Asynchronous firmware loading callback causes a use-after-free if the device is unbound before completion.
- [High] Pointer type mismatch when calling gen_pool_dma_alloc() leads to memory corruption and invalid DMA mapping.
- [High] Disabling PM runtime prematurely leaks clock enable references on the parent device.
- [High] Silent truncation of the firmware code buffer during memory remapping.
- [High] Calling of_irq_get() inside an asynchronous firmware load callback prevents handling -EPROBE_DEFER.
- [High] Failures in wave6_vpu_init_work_buf() are ignored, leading to a broken VPU boot state.
- [High] wave6_vpu_detach_pm_domains() destroys PM domain virtual devices before the devm-managed thermal cooling device is unregistered, creating a UAF window.
- [High] The driver frees the IRQ kfifo before disabling the IRQ during core removal, leading to Use-After-Free.
- [Medium] Conditionally calling pm_runtime_put_sync() based on pm_runtime_suspended() leaks the usage counter.
- [Medium] Checking dma_addr against 0 to determine validity skips unmapping on platforms where 0 is a valid DMA address.
--
commit e4a2fe67f6aa37393c77f5308ac691953a00a6e8
Author: Nas Chung <nas.chung@chipsnmedia.com>
Subject: media: chips-media: wave6: Add Wave6 control driver
This commit adds the control driver for the Chips&Media Wave6 video codec IP.
It handles the control region and manages shared resources such as firmware
loading, memory allocation, and the creation of wave6-core child devices.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu.c b/drivers/media/platform/chips-media/wave6/wave6-vpu.c
> new file mode 100644
> index 0000000000000..c6efc45781833
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu.c
[ ... ]
> +static void wave6_vpu_remap_code_buf(struct wave6_vpu_device *vpu)
> +{
> + dma_addr_t code_base = vpu->code_buf.dma_addr;
> + u32 i, reg_val;
> +
> + for (i = 0; i < wave6_vpu_get_code_buf_size(vpu) / W6_MAX_REMAP_PAGE_SIZE; i++) {
[Severity: High]
Does this code silently truncate the firmware code buffer during memory
remapping?
If the buffer size is not an exact multiple of W6_MAX_REMAP_PAGE_SIZE,
the integer division will discard the remainder and the tail end of the
buffer will not be mapped. If the firmware requires the unmapped tail,
could the VPU encounter a memory fault?
[ ... ]
> +static int wave6_vpu_init_vpu(struct wave6_vpu_device *vpu,
> + struct vpu_core_device *core)
> +{
[ ... ]
> +init_done:
> + wave6_vpu_init_work_buf(vpu, core);
> + wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_ON);
> +
> + return 0;
> +}
[Severity: High]
Is the return value of wave6_vpu_init_work_buf() intentionally ignored here?
If wave6_vpu_wait_busy() or wave6_vpu_check_result() fails inside
wave6_vpu_init_work_buf(), it will return early without setting
vpu->work_buffers_avail. Because the failure is ignored, the VPU state
transitions to WAVE6_VPU_STATE_ON anyway, and later requests for work
buffers might feed uninitialized addresses to the hardware.
[ ... ]
> +static void wave6_vpu_put(struct wave6_vpu_device *vpu,
> + struct vpu_core_device *core)
> +{
[ ... ]
> + wave6_vpu_sleep(vpu, core);
> +
> + if (!pm_runtime_suspended(vpu->dev))
> + pm_runtime_put_sync(vpu->dev);
> +}
[Severity: Medium]
Could this conditional check cause the runtime PM usage counter to leak?
If the device's status is forcefully set to suspended, for example via
pm_runtime_force_suspend() during system sleep, pm_runtime_suspended()
will return true and this check will skip the put operation, leaving the
counter permanently incremented.
[ ... ]
> +static void wave6_vpu_require_work_buffer(struct wave6_vpu_device *vpu,
> + struct vpu_core_device *core)
> +{
[ ... ]
> + if (WARN_ON(vpu->work_buffers_avail == 0))
> + goto exit;
> +
> + vpu->work_buffers_avail--;
> + vb = &vpu->work_buffers[vpu->work_buffers_avail];
[Severity: Critical]
Is there a data race when accessing the work buffers pool?
Since wave6_vpu_require_work_buffer() modifies vpu->work_buffers_avail
without a lock, and wave6_vpu_core_irq() can call this from a hardirq
context across multiple child cores concurrently, could this result in
an array out-of-bounds access or assigning the exact same DMA buffer to
multiple instances?
[ ... ]
> +static int wave6_vpu_create_cores(struct wave6_vpu_device *vpu)
> +{
[ ... ]
> + irq = of_irq_get(child, 0);
> + if (irq < 0) {
> + dev_warn(vpu->dev, "%pOF: missing interrupts property\n", child);
> + continue;
> + }
[Severity: High]
Does this error handling prevent the driver from properly handling probe
deferral?
Because wave6_vpu_create_cores() executes in the asynchronous firmware loader
worker context, returning -EPROBE_DEFER here won't defer the probe process.
If the IRQ controller probes later, the core device might be permanently
ignored instead of waiting for the controller.
[ ... ]
> +static void wave6_vpu_release(struct wave6_vpu_device *vpu)
> +{
> + guard(mutex)(&vpu->lock);
> +
> + vpu->fw_available = false;
> + wave6_vpu_destroy_cores(vpu);
> + wave6_vpu_free_work_buffers(vpu);
[Severity: Critical]
Can holding vpu->lock during core destruction result in a self-deadlock?
When wave6_vpu_destroy_cores() unregisters the core devices, the child
removal triggers pm_runtime_disable() which blocks via __pm_runtime_barrier()
waiting for ongoing PM transitions. If another thread is executing
wave6_vpu_put() or wave6_vpu_get() inside a runtime PM callback, it will
block trying to acquire vpu->lock, causing a deadlock.
[Severity: Critical]
Are the work buffers and SRAM DMA memory freed while the VPU hardware
might still be accessing them?
This function explicitly frees the DMA memory, but it doesn't appear to send
a sleep command, disable the PM runtime properly, or assert a hardware reset
first. If the hardware is still active, could it continue performing DMA into
the kernel memory that was just released?
[Severity: High]
When wave6_vpu_destroy_cores() tears down the core devices, does the
sub-driver wave6-vpu-core.c free the IRQ kfifo before disabling the IRQ?
Looking at wave6_vpu_core_remove(), it calls kfifo_free(&core->irq_fifo)
manually. Since devm_request_threaded_irq() is used, the IRQ is only
disabled after the remove routine completes. If an interrupt fires during
this window, the IRQ handler might call kfifo_in() on a freed structure.
[ ... ]
> + if (vpu->code_buf.dma_addr)
> + dma_unmap_resource(vpu->dev,
> + vpu->code_buf.dma_addr,
[Severity: Medium]
Is it safe to check the DMA address against 0 to determine if it is valid?
On architectures where 0 is a valid DMA address, this condition will
evaluate to false, which would skip the dma_unmap_resource() call and
permanently leak the mapping.
[ ... ]
> +static void wave6_vpu_load_firmware(const struct firmware *fw, void *context)
> +{
> + struct wave6_vpu_device *vpu = context;
> +
> + guard(mutex)(&vpu->lock);
[Severity: High]
Can this asynchronous callback trigger a use-after-free if the device is
unbound?
If the device is unbound via sysfs while the firmware load is still pending,
devres will free the devm-allocated vpu structure. Because there is no
cancellation mechanism on unbind, the callback might execute later and
dereference the freed vpu pointer when trying to acquire the lock.
[ ... ]
> +static int wave6_vpu_probe(struct platform_device *pdev)
> +{
[ ... ]
> + vpu->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> + if (vpu->sram_pool) {
> + vpu->sram_buf.size = vpu->res->sram_size;
> + vpu->sram_buf.vaddr = gen_pool_dma_alloc(vpu->sram_pool,
> + vpu->sram_buf.size,
> + &vpu->sram_buf.phys_addr);
[Severity: High]
Is the third argument to gen_pool_dma_alloc() using the correct pointer type?
The function gen_pool_dma_alloc() expects a dma_addr_t pointer for its
third argument, but here it receives a phys_addr_t pointer. On 32-bit
platforms where phys_addr_t and dma_addr_t have different sizes, could this
overwrite adjacent struct members? Furthermore, it returns a device DMA
address which is later passed to dma_map_resource() as if it were a physical
address.
[ ... ]
> + ret = firmware_request_nowait_nowarn(THIS_MODULE,
> + vpu->res->fw_name,
> + &pdev->dev,
> + GFP_KERNEL,
> + vpu,
> + wave6_vpu_load_firmware);
[Severity: High]
Does this firmware request need a cancellation mechanism on unbind?
As noted above in wave6_vpu_load_firmware(), without a way to cancel
the pending firmware request or hold a reference to the vpu structure,
an unbind operation could lead to a use-after-free when the callback
eventually runs.
[ ... ]
> +static void wave6_vpu_remove(struct platform_device *pdev)
> +{
> + struct wave6_vpu_device *vpu = dev_get_drvdata(&pdev->dev);
> +
> + pm_runtime_disable(vpu->dev);
> + wave6_vpu_release(vpu);
> + wave6_vpu_detach_pm_domains(vpu);
> +}
[Severity: High]
Does disabling the PM runtime prematurely here leak clock enable references?
When pm_runtime_disable() is called before the child cores are released via
wave6_vpu_release(), the parent's PM runtime is disabled. When the child
cores subsequently drop their references to the parent device, the parent's
runtime_suspend callback won't execute, which might leave the hardware
clocks running indefinitely.
[Severity: High]
Does detaching the PM domains here create a use-after-free window for
the thermal cooling device?
wave6_vpu_detach_pm_domains() destroys the PM domain virtual devices.
However, the thermal cooling device, which uses one of these virtual devices
as its parent, was registered with devm_ and won't be cleaned up until after
wave6_vpu_remove() returns. If a thermal event fires in that window, could
it access the freed virtual device?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=8
^ permalink raw reply
* Re: [PATCH v6 3/9] media: chips-media: wave6: Add Wave6 VPU interface
From: sashiko-bot @ 2026-06-24 7:43 UTC (permalink / raw)
To: Nas Chung; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260624072043.238-4-nas.chung@chipsnmedia.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing bounds validation on hardware-supplied frame buffer counts causes array buffer overflows and potential infinite loops.
- [Medium] Leaking dynamically allocated `inst->codec_info` when closing decoder/encoder instance fails.
- [Medium] Hardcoded `BIG_ENDIAN` mismatch leads to byte-swapped decoder pixel formats on little-endian hosts.
- [Low] Endianness bug on Big-Endian architectures during product name string extraction.
- [Low] Mixing goto-based cleanup and `guard()` in the same function violates cleanup subsystem guidelines.
--
commit 61aa06e0583bb5f57f21d3d07d1ad5f6245d0508
Author: Nas Chung <nas.chung@chipsnmedia.com>
media: chips-media: wave6: Add Wave6 VPU interface
Add an interface layer to manage hardware register configuration
and communication with the Chips&Media Wave6 video codec IP.
The interface provides low-level helper functions used by the
Wave6 core driver to implement video encoding and decoding operations.
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-hw.c b/drivers/media/platform/chips-media/wave6/wave6-hw.c
> index 0000000000000..b49348d6119c8 100644
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-hw.c
> @@ -230,22 +230,22 @@ int wave6_vpu_get_version(struct vpu_core_device *core)
> if (ret)
> return ret;
>
> reg_val = vpu_read_reg(core, W6_RET_PRODUCT_NAME);
> str = (u8 *)®_val;
> attr->product_name[0] = str[3];
> attr->product_name[1] = str[2];
> attr->product_name[2] = str[1];
> attr->product_name[3] = str[0];
> attr->product_name[4] = 0;
[Severity: Low]
On big-endian architectures, vpu_read_reg() returns the integer in native
byte order. If the bytes are reversed using fixed indices like this, could
it result in the string being stored backwards or reading a null terminator
first?
[ ... ]
> @@ -345,13 +345,13 @@ int wave6_vpu_dec_register_frame_buffer(struct vpu_instance *inst)
>
> fb_arr = &inst->frame_buf[0];
> fbc_count = p_dec_info->seq_info.min_frame_buffer_count;
> mv_count = p_dec_info->seq_info.req_mv_buffer_count;
>
> for (i = 0; i < fbc_count; i++) {
> if (!inst->aux_vbuf[AUX_BUF_FBC_Y_TBL][i].daddr)
> return -EINVAL;
> if (!inst->aux_vbuf[AUX_BUF_FBC_C_TBL][i].daddr)
> return -EINVAL;
> }
[Severity: High]
If the hardware returns a value for fbc_count that is larger than
WAVE6_MAX_FBS (31), could this loop read past the end of the inst->aux_vbuf
arrays?
[ ... ]
> @@ -377,7 +377,7 @@ int wave6_vpu_dec_register_frame_buffer(struct vpu_instance *inst)
>
> fbc_remain = fbc_count;
> mv_remain = mv_count;
> group_num = (fbc_count > mv_count) ? ((ALIGN(fbc_count, 16) / 16) - 1) :
> ((ALIGN(mv_count, 16) / 16) - 1);
[Severity: High]
If the hardware unexpectedly returns zero for both fbc_count and mv_count,
would the subtraction of 1 after division underflow to SIZE_MAX and cause
an infinite loop here?
[ ... ]
> @@ -445,9 +445,9 @@ int wave6_vpu_dec_register_display_buffer(struct vpu_instance *inst, struct fram
>
> reg_val = SET_DISP_COMMON_PIC_INFO_BWB_ON |
> SET_DISP_COMMON_PIC_INFO_C_FMT_IDC(c_fmt_idc) |
> SET_DISP_COMMON_PIC_INFO_PIXEL_ORDER(PIXEL_ORDER_INCREASING) |
> SET_DISP_COMMON_PIC_INFO_OUT_MODE(out_mode) |
> SET_DISP_COMMON_PIC_INFO_OUT_FMT(out_fmt) |
> SET_DISP_COMMON_PIC_INFO_STRIDE(fb.stride);
> vpu_write_reg(inst->dev, W6_CMD_DEC_SET_DISP_COMMON_PIC_INFO, reg_val);
> reg_val = SET_DISP_OPTION_ENDIAN(VDI_128BIT_BIG_ENDIAN);
> vpu_write_reg(inst->dev, W6_CMD_DEC_SET_DISP_OPTION, reg_val);
[Severity: Medium]
Is it expected to strictly use VDI_128BIT_BIG_ENDIAN here? On little-endian
hosts, enforcing BIG_ENDIAN for output buffers might cause the hardware
state machine to output byte-swapped pixels. Should this use HOST_ENDIAN
instead?
[ ... ]
> @@ -1585,11 +1585,11 @@ static void wave6_gen_set_fb_reg(struct vpu_instance *inst, struct enc_cmd_set_f
> reg->num_fb = SET_FB_NUM_FBC_END_IDX(fbc_count - 1) |
> SET_FB_NUM_MV_COL_END_IDX(mv_count - 1);
> reg->fbc_stride = SET_FB_FBC_STRIDE_L(fb_arr[0].stride) |
> SET_FB_FBC_STRIDE_C(fb_arr[0].stride_c);
> reg->default_cdf = 0;
>
> for (i = 0; i < fbc_count; i++) {
> reg->fbc_y[i] = fb_arr[i].buf_y;
> reg->fbc_c[i] = fb_arr[i].buf_cb;
> reg->fbc_cr[i] = fb_arr[i].buf_cr;
> reg->fbc_y_offset[i] = inst->aux_vbuf[AUX_BUF_FBC_Y_TBL][i].daddr;
[Severity: High]
Could this loop overflow the reg->fbc_y array if fbc_count exceeds
WAVE6_MAX_FBS?
[ ... ]
> @@ -1617,11 +1617,11 @@ int wave6_vpu_enc_register_frame_buffer(struct vpu_instance *inst)
> struct enc_cmd_set_fb_reg *reg;
> struct enc_info *p_enc_info;
> u32 fbc_count, mv_count, idx;
> int ret;
>
> guard(mutex)(&inst->dev->hw_lock);
>
> p_enc_info = &inst->codec_info->enc_info;
[ ... ]
> @@ -1665,11 +1665,11 @@ int wave6_vpu_enc_register_frame_buffer(struct vpu_instance *inst)
> goto exit;
> }
>
> if (!vpu_read_reg(inst->dev, W6_RET_SUCCESS)) {
> ret = -EIO;
> goto exit;
> }
>
> ret = 0;
>
> exit:
> kfree(reg);
> return ret;
> }
[Severity: Low]
The cleanup subsystem guidelines strictly forbid mixing goto-based error
handling with scope-based cleanup helpers like guard() in the same function,
as it can create confusing ownership semantics. Could the dynamically
allocated reg object be managed using __free(kfree) to avoid manual goto
blocks?
> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpuapi.c b/drivers/media/platform/chips-media/wave6/wave6-vpuapi.c
> index 0000000000000..266885de92afd 100644
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpuapi.c
> @@ -42,16 +42,16 @@ int wave6_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> if (WARN_ON(!inst->codec_info))
> return -EINVAL;
>
> guard(mutex)(&inst->dev->hw_lock);
>
> ret = wave6_vpu_dec_fini_seq(inst, fail_res);
> if (ret) {
> dev_warn(inst->dev->dev, "dec seq end timed out\n");
>
> return ret;
> }
>
> dev_dbg(inst->dev->dev, "dec seq end complete\n");
>
> kfree(inst->codec_info);
>
> return 0;
> }
[Severity: Medium]
If hardware teardown in wave6_vpu_dec_fini_seq() fails and returns an
error, does this skip freeing the inst->codec_info struct?
[ ... ]
> @@ -233,16 +233,16 @@ int wave6_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
> if (WARN_ON(!inst->codec_info))
> return -EINVAL;
>
> guard(mutex)(&inst->dev->hw_lock);
>
> ret = wave6_vpu_enc_fini_seq(inst, fail_res);
> if (ret) {
> dev_warn(inst->dev->dev, "enc seq end timed out\n");
>
> return ret;
> }
>
> dev_dbg(inst->dev->dev, "enc seq end complete\n");
> kfree(inst->codec_info);
>
> return 0;
> }
[Severity: Medium]
Similarly, if wave6_vpu_enc_fini_seq() fails here, will it result in a
memory leak of inst->codec_info?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=3
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Krzysztof Kozlowski @ 2026-06-24 7:45 UTC (permalink / raw)
To: Flaviu Nistor
Cc: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan, linux-hwmon,
linux-kernel, devicetree, linux-doc
In-Reply-To: <20260623192217.4804-1-flaviu.nistor@gmail.com>
On Tue, Jun 23, 2026 at 10:22:17PM +0300, Flaviu Nistor wrote:
> On 6/23/26 9:58 PM CET, Guenter Roeck wrote:
> >On 6/23/26 11:16, Flaviu Nistor wrote:
> >> On Mon Jun 22, 2026 at 7:29 PM CEST, Javier Carrasco wrote:
> >>> On Mon Jun 22, 2026 at 2:21 PM CEST, Flaviu Nistor wrote:
> >>>> Add support for an optional label property similar to other hwmon devices
> >>>> This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
> >>>> distinct names to each instance.
> >>>>
> >>>> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> >>>> ---
> >>>> .../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 5 +++++
> >>>> 1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.ya=
> >>> ml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> >>>> index 17351fdbefce..f00b5a4b14dd 100644
> >>>> --- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> >>>> +++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> >>>> @@ -33,6 +33,10 @@ properties:
> >>>> reg:
> >>>> maxItems: 1
> >>>>
> >>>> + label:
> >>>> + description:
> >>>> + A descriptive name for this channel, like "ambient" or "psu".
> >>>> +
> >>>> interrupts:
> >>>> items:
> >>>> - description: measurement ready indicator
> >>>> @@ -72,6 +76,7 @@ examples:
> >>>> <5 IRQ_TYPE_EDGE_RISING>,
> >>>> <6 IRQ_TYPE_EDGE_RISING>;
> >>>> interrupt-names =3D "ready", "low", "high";
> >>>> + label =3D "somelabel";
> >>>> vdd-supply =3D <®_vdd>;
> >>>> };
> >>> };
> >>>
> >>> Hello Falviu, thank you for your patch.
> >>>
> >>
> >> Hello Javier, thanks for your reply.
> >>
> >>> Should we not add a reference to hwmon-common.yaml (with
> >>> unevelautedProperties instead of additionalProperties), as label is
> >>> defined there? I believe that Krzysztof Kozlowski did something similar
> >>> for the shunt-resistor-micro-ohms property. Could we follow suit here?
> >>>
> >>
> >> This is a good question and I am happy you asked. I also thought a lot
> >> about this and the reason I decided to go for this approach is that by using
> >> $ref: hwmon-common.yaml#, I would have to change additionalProperties: false
> >> to unevaluatedProperties: false, which will evaluate in case it is used, also
> >> shunt-resistor-micro-ohms property which does not apply to this sensor. At
> >> least this is my understanding, but of course I can be wrong (I see lm75 binding
> >> also uses $ref: hwmon-common.yaml# but shunt-resistor-micro-ohms does not apply).
> >>
> >
> >Where does the idea come from that shunt-resistor-micro-ohms would be mandatory ?
> >That would make hwmon-common.yaml unusable for most chips.
>
> I think this is a misunderstanding since I never had the intention to imply that
> shunt-resistor-micro-ohms would be mandatory, but rather I observed that if I used
> $ref: hwmon-common.yaml#, property shunt-resistor-micro-ohms can be added (no need to,
> but still possible) in the example section and the dt_binding_check will pass.
> Since hwmon-common.yaml is already there I will change the binding in a v2 and use it.
I propose to reference hwmon-common.yaml with:
label: true
additionalProperties: false
thus shunt-resistor won't be allowed but you still will reference common
schema for the definition of common properties.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
From: Krzysztof Kozlowski @ 2026-06-24 7:47 UTC (permalink / raw)
To: Andrew Lunn
Cc: Krzysztof Kozlowski, Jie Luo, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Brian Masney, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Lei Wei, Suruchi Agarwal, Pavithra R,
linux-kernel, linux-arm-msm, linux-clk, devicetree, netdev
In-Reply-To: <0c1be662-3c6a-41be-bc19-081d0299ceae@lunn.ch>
On Tue, Jun 23, 2026 at 01:33:05PM +0200, Andrew Lunn wrote:
> > If address did not work for half a year, I really doubt that you commit
> > to above.
>
> I tend to agree. Maybe we should set it to Orphaned, and then decide
> in 6 months time if it can be set back to Maintained?
That would be a bit stronger than I intended and actual responses from
Jie did happen, thus the driver actually might be maintained. I also
think that Qualcomm is committed to maintain it, I only doubt about
supported.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 10/16] media: iris: add minimal GET_PROPERTY implementation
From: Vikash Garodia @ 2026-06-24 7:51 UTC (permalink / raw)
To: Dmitry Baryshkov, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vishnu Reddy
Cc: linux-media, linux-arm-msm, linux-kernel, devicetree,
Dikshita Agarwal
In-Reply-To: <20260616-iris-ar50lt-v5-10-583b42770b6a@oss.qualcomm.com>
On 6/16/2026 5:34 AM, Dmitry Baryshkov wrote:
> AR50Lt with the Gen1 firmware requires host to read
> HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS property, otherwise it doesn't
> update internal data and fails the HFI_CMD_SESSION_LOAD_RESOURCES
> command. Implement minimal support for querying the properties from the
> firmware.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 +
> .../platform/qcom/iris/iris_hfi_gen1_command.c | 21 +++++++++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_defines.h | 15 +++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_response.c | 6 ++++++
> 4 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> index a27447eb2519..16099f9a25b6 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> @@ -121,6 +121,7 @@ struct iris_hfi_session_ops {
> int (*session_set_property)(struct iris_inst *inst,
> u32 packet_type, u32 flag, u32 plane, u32 payload_type,
> void *payload, u32 payload_size);
> + int (*session_get_property)(struct iris_inst *inst, u32 packet_type);
> int (*session_open)(struct iris_inst *inst);
> int (*session_start)(struct iris_inst *inst, u32 plane);
> int (*session_queue_buf)(struct iris_inst *inst, struct iris_buffer *buffer);
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 7674b47ad6c4..99e82e5510ab 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -1117,10 +1117,31 @@ static int iris_hfi_gen1_session_set_config_params(struct iris_inst *inst, u32 p
> return 0;
> }
>
> +static int iris_hfi_gen1_session_get_property(struct iris_inst *inst, u32 packet_type)
> +{
> + struct hfi_session_get_property_pkt pkt;
> + int ret;
> +
> + pkt.shdr.hdr.size = sizeof(pkt);
> + pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_GET_PROPERTY;
> + pkt.shdr.session_id = inst->session_id;
> + pkt.num_properties = 1;
> + pkt.data = packet_type;
> +
> + reinit_completion(&inst->completion);
> +
> + ret = iris_hfi_queue_cmd_write(inst->core, &pkt, pkt.shdr.hdr.size);
> + if (ret)
> + return ret;
> +
> + return iris_wait_for_session_response(inst, false);
> +}
> +
> static const struct iris_hfi_session_ops iris_hfi_gen1_session_ops = {
> .session_open = iris_hfi_gen1_session_open,
> .session_set_config_params = iris_hfi_gen1_session_set_config_params,
> .session_set_property = iris_hfi_gen1_session_set_property,
> + .session_get_property = iris_hfi_gen1_session_get_property,
was looking for caller for this property, saw that in patch#12 to update
the size. Commit should have mentioned about it.
> .session_start = iris_hfi_gen1_session_start,
> .session_queue_buf = iris_hfi_gen1_session_queue_buffer,
> .session_release_buf = iris_hfi_gen1_session_unset_buffers,
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> index 0e4dee192384..bb495a1d2623 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> @@ -35,6 +35,7 @@
> #define HFI_CMD_SESSION_EMPTY_BUFFER 0x211004
> #define HFI_CMD_SESSION_FILL_BUFFER 0x211005
> #define HFI_CMD_SESSION_FLUSH 0x211008
> +#define HFI_CMD_SESSION_GET_PROPERTY 0x211009
> #define HFI_CMD_SESSION_RELEASE_BUFFERS 0x21100b
> #define HFI_CMD_SESSION_RELEASE_RESOURCES 0x21100c
> #define HFI_CMD_SESSION_CONTINUE 0x21100d
> @@ -113,6 +114,7 @@
> #define HFI_MSG_SESSION_FLUSH 0x221006
> #define HFI_MSG_SESSION_EMPTY_BUFFER 0x221007
> #define HFI_MSG_SESSION_FILL_BUFFER 0x221008
> +#define HFI_MSG_SESSION_PROPERTY_INFO 0x221009
> #define HFI_MSG_SESSION_RELEASE_RESOURCES 0x22100a
> #define HFI_MSG_SESSION_RELEASE_BUFFERS 0x22100c
>
> @@ -205,6 +207,12 @@ struct hfi_session_set_property_pkt {
> u32 data[];
> };
>
> +struct hfi_session_get_property_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 data;
> +};
> +
> struct hfi_sys_pc_prep_pkt {
> struct hfi_pkt_hdr hdr;
> };
> @@ -574,6 +582,13 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> u32 data[];
> };
>
> +struct hfi_msg_session_property_info_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 property;
> + u8 data[];
> +};
> +
> struct hfi_msg_session_release_buffers_done_pkt {
> struct hfi_msg_session_hdr_pkt shdr;
> u32 num_buffers;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index bfd7495bf44f..23fc7194b1e3 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -591,6 +591,10 @@ static const struct iris_hfi_gen1_response_pkt_info pkt_infos[] = {
> .pkt = HFI_MSG_SESSION_RELEASE_BUFFERS,
> .pkt_sz = sizeof(struct hfi_msg_session_release_buffers_done_pkt),
> },
> + {
> + .pkt = HFI_MSG_SESSION_PROPERTY_INFO,
> + .pkt_sz = sizeof(struct hfi_msg_session_property_info_pkt),
> + },
> };
>
> static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response)
> @@ -652,6 +656,8 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> iris_hfi_gen1_session_etb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> + } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> + complete(&inst->completion);
> } else {
> struct hfi_msg_session_hdr_pkt *shdr;
>
>
Reviewed-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH] dt-bindings: mmc: Convert TI OMAP2420 MMC to DT schema
From: Krzysztof Kozlowski @ 2026-06-24 7:51 UTC (permalink / raw)
To: Eduard Bostina
Cc: daniel.baluta, simona.toaca, goledhruva, m-chawdhry, Ulf Hansson,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc,
devicetree, linux-kernel
In-Reply-To: <20260623094336.4172741-1-egbostina@gmail.com>
On Tue, Jun 23, 2026 at 09:43:35AM +0000, Eduard Bostina wrote:
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + ti,hwmods:
> + description: |
> + Name of the hwmod associated to the MMC controller. Must be
> + "msdi<n>" where n is the controller instance starting from 1.
> + $ref: /schemas/types.yaml#/definitions/string-array
You should not use different type than already used and currently this
is a string (so also no items below).
> + items:
> + - pattern: "^msdi[0-9]+$"
"^msdi([1-9]|[1-9][0-9]+)$"
> +
> + dmas:
> + maxItems: 2
> +
> + dma-names:
> + items:
> + - const: tx
> + - const: rx
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 3/6] dt-bindings: clock: qcom: Document Nord display clock controller
From: Krzysztof Kozlowski @ 2026-06-24 7:53 UTC (permalink / raw)
To: Taniya Das
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Shawn Guo, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Konrad Dybcio, Ajit Pandey,
Imran Shaik, Jagadeesh Kona, linux-arm-msm, linux-clk,
linux-kernel, devicetree
In-Reply-To: <20260623-nords_mm_v1-v1-3-860c84539804@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 04:24:06PM +0530, Taniya Das wrote:
> Add Device Tree binding documentation for the display clock controller
> on the Qualcomm Nord SoC.
>
> The Nord platform contains two instances of the display clock controller,
> DISPCC_0 and DISPCC_1. Update the bindings to include compatible strings
> for both instances.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
...
> diff --git a/include/dt-bindings/clock/qcom,nord-dispcc.h b/include/dt-bindings/clock/qcom,nord-dispcc.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..9f6c9979e0f358678f28a992af6083b0ae6c97e1
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,nord-dispcc.h
> @@ -0,0 +1,115 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_NORD_H
> +#define _DT_BINDINGS_CLK_QCOM_DISP_CC_NORD_H
> +
> +/* DISP_CC_0 clocks */
Where are clocks/resets/power domains for DISP_CC_1?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: altera: Add Agilex7 Gen2 SoCFPGA compatible strings
From: Krzysztof Kozlowski @ 2026-06-24 7:55 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade
Cc: dinguyen, robh, krzk+dt, conor+dt, devicetree, linux-kernel
In-Reply-To: <20260623111716.16690-2-muhammad.nazim.amirul.nazle.asmade@altera.com>
On Tue, Jun 23, 2026 at 04:17:15AM -0700, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>
> Add the SoC and board compatible strings for the Intel SoCFPGA
> Agilex7 Gen2 platform.
>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> ---
> Documentation/devicetree/bindings/arm/altera.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: socfpga: agilex7-gen2: Add initial device tree
From: Krzysztof Kozlowski @ 2026-06-24 7:57 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade
Cc: dinguyen, robh, krzk+dt, conor+dt, devicetree, linux-kernel
In-Reply-To: <20260623111716.16690-3-muhammad.nazim.amirul.nazle.asmade@altera.com>
On Tue, Jun 23, 2026 at 04:17:16AM -0700, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> + intc: interrupt-controller@7000000 {
MMIO goes to MMIO, please read writing bindings and submitting patches docs in DT dir.
I think this also fails tests (W=1). If that is true, then review should
finish here, because instead of using machine to find issues you use
community.
> + compatible = "arm,gic-v3";
> + reg = <0x0 0x7000000 0x0 0x10000>,
> + <0x0 0x7080000 0x0 0x100000>;
> + ranges;
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x40000>;
> +
> + its: msi-controller@7040000 {
> + compatible = "arm,gic-v3-its";
> + reg = <0x0 0x7040000 0x0 0x20000>;
> + msi-controller;
> + #msi-cells = <1>;
> + };
> + };
> +
> + soc: soc@0 {
> + compatible = "simple-bus";
> + ranges = <0 0 0 0xffffffff>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
> + interrupt-parent = <&intc>;
> +
> + smmu: iommu@c100000 {
> + compatible = "arm,smmu-v3";
> + reg = <0x0c100000 0x30000>;
> + interrupts = <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "eventq", "gerror", "priq";
> + dma-coherent;
> + #iommu-cells = <1>;
> + };
> +
> + ocram: sram@0 {
> + compatible = "mmio-sram";
> + reg = <0x00000000 0x80000>;
> + ranges = <0 0 0x80000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
SoC without any interface, serial or storage or network, is close to
useless one.
I don't see a point in having it in mainline. Serial is usually ABSOLUTE
minimum.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: mediatek: cec: Correct the compatibles for mt7623-mt8167
From: Krzysztof Kozlowski @ 2026-06-24 7:58 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Chun-Kuang Hu, Philipp Zabel, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, CK Hu, Jitao shi,
dri-devel, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260623135757.5111-1-l.scorcia@gmail.com>
On Tue, Jun 23, 2026 at 03:57:53PM +0200, Luca Leonardo Scorcia wrote:
> The HDMI CEC driver for both mt7623 and mt8167 is actually the same as
> mt8173-cec and the mt7623n.dtsi board include file already uses mt8173-cec
> compatible as a fallback, but the documentation lists them as separate
> entries. Correct the binding by adding the correct fallback.
>
> This change fixes a dtbs_check error.
which one? here you paste it (can be trimmed a bit)
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../bindings/display/mediatek/mediatek,cec.yaml | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,cec.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,cec.yaml
> index 080cf321209e..4d741ba415e8 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,cec.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,cec.yaml
> @@ -15,10 +15,13 @@ description: |
>
> properties:
> compatible:
> - enum:
> - - mediatek,mt7623-cec
> - - mediatek,mt8167-cec
> - - mediatek,mt8173-cec
> + oneOf:
> + - const: mediatek,mt8173-cec
> + - items:
> + - enum:
"This change fixes a dtbs_check error."
... and introduces new other errors, so error count stays the same. Not
great.
It does not look like you tested the bindings, at least after quick
look. Please run 'make dt_binding_check' (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/2] arm64: dts: qcom: lemans-evk: Describe the PCIe M.2 Key E connector
From: Konrad Dybcio @ 2026-06-24 7:59 UTC (permalink / raw)
To: Wei Deng, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, quic_chezhou,
cheng.jiang, shuai.zhang, jinwang.li, xiuzhuo.shang, mengshi.wu,
Dmitry Baryshkov, Manivannan Sadhasivam, Bartosz Golaszewski
In-Reply-To: <20260622-v3-lemans-split-v3-2-d26bb22594e3@oss.qualcomm.com>
On 6/22/26 8:41 AM, Wei Deng wrote:
> The lemans EVK has the PCIe M.2 Mechanical Key E connector to connect
> wireless connectivity cards over PCIe and UART interfaces. Hence,
> describe the connector node and link it with the PCIe 0 Root Port and
> UART17 nodes through graph port/endpoint.
>
> The M.2 Key E connector is powered by a 3.3V fixed regulator
> (vreg_wcn_3p3) which is sourced from the board's 12V DC input rail
> (vreg_dcin_12v). Both regulators are always-on and are required by the
> pcie-m2-e-connector binding.
>
> Also add the serial1 = &uart17 alias, which is required for the
> Bluetooth serdev device to be enumerated on the UART17 interface.
>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Wei Deng <wei.deng@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: kodiak: Sort pinctrl subnodes by pins
From: Konrad Dybcio @ 2026-06-24 7:59 UTC (permalink / raw)
To: Luca Weiss, Vladimir Zapolskiy, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <DJFDYAS1AY8Z.3GD4A5UMWLAUC@fairphone.com>
On 6/22/26 9:05 AM, Luca Weiss wrote:
> On Mon Jun 15, 2026 at 2:09 PM CEST, Konrad Dybcio wrote:
>> On 6/12/26 3:46 PM, Luca Weiss wrote:
>>> On Fri Jun 12, 2026 at 2:59 PM CEST, Vladimir Zapolskiy wrote:
>>>> As documented in the "Devicetree Sources (DTS) Coding Style" document,
>>>> pinctrl subnodes should be sorted by the pins property. Do this once for
>>>> kodiak.dtsi so that future additions can be added at the right places.
>>>>
>>>> No functional change intended, verified with dtx_diff.
>>>>
>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/kodiak.dtsi | 1382 +++++++++++++++++-----------------
>>>> 1 file changed, 691 insertions(+), 691 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>> index fa540d8c2615..62daef726d32 100644
>>>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>
>>> <snip>
>>>
>>>> + qup_uart12_cts: qup-uart12-cts-state {
>>>> + pins = "gpio48";
>>>> + function = "qup14";
>>>> + };
>>>> +
>>>> + qup_uart12_rts: qup-uart12-rts-state {
>>>> + pins = "gpio49";
>>>> + function = "qup14";
>>>> + };
>>>> +
>>>> + qup_uart12_tx: qup-uart12-tx-state {
>>>> + pins = "gpio50";
>>>> + function = "qup14";
>>>> + };
>>>>
>>>> I understand and support the intention to keep this change non-functional,
>>>> but this pad "gpio50" is for qup16 also, right?
>>>
>>> According to my QCM6490 data sheet, GPIO_50 has these functions:
>>> * UART for qup14 (OK)
>>> * SPI for qup14 (OK)
>>> * SPI for qup16 (no pinctrl)
>>
>> "no pinctrl" meaning "not defined in the upstream dt as of today"?
>
> Correct.
>
>>>> Similarly pads "gpio54"/"gpio55" for qup14 function, "gpio62"/"gpio63"
>>>> for qup16 function, I find all of these are missing on the original list.
>>>
>>> GPIO_54:
>>> * UART qup15 (OK)
>>> * SPI qup15 (OK)
>>> * SPI qup14 (no pinctrl)
>>>
>>> GPIO_55:
>>> * UART qup15 (OK)
>>> * SPI qup15 (OK)
>>> * SPI qup14 (no pinctrl)
>>>
>>> GPIO_62:
>>> * UART qup17 (OK)
>>> * SPI qup17 (OK)
>>> * SPI qup16 (no pinctrl)
>>>
>>> GPIO_63:
>>> * UART qup16 (?)
>>> * SPI qup16 (lane 3) (?)
>>> * SPI qup16 (lane 5) (?)
>>>
>>> But the GPIO_63 looks weird, is the data sheet wrong?! Where would
>>> UART_RX of QUP1 SE7 go? Maybe it should be UART qup17 and SPI qup17 and
>>> then SPI qup16 ??
>>
>> GPIO63:
>>
>> QUP1_SE6 SPI_CS2
>> QUP1_SE7 UART_RX/SPI_CS0
>
> That matches pinctrl driver and kodiak.dtsi at least. Still, the data
> sheet is just wrong there. If you have any contact to relevant people
> there, please let them know!
Please share the 80-xxxx-xx number and I'll see what I can do
Konrad
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: kodiak: Sort pinctrl subnodes by pins
From: Luca Weiss @ 2026-06-24 8:00 UTC (permalink / raw)
To: Konrad Dybcio, Luca Weiss, Vladimir Zapolskiy, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <99654e01-3a52-4a9e-b003-24230aa6a248@oss.qualcomm.com>
On Wed Jun 24, 2026 at 9:59 AM CEST, Konrad Dybcio wrote:
> On 6/22/26 9:05 AM, Luca Weiss wrote:
>> On Mon Jun 15, 2026 at 2:09 PM CEST, Konrad Dybcio wrote:
>>> On 6/12/26 3:46 PM, Luca Weiss wrote:
>>>> On Fri Jun 12, 2026 at 2:59 PM CEST, Vladimir Zapolskiy wrote:
>>>>> As documented in the "Devicetree Sources (DTS) Coding Style" document,
>>>>> pinctrl subnodes should be sorted by the pins property. Do this once for
>>>>> kodiak.dtsi so that future additions can be added at the right places.
>>>>>
>>>>> No functional change intended, verified with dtx_diff.
>>>>>
>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/qcom/kodiak.dtsi | 1382 +++++++++++++++++-----------------
>>>>> 1 file changed, 691 insertions(+), 691 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>> index fa540d8c2615..62daef726d32 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>>
>>>> <snip>
>>>>
>>>>> + qup_uart12_cts: qup-uart12-cts-state {
>>>>> + pins = "gpio48";
>>>>> + function = "qup14";
>>>>> + };
>>>>> +
>>>>> + qup_uart12_rts: qup-uart12-rts-state {
>>>>> + pins = "gpio49";
>>>>> + function = "qup14";
>>>>> + };
>>>>> +
>>>>> + qup_uart12_tx: qup-uart12-tx-state {
>>>>> + pins = "gpio50";
>>>>> + function = "qup14";
>>>>> + };
>>>>>
>>>>> I understand and support the intention to keep this change non-functional,
>>>>> but this pad "gpio50" is for qup16 also, right?
>>>>
>>>> According to my QCM6490 data sheet, GPIO_50 has these functions:
>>>> * UART for qup14 (OK)
>>>> * SPI for qup14 (OK)
>>>> * SPI for qup16 (no pinctrl)
>>>
>>> "no pinctrl" meaning "not defined in the upstream dt as of today"?
>>
>> Correct.
>>
>>>>> Similarly pads "gpio54"/"gpio55" for qup14 function, "gpio62"/"gpio63"
>>>>> for qup16 function, I find all of these are missing on the original list.
>>>>
>>>> GPIO_54:
>>>> * UART qup15 (OK)
>>>> * SPI qup15 (OK)
>>>> * SPI qup14 (no pinctrl)
>>>>
>>>> GPIO_55:
>>>> * UART qup15 (OK)
>>>> * SPI qup15 (OK)
>>>> * SPI qup14 (no pinctrl)
>>>>
>>>> GPIO_62:
>>>> * UART qup17 (OK)
>>>> * SPI qup17 (OK)
>>>> * SPI qup16 (no pinctrl)
>>>>
>>>> GPIO_63:
>>>> * UART qup16 (?)
>>>> * SPI qup16 (lane 3) (?)
>>>> * SPI qup16 (lane 5) (?)
>>>>
>>>> But the GPIO_63 looks weird, is the data sheet wrong?! Where would
>>>> UART_RX of QUP1 SE7 go? Maybe it should be UART qup17 and SPI qup17 and
>>>> then SPI qup16 ??
>>>
>>> GPIO63:
>>>
>>> QUP1_SE6 SPI_CS2
>>> QUP1_SE7 UART_RX/SPI_CS0
>>
>> That matches pinctrl driver and kodiak.dtsi at least. Still, the data
>> sheet is just wrong there. If you have any contact to relevant people
>> there, please let them know!
>
> Please share the 80-xxxx-xx number and I'll see what I can do
80-20659-1 Rev. AM
Regards
Luca
^ permalink raw reply
* Re: [PATCH v5 11/16] media: iris: update buffer requirements based on received info
From: Vikash Garodia @ 2026-06-24 8:07 UTC (permalink / raw)
To: Dmitry Baryshkov, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vishnu Reddy
Cc: linux-media, linux-arm-msm, linux-kernel, devicetree,
Dikshita Agarwal
In-Reply-To: <20260616-iris-ar50lt-v5-11-583b42770b6a@oss.qualcomm.com>
On 6/16/2026 5:34 AM, Dmitry Baryshkov wrote:
> Upon receiving data for HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS the
> driver should update buffer sizes and counts from the received data.
> Implement corresponding functionality updating buffers data. This will
> be used for upcoming support of AR50Lt platforms with Gen1 firmware.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> .../platform/qcom/iris/iris_hfi_gen1_response.c | 75 +++++++++++++++++++++-
> 1 file changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index 23fc7194b1e3..896953ea62ea 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -533,6 +533,79 @@ static void iris_hfi_gen1_session_ftb_done(struct iris_inst *inst, void *packet)
> dev_err(core->dev, "error in ftb done\n");
> }
>
> +static enum iris_buffer_type iris_hfi_gen1_buf_type(struct iris_inst *inst, u32 type)
> +{
> + switch (type) {
> + case HFI_BUFFER_INPUT:
> + return BUF_INPUT;
> + case HFI_BUFFER_OUTPUT:
> + if (iris_split_mode_enabled(inst))
> + return BUF_DPB;
> + return BUF_OUTPUT;
> + case HFI_BUFFER_OUTPUT2:
> + if (iris_split_mode_enabled(inst))
> + return BUF_OUTPUT;
> + return BUF_DPB;
> + case HFI_BUFFER_INTERNAL_PERSIST_1:
> + return BUF_PERSIST;
> + case HFI_BUFFER_INTERNAL_SCRATCH:
> + return BUF_BIN;
> + case HFI_BUFFER_INTERNAL_SCRATCH_1:
> + return BUF_SCRATCH_1;
> + case HFI_BUFFER_INTERNAL_SCRATCH_2:
> + return BUF_SCRATCH_2;
> + case HFI_BUFFER_INTERNAL_PERSIST:
> + return BUF_ARP;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static void iris_hfi_gen1_session_buffer_requirements(struct iris_inst *inst,
> + void *data, size_t size)
> +{
> + struct hfi_buffer_requirements *req;
> +
> + if (!size || size % sizeof(*req))
> + return;
> +
> + for (req = data; size; size -= sizeof(*req), req++) {
> + enum iris_buffer_type type = iris_hfi_gen1_buf_type(inst, req->type);
> +
> + if (type == -EINVAL)
> + continue;
> +
> + /* on relevant platforms hold_count and min_count are swapped */
> + inst->buffers[type].min_count = req->hold_count;
> + inst->buffers[type].size = req->size;
> +
> + if (type == BUF_OUTPUT)
> + inst->fw_min_count = req->count_actual;
> + }
> +}
> +
> +static void iris_hfi_gen1_session_property_info(struct iris_inst *inst, void *packet)
> +{
> + struct hfi_msg_session_property_info_pkt *pkt = packet;
> +
> + if (!pkt->num_properties) {
> + dev_err(inst->core->dev, "error, no properties\n");
> + goto out;
> + }
> +
> + switch (pkt->property) {
> + case HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS:
If i check the venus message parsing logic, it loops through the packet
till it hits the case. Here, we are assuming the first packet type, in
the pkt to be the property "HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS".
There can be multiple properties in the packet, and if
"HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS" is not the first one, the
usecase would break with incorrect buffer sizes.
> + iris_hfi_gen1_session_buffer_requirements(inst, pkt->data,
> + pkt->shdr.hdr.size - sizeof(*pkt));
> + break;
> + default:
> + dev_warn(inst->core->dev, "unknown property id: %x\n", pkt->property);
> + }
> +
> +out:
> + complete(&inst->completion);
> +}
> +
> struct iris_hfi_gen1_response_pkt_info {
> u32 pkt;
> u32 pkt_sz;
> @@ -657,7 +730,7 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> - complete(&inst->completion);
> + iris_hfi_gen1_session_property_info(inst, hdr);
> } else {
> struct hfi_msg_session_hdr_pkt *shdr;
>
>
Regards,
Vikash
^ permalink raw reply
* Re: [PATCH v7 5/6] remoteproc: qcom: pas: Add late attach support for subsystems
From: Mukesh Ojha @ 2026-06-24 8:08 UTC (permalink / raw)
To: Jingyi Wang
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
Luca Weiss, Bartosz Golaszewski, Kumar Patro, Komal Bajaj,
Konrad Dybcio, aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang,
linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Gokul Krishna Krishnakumar, Shawn Guo
In-Reply-To: <20260623-knp-soccp-v7-5-1ec7bb5c9fec@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 03:05:21AM -0700, Jingyi Wang wrote:
> Subsystems can be brought out of reset by entities such as bootloaders.
> As the irq enablement could be later than subsystem bring up, the state
> of subsystem should be checked by reading SMP2P bits.
>
> A new qcom_pas_attach() function is introduced. if crash state is detected
> for the subsystem, rproc_report_crash() is called. If the ready state is
> detected meanwhile stop state is not detected, it will be marked as
> "attached", otherwise it could be the early boot feature is not supported
> by other entities or it has already been stopped. In above cases, the
> state will be marked as RPROC_OFFLINE so that the PAS driver can load the
> firmware and start the remoteproc.
>
> Co-developed-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
> Signed-off-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
> Tested-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> ---
> drivers/remoteproc/qcom_common.h | 6 ++++
> drivers/remoteproc/qcom_q6v5.c | 3 +-
> drivers/remoteproc/qcom_q6v5_pas.c | 68 ++++++++++++++++++++++++++++++++++++++
> drivers/remoteproc/qcom_sysmon.c | 19 +++++++++++
> 4 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index b07fbaa091a0..b0e7e336d363 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -68,6 +68,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
> int ssctl_instance);
> void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon);
> bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon);
> +bool qcom_sysmon_shutdown_irq_state(struct qcom_sysmon *sysmon);
> #else
> static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
> const char *name,
> @@ -84,6 +85,11 @@ static inline bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)
> {
> return false;
> }
> +
> +static inline bool qcom_sysmon_shutdown_irq_state(struct qcom_sysmon *sysmon)
> +{
> + return false;
> +}
> #endif
>
> #endif
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 58d5b85e58cd..a11d8ace554b 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -202,7 +202,8 @@ int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon)
> q6v5->running = false;
>
> /* Don't perform SMP2P dance if remote isn't running */
> - if (q6v5->rproc->state != RPROC_RUNNING || qcom_sysmon_shutdown_acked(sysmon))
> + if ((q6v5->rproc->state != RPROC_RUNNING && q6v5->rproc->state != RPROC_ATTACHED) ||
> + qcom_sysmon_shutdown_acked(sysmon))
> return 0;
>
> qcom_smem_state_update_bits(q6v5->state,
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 808e9609988d..8a0bb4b2e71c 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -60,6 +60,7 @@ struct qcom_pas_data {
> int region_assign_count;
> bool region_assign_shared;
> int region_assign_vmid;
> + bool early_boot;
> };
>
> struct qcom_pas {
> @@ -507,6 +508,67 @@ static unsigned long qcom_pas_panic(struct rproc *rproc)
> return qcom_q6v5_panic(&pas->q6v5);
> }
>
> +static int qcom_pas_attach(struct rproc *rproc)
> +{
> + struct qcom_pas *pas = rproc->priv;
> + bool ready_state;
> + bool crash_state;
> + bool stop_state;
> + int ret;
> +
> + pas->q6v5.handover_issued = true;
> + enable_irq(pas->q6v5.handover_irq);
> +
> + pas->q6v5.running = true;
> + ret = irq_get_irqchip_state(pas->q6v5.fatal_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &crash_state);
> + if (ret)
> + goto disable_running;
> +
> + if (crash_state) {
> + dev_err(pas->dev, "Subsystem has crashed before driver probe\n");
> + rproc_report_crash(rproc, RPROC_FATAL_ERROR);
> + ret = -EINVAL;
> + goto disable_running;
> + }
> +
> + ret = irq_get_irqchip_state(pas->q6v5.stop_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &stop_state);
> + if (ret)
> + goto disable_running;
> +
> + if (stop_state || qcom_sysmon_shutdown_irq_state(pas->sysmon)) {
> + dev_info(pas->dev, "Subsystem found stop state set. Falling back to start.\n");
> + goto unroll_attach;
> + }
> +
> + ret = irq_get_irqchip_state(pas->q6v5.ready_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &ready_state);
> + if (ret)
> + goto disable_running;
> +
> + if (unlikely(!ready_state)) {
> + /*
> + * The bootloader may not support early boot, mark the state as
> + * RPROC_OFFLINE so that the PAS driver can load the firmware and
> + * start the remoteproc.
> + */
> + dev_err(pas->dev, "Failed to get subsystem ready interrupt\n");
> + goto unroll_attach;
> + }
> +
> + return 0;
> +
> +unroll_attach:
> + pas->rproc->state = RPROC_OFFLINE;
> + ret = -EINVAL;
> +disable_running:
> + disable_irq(pas->q6v5.handover_irq);
> + pas->q6v5.running = false;
> +
> + return ret;
> +}
> +
> static const struct rproc_ops qcom_pas_ops = {
> .unprepare = qcom_pas_unprepare,
> .start = qcom_pas_start,
> @@ -515,6 +577,7 @@ static const struct rproc_ops qcom_pas_ops = {
> .parse_fw = qcom_pas_parse_firmware,
> .load = qcom_pas_load,
> .panic = qcom_pas_panic,
> + .attach = qcom_pas_attach,
> };
>
> static const struct rproc_ops qcom_pas_minidump_ops = {
> @@ -526,6 +589,7 @@ static const struct rproc_ops qcom_pas_minidump_ops = {
> .load = qcom_pas_load,
> .panic = qcom_pas_panic,
> .coredump = qcom_pas_minidump,
> + .attach = qcom_pas_attach,
> };
>
> static int qcom_pas_init_clock(struct qcom_pas *pas)
> @@ -852,6 +916,10 @@ static int qcom_pas_probe(struct platform_device *pdev)
>
> pas->pas_ctx->use_tzmem = rproc->has_iommu;
> pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> +
> + if (desc->early_boot)
> + pas->rproc->state = RPROC_DETACHED;
> +
> ret = rproc_add(rproc);
> if (ret)
> goto remove_ssr_sysmon;
> diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
> index 913e3b750a86..a0830a48b1f4 100644
> --- a/drivers/remoteproc/qcom_sysmon.c
> +++ b/drivers/remoteproc/qcom_sysmon.c
> @@ -736,6 +736,25 @@ bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)
> }
> EXPORT_SYMBOL_GPL(qcom_sysmon_shutdown_acked);
>
> +bool qcom_sysmon_shutdown_irq_state(struct qcom_sysmon *sysmon)
> +{
> + bool shutdown_state;
> + int ret;
> +
> + if (!sysmon)
> + return false;
> +
> + ret = irq_get_irqchip_state(sysmon->shutdown_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &shutdown_state);
> + if (ret) {
> + dev_warn(sysmon->dev, "failed to get shutdown_state: %d\n", ret);
> + return false;
> + }
> +
> + return shutdown_state;
> +}
> +EXPORT_SYMBOL_GPL(qcom_sysmon_shutdown_irq_state);
> +
> /**
> * sysmon_probe() - probe sys_mon channel
> * @rpdev: rpmsg device handle
>
> --
> 2.34.1
>
I tested the series on Hawi., it works but unsure about newly introduced
stop and shutdown status checking why is it required ? I think, you are
checking this for sanity if it is left some random state by the boot
loader..
Tested-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
--
-Mukesh Ojha
^ permalink raw reply
* Re: [PATCH v1 2/8] arm64: dts: qcom: shikra: Add QAIF CPU node for audio
From: Konrad Dybcio @ 2026-06-24 8:20 UTC (permalink / raw)
To: Mohammad Rafi Shaik, Srinivas Kandagatla, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-sound, linux-arm-msm, devicetree, linux-kernel,
Harendra Gautam
In-Reply-To: <20260616201315.2565115-3-mohammad.rafi.shaik@oss.qualcomm.com>
On 6/16/26 10:13 PM, Mohammad Rafi Shaik wrote:
> Add the QAIF CPU endpoint in shikra.dtsi so board files can connect LPASS
> front-end links to backend codecs.
>
> Describe the MMIO region, interrupt, IOMMU mapping and required clocks for
> the QAIF block. Keep the node disabled at SoC level; board dts files enable
> and consume it in subsequent patches.
>
> Co-developed-by: Harendra Gautam <harendra.gautam@oss.qualcomm.com>
> Signed-off-by: Harendra Gautam <harendra.gautam@oss.qualcomm.com>
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/shikra.dtsi | 46 ++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> index bc4ad2bcbbec..e58c87fc8cb0 100644
> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
> @@ -2017,6 +2017,52 @@ dispcc: clock-controller@5f00000 {
> #power-domain-cells = <1>;
> };
>
> + qaif_cpu: audio@a000000 {
> + compatible = "qcom,shikra-qaif-cpu";
> + reg = <0x0 0x0a000000 0x0 0x20000>;
> +
> + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH 0>;
> +
> + clocks = <&gcc GCC_LPASS_CONFIG_CLK>,
> + <&gcc GCC_LPASS_CORE_AXIM_CLK>,
Please align the <s and "s
Konrad
^ 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