* Re: [PATCH] arm64: dts: exynos850: Add SRAM node
From: Krzysztof Kozlowski @ 2026-04-14 9:08 UTC (permalink / raw)
To: Alexey Klimov, Sam Protsenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alim Akhtar
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <DHSR70EGYY4N.2EA2HWIXJR7QR@linaro.org>
On 14/04/2026 11:00, Alexey Klimov wrote:
> On Mon Apr 13, 2026 at 4:23 PM BST, Krzysztof Kozlowski wrote:
>> On 13/04/2026 16:52, Alexey Klimov wrote:
>>> SRAM is used by the ACPM protocol to retrieve the ACPM channels
>>> information and configuration data. Add the SRAM node.
>>>
>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>> ---
>>> arch/arm64/boot/dts/exynos/exynos850.dtsi | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>> index cb55015c8dce..cf4a6168846c 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>> @@ -910,6 +910,14 @@ spi_2: spi@11d20000 {
>>> };
>>> };
>>> };
>>> +
>>> + apm_sram: sram@2039000 {
>>> + compatible = "mmio-sram";
>>> + reg = <0x0 0x2039000 0x40000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges = <0x0 0x0 0x2039000 0x40000>;
>>
>> You miss here children.
>
> Thank you! I guess I should convert it to smth like this:
>
> apm_sram: sram@2039000 {
> compatible = "mmio-sram";
> reg = <0x0 0x2039000 0x40000>;
> ranges = <0x0 0x0 0x2039000 0x40000>;
> #address-cells = <1>;
> #size-cells = <1>;
>
> acpm_sram_region: sram-section@0 {
> reg = <0x0 0x40000>;
This covers entire block, so feels pointless. Maybe requirement of
children should be dropped. What's the point of having children? Why
does the driver need them?
> };
> };
>
> And then later reference shmem = &acpm_sram_region from acpm node.
>
>> Also, 'ranges' should be after 'reg'.
>
> Thanks, will fix this.
>
> FWIW this commit is a copy of commit 48e7821b26904
> https://lore.kernel.org/r/20250207-gs101-acpm-dt-v4-1-230ba8663a2d@linaro.org
Huh, we should fix that one as well.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 00/12] Add Linux RISC-V trace support via CoreSight
From: Zane Leung @ 2026-04-14 9:04 UTC (permalink / raw)
To: Anup Patel
Cc: robh, krzk+dt, conor+dt, palmer, pjw, gregkh, alexander.shishkin,
irogers, coresight, peterz, mingo, namhyung, mark.rutland, jolsa,
adrian.hunter, mchitale, atish.patra, andrew.jones, sunilvl,
linux-riscv, linux-kernel, anup.patel, mayuresh.chitale,
zhuangqiubin, suzuki.poulose, mike.leach, james.clark,
alexander.shishkin, linux-arm-kernel
In-Reply-To: <CAAhSdy2Sx98qcCMBVF+_BV1G-syrO74Y_S298hMwkW+vgLtb5Q@mail.gmail.com>
On 4/14/2026 3:23 PM, Anup Patel wrote:
> On Tue, Apr 14, 2026 at 9:12 AM Zane Leung <liangzhen@linux.spacemit.com> wrote:
>> From: liangzhen <zhen.liang@spacemit.com>
>>
>> This series adds Linux RISC-V trace support via CoreSight, implementing RISC-V
>> trace drivers within the CoreSight framework and integrating them with perf tools.
>> The K3 SoC contains RISC-V Encoder, Funnel, ATB, CoreSight Funnel, and CoreSight TMC
>> components, which can be directly leveraged through the existing CoreSight infrastructure.
>>
>> Linux RISC-V trace support form Anup Patel:
>> (https://patchwork.kernel.org/project/linux-riscv/cover/20260225062448.4027948-1-anup.patel@oss.qualcomm.com/)
>> which currently lacks ATB component support and guidance on reusing CoreSight components.
> What stops you from adding RISC-V trace funnel and ATB bridge drivers
> on top of this series ?
>
Firstly, my works started much earlier than this series. Secondly, it is difficult to add the coresight funnel and coresight tmc components to this series. Based on the coresight framework, I can directly reuse them.
>
>> The series includes:
>> - RISC-V trace driver implementation within the CoreSight framework
> The RISC-V trace very different from the CoreSight framework in many ways:
> 1) Types of components supported
> 2) Trace packet formats
> 3) The way MMIO based components are discoverd
> 4) ... and more ...
1) I believe that RISC-V tracing is coresight-alike, where have encoders/funnel/sink/bridge that are described through DT and controlled by MMIO.
2) I think the difference in package format is nothing more than the coresight frame generated by the ATB component to distinguish the trace source. After removing it, it becomes riscv trace data.
3) The current CoreSight framework code does not introduce this mechanism, it is described through DT.
>> - RISC-V Trace Encoder, Funnel, and ATB Bridge drivers as CoreSight devices
>> - RISC-V trace PMU record capabilities and parsing events in perf.
>> - RISC-V Nexus Trace decoder for perf tools
>>
>> Any comments or suggestions are welcome.
>>
>> Verification on K3 SoC:
>> To verify this patch series on K3 hardware, the following device tree are required:
>> 1. RISC-V Trace Encoder node (8)
>> 2. RISC-V ATB Bridge node (8)
>> 3. RISC-V Trace Funnel node (2)
>> 3. CoreSight Funnel configuration for RISC-V (1)
>> 4. CoreSight TMC configuration for trace buffer (1)
>>
>> /{
>> dummy_clk: apb-pclk {
>> compatible = "fixed-clock";
>> #clock-cells = <0x0>;
>> clock-output-names = "clk14mhz";
>> clock-frequency = <14000000>;
>> };
>>
>>
>> soc: soc {
>> #address-cells = <2>;
>> #size-cells = <2>;
>>
>> encoder0: encoder@d9002000 {
>> compatible = "riscv,trace-encoder";
>> reg = <0x0 0xd9002000 0x0 0x1000>;
>> cpus = <&cpu_0>;
>> out-ports {
>> port {
>> cluster0_encoder0_out_port: endpoint {
>> remote-endpoint = <&cluster0_bridge0_in_port>;
>> };
>> };
>> };
>> };
>>
>> bridge0: bridge@d9003000 {
>> compatible = "riscv,trace-atbbridge";
>> reg = <0x0 0xd9003000 0x0 0x1000>;
>> cpus = <&cpu_0>;
>> out-ports {
>> port {
>> cluster0_bridge0_out_port: endpoint {
>> remote-endpoint = <&cluster0_funnel_in_port0>;
>> };
>> };
>> };
>> in-ports {
>> port {
>> cluster0_bridge0_in_port: endpoint {
>> remote-endpoint = <&cluster0_encoder0_out_port>;
>> };
>> };
>> };
>> };
>>
>> ...
>>
>> cluster0_funnel: funnel@d9000000 {
>> compatible = "riscv,trace-funnel";
>> reg = <0x0 0xd9000000 0x0 0x1000>;
>> cpus = <&cpu_0>, <&cpu_1>, <&cpu_2>, <&cpu_3>;
>> riscv,timestamp-present;
>> out-ports {
>> port {
>> cluster0_funnel_out_port: endpoint {
>> remote-endpoint = <&main_funnel_in_port0>;
>> };
>> };
>> };
>> in-ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> reg = <0>;
>> cluster0_funnel_in_port0: endpoint {
>> remote-endpoint = <&cluster0_bridge0_out_port>;
>> };
>> };
>>
>> port@1 {
>> reg = <1>;
>> cluster0_funnel_in_port1: endpoint {
>> remote-endpoint = <&cluster0_bridge1_out_port>;
>> };
>> };
>>
>> port@2 {
>> reg = <2>;
>> cluster0_funnel_in_port2: endpoint {
>> remote-endpoint = <&cluster0_bridge2_out_port>;
>> };
>> };
>>
>> port@3 {
>> reg = <3>;
>> cluster0_funnel_in_port3: endpoint {
>> remote-endpoint = <&cluster0_bridge3_out_port>;
>> };
>> };
>> };
>> };
>>
>> cluster1_funnel: funnel@d9010000 {
>> compatible = "riscv,trace-funnel";
>> reg = <0x0 0xd9010000 0x0 0x1000>;
>> cpus = <&cpu_4>, <&cpu_5>, <&cpu_6>, <&cpu_7>;
>> riscv,timestamp-present;
>> out-ports {
>> port {
>> cluster1_funnel_out_port: endpoint {
>> remote-endpoint = <&main_funnel_in_port1>;
>> };
>> };
>> };
>> in-ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> reg = <0>;
>> cluster1_funnel_in_port0: endpoint {
>> remote-endpoint = <&cluster1_bridge0_out_port>;
>> };
>> };
>>
>> port@1 {
>> reg = <1>;
>> cluster1_funnel_in_port1: endpoint {
>> remote-endpoint = <&cluster1_bridge1_out_port>;
>> };
>> };
>>
>> port@2 {
>> reg = <2>;
>> cluster1_funnel_in_port2: endpoint {
>> remote-endpoint = <&cluster1_bridge2_out_port>;
>> };
>> };
>>
>> port@3 {
>> reg = <3>;
>> cluster1_funnel_in_port3: endpoint {
>> remote-endpoint = <&cluster1_bridge3_out_port>;
>> };
>> };
>> };
>> };
>>
>> main_funnel: funnel@d9042000 {
>> compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
> Is it legally allowed to mix and match ARM coresight IPs with
> RISC-V trace components at hardware level ?
The ATB Bridge allows sending RISC-V trace to Arm CoreSight infrastructure (instead of RISC-V compliant sink defined in this document) as an ATB initiator. see:
https://github.com/riscv-non-isa/riscv-nexus-trace/blob/105dc1c349556622e4d202d22b584a887ded462f/docs/RISC-V-Trace-Control-Interface.adoc#L184
For ATB Bridge, read trace using Coresight components (ETB/TMC/TPIU). see:
https://github.com/riscv-non-isa/riscv-nexus-trace/blob/105dc1c349556622e4d202d22b584a887ded462f/docs/RISC-V-Trace-Control-Interface.adoc#L1684
>
>> reg = <0x0 0xd9042000 0x0 0x1000>;
>> clocks = <&dummy_clk>;
>> clock-names = "apb_pclk";
>> out-ports {
>> port {
>> main_funnel_out_port: endpoint {
>> remote-endpoint = <&etf_in_port>;
>> };
>> };
>> };
>> in-ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> reg = <0>;
>> main_funnel_in_port0: endpoint {
>> remote-endpoint = <&cluster0_funnel_out_port>;
>> };
>> };
>>
>> port@1 {
>> reg = <1>;
>> main_funnel_in_port1: endpoint {
>> remote-endpoint = <&cluster1_funnel_out_port>;
>> };
>> };
>> };
>> };
>>
>> etf: etf@d9043000 {
>> compatible = "arm,coresight-tmc", "arm,primecell";
>> reg = <0x0 0xd9043000 0x0 0x1000>;
>> clocks = <&dummy_clk>;
>> clock-names = "apb_pclk";
>> out-ports {
>> port {
>> etf_out_port: endpoint {
>> remote-endpoint = <&etr_in_port>;
>> };
>> };
>> };
>> in-ports {
>> port {
>> etf_in_port: endpoint {
>> remote-endpoint = <&main_funnel_out_port>;
>> };
>> };
>> };
>> };
>>
>> etr: etr@d9044000 {
>> compatible = "arm,coresight-tmc", "arm,primecell";
>> reg = <0x0 0xd9044000 0x0 0x1000>;
>> clocks = <&dummy_clk>;
>> clock-names = "apb_pclk";
>> arm,scatter-gather;
>> in-ports {
>> port {
>> etr_in_port: endpoint {
>> remote-endpoint = <&etf_out_port>;
>> };
>> };
>> };
>> };
>> };
>> };
>>
>> Verification case:
>>
>> ~ # perf list pmu
>> rvtrace// [Kernel PMU event]
>>
>> ~ # perf record -e rvtrace/@tmc_etr0/ --per-thread uname
>> Linux
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.191 MB perf.data ]
>> ~ # perf script
>> uname 137 [003] 1 branches: ffffffff80931470 rvtrace_poll_bit+0x38 ([kernel.kallsyms]) => ffffffff80931492 rvtrace_poll_bit+0x5a ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff809328a6 encoder_enable_hw+0x252 ([kernel.kallsyms]) => ffffffff809328ba encoder_enable_hw+0x266 ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff80932c4a encoder_enable+0x82 ([kernel.kallsyms]) => ffffffff80932c36 encoder_enable+0x6e ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff80928198 etm_event_start+0xf0 ([kernel.kallsyms]) => ffffffff809281aa etm_event_start+0x102 ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff809281e6 etm_event_start+0x13e ([kernel.kallsyms]) => ffffffff8092755e coresight_get_sink_id+0x16 ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff8092820e etm_event_start+0x166 ([kernel.kallsyms]) => ffffffff80928226 etm_event_start+0x17e ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff801c3bb4 perf_report_aux_output_id+0x0 ([kernel.kallsyms]) => ffffffff801c3bd6 perf_report_aux_output_id+0x22 ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff801c3c5a perf_report_aux_output_id+0xa6 ([kernel.kallsyms]) => ffffffff801c3bf0 perf_report_aux_output_id+0x3c ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff801c3c40 perf_report_aux_output_id+0x8c ([kernel.kallsyms]) => ffffffff801c3aea __perf_event_header__init_id+0x2a ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff801c3b42 __perf_event_header__init_id+0x82 ([kernel.kallsyms]) => ffffffff801c3b4a __perf_event_header__init_id+0x8a ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff801c3bb0 __perf_event_header__init_id+0xf0 ([kernel.kallsyms]) => ffffffff801c3b58 __perf_event_header__init_id+0x98 ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff8004c658 __task_pid_nr_ns+0x0 ([kernel.kallsyms]) => ffffffff8004c696 __task_pid_nr_ns+0x3e ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff8004c71e __task_pid_nr_ns+0xc6 ([kernel.kallsyms]) => ffffffff8004c6a4 __task_pid_nr_ns+0x4c ([kernel.kallsyms])
>> uname 137 [003] 1 branches: ffffffff8004c6e4 __task_pid_nr_ns+0x8c ([kernel.kallsyms]) => ffffffff8004c6e4 __task_pid_nr_ns+0x8c ([kernel.kallsyms])
>> ...
>>
>> liangzhen (12):
>> coresight: Add RISC-V support to CoreSight tracing
>> coresight: Initial implementation of RISC-V trace driver
>> coresight: Add RISC-V Trace Encoder driver
>> coresight: Add RISC-V Trace Funnel driver
>> coresight: Add RISC-V Trace ATB Bridge driver
>> coresight rvtrace: Add timestamp component support for encoder and
>> funnel
>> coresight: Add RISC-V PMU name support
>> perf tools: riscv: making rvtrace PMU listable
>> perf tools: Add RISC-V trace PMU record capabilities
>> perf tools: Add Nexus RISC-V Trace decoder
>> perf symbols: Add RISC-V PLT entry sizes
>> perf tools: Integrate RISC-V trace decoder into auxtrace
>>
>> drivers/hwtracing/Kconfig | 2 +
>> drivers/hwtracing/coresight/Kconfig | 46 +-
>> drivers/hwtracing/coresight/Makefile | 6 +
>> drivers/hwtracing/coresight/coresight-core.c | 8 +
>> .../hwtracing/coresight/coresight-etm-perf.c | 1 -
>> .../hwtracing/coresight/coresight-etm-perf.h | 21 +
>> .../hwtracing/coresight/coresight-platform.c | 1 -
>> .../hwtracing/coresight/coresight-tmc-etf.c | 4 +
>> .../hwtracing/coresight/coresight-tmc-etr.c | 4 +
>> .../hwtracing/coresight/rvtrace-atbbridge.c | 239 +++
>> drivers/hwtracing/coresight/rvtrace-core.c | 135 ++
>> .../coresight/rvtrace-encoder-core.c | 562 +++++++
>> .../coresight/rvtrace-encoder-sysfs.c | 363 +++++
>> drivers/hwtracing/coresight/rvtrace-encoder.h | 151 ++
>> drivers/hwtracing/coresight/rvtrace-funnel.c | 337 ++++
>> drivers/hwtracing/coresight/rvtrace-funnel.h | 39 +
>> .../hwtracing/coresight/rvtrace-timestamp.c | 278 ++++
>> .../hwtracing/coresight/rvtrace-timestamp.h | 64 +
>> include/linux/coresight-pmu.h | 4 +
>> include/linux/rvtrace.h | 116 ++
>> tools/arch/riscv/include/asm/insn.h | 645 ++++++++
>> tools/perf/arch/riscv/util/Build | 2 +
>> tools/perf/arch/riscv/util/auxtrace.c | 490 ++++++
>> tools/perf/arch/riscv/util/pmu.c | 20 +
>> tools/perf/util/Build | 3 +
>> tools/perf/util/auxtrace.c | 4 +
>> tools/perf/util/auxtrace.h | 1 +
>> tools/perf/util/nexus-rv-decoder/Build | 1 +
>> .../util/nexus-rv-decoder/nexus-rv-decoder.c | 1364 +++++++++++++++++
>> .../util/nexus-rv-decoder/nexus-rv-decoder.h | 139 ++
>> .../perf/util/nexus-rv-decoder/nexus-rv-msg.h | 190 +++
>> tools/perf/util/rvtrace-decoder.c | 1039 +++++++++++++
>> tools/perf/util/rvtrace.h | 40 +
>> tools/perf/util/symbol-elf.c | 4 +
>> 34 files changed, 6320 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-atbbridge.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-core.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-encoder-core.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-encoder-sysfs.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-encoder.h
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-funnel.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-funnel.h
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-timestamp.c
>> create mode 100644 drivers/hwtracing/coresight/rvtrace-timestamp.h
>> create mode 100644 include/linux/rvtrace.h
>> create mode 100644 tools/arch/riscv/include/asm/insn.h
>> create mode 100644 tools/perf/arch/riscv/util/auxtrace.c
>> create mode 100644 tools/perf/arch/riscv/util/pmu.c
>> create mode 100644 tools/perf/util/nexus-rv-decoder/Build
>> create mode 100644 tools/perf/util/nexus-rv-decoder/nexus-rv-decoder.c
>> create mode 100644 tools/perf/util/nexus-rv-decoder/nexus-rv-decoder.h
>> create mode 100644 tools/perf/util/nexus-rv-decoder/nexus-rv-msg.h
>> create mode 100644 tools/perf/util/rvtrace-decoder.c
>> create mode 100644 tools/perf/util/rvtrace.h
>>
>> --
>> 2.34.1
>>
> NACK to this approach of retrofitting RISC-V trace into ARM coresight.
I agree that integrating RISC-V trace directly into CoreSight is not a good approach, so I think we should abstract some of the logic of coresight and reuse it in RISC-V Trace.
>
> Regards,
> Anup
Thanks,
Zane
^ permalink raw reply
* Re: [PATCH] arm64: dts: exynos850: Add SRAM node
From: Alexey Klimov @ 2026-04-14 9:00 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alexey Klimov, Sam Protsenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alim Akhtar
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <2ff077e1-8983-4a41-bb21-5e4140545aa3@kernel.org>
On Mon Apr 13, 2026 at 4:23 PM BST, Krzysztof Kozlowski wrote:
> On 13/04/2026 16:52, Alexey Klimov wrote:
>> SRAM is used by the ACPM protocol to retrieve the ACPM channels
>> information and configuration data. Add the SRAM node.
>>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>> arch/arm64/boot/dts/exynos/exynos850.dtsi | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>> index cb55015c8dce..cf4a6168846c 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>> @@ -910,6 +910,14 @@ spi_2: spi@11d20000 {
>> };
>> };
>> };
>> +
>> + apm_sram: sram@2039000 {
>> + compatible = "mmio-sram";
>> + reg = <0x0 0x2039000 0x40000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x0 0x0 0x2039000 0x40000>;
>
> You miss here children.
Thank you! I guess I should convert it to smth like this:
apm_sram: sram@2039000 {
compatible = "mmio-sram";
reg = <0x0 0x2039000 0x40000>;
ranges = <0x0 0x0 0x2039000 0x40000>;
#address-cells = <1>;
#size-cells = <1>;
acpm_sram_region: sram-section@0 {
reg = <0x0 0x40000>;
};
};
And then later reference shmem = &acpm_sram_region from acpm node.
> Also, 'ranges' should be after 'reg'.
Thanks, will fix this.
FWIW this commit is a copy of commit 48e7821b26904
https://lore.kernel.org/r/20250207-gs101-acpm-dt-v4-1-230ba8663a2d@linaro.org
Best regards,
Alexey
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Add Broadcom channel priority commands
From: Neal Gompa @ 2026-04-14 8:59 UTC (permalink / raw)
To: fnkl.kernel
Cc: Sven Peter, Janne Grunau, Marcel Holtmann, Luiz Augusto von Dentz,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, asahi, linux-arm-kernel,
linux-bluetooth, netdev
In-Reply-To: <20260407-brcm-prio-v2-1-3f745edf49af@gmail.com>
On Tue, Apr 7, 2026 at 1:46 PM Sasha Finkelstein via B4 Relay
<devnull+fnkl.kernel.gmail.com@kernel.org> wrote:
>
> From: Sasha Finkelstein <fnkl.kernel@gmail.com>
>
> Certain Broadcom bluetooth chips (bcm4377/bcm4378/bcm438) need ACL
> streams carrying audio to be set as "high priority" using a vendor
> specific command to prevent 10-ish second-long dropouts whenever
> something does a device scan. This patch sends the command when the
> socket priority is set to TC_PRIO_INTERACTIVE, as BlueZ does for audio.
>
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
> Changes in v2:
> - new ioctl got nack-ed, so let's use sk_priority as the trigger
> - Link to v1: https://lore.kernel.org/r/20260407-brcm-prio-v1-1-f38b17376640@gmail.com
> ---
Thank you so much for this!
Reviewed-by: Neal Gompa <neal@gompa.dev>
--
真実はいつも一つ！/ Always, there's only one truth!
^ permalink raw reply
* Re: [net-next] net: airoha: Wait for TX to complete in airoha_dev_stop()
From: Lorenzo Bianconi @ 2026-04-14 8:57 UTC (permalink / raw)
To: Paolo Abeni
Cc: edumazet, kuba, davem, netdev, andrew+netdev, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20260414082949.104611-1-pabeni@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: airoha: Wait for TX to complete in airoha_dev_stop()
>
> Wait for TX to complete in airoha_dev_stop routine before stopping the
> TX DMA and run airoha_qdma_cleanup_tx_queue routine. Moreover,
> start/stop TX/RX NAPIs in ndo_open()/ndo_stop() callbacks in order to be
> sure the TX NAPIs have completed before stopping the TX DMA engine in
> airoha_dev_stop routine.
>
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 8e4b043af4bc..9e40c8f375c1 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
>
> [ ... ]
>
> > @@ -1684,18 +1686,26 @@ static int airoha_dev_stop(struct net_device *dev)
> > struct airoha_qdma *qdma = port->qdma;
> > int i, err;
> >
> > - netif_tx_disable(dev);
> > err = airoha_set_vip_for_gdm_port(port, false);
> > if (err)
> > return err;
> >
> > - for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> > - netdev_tx_reset_subqueue(dev, i);
> > -
> > airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> > FE_PSE_PORT_DROP);
> >
> > + netif_tx_disable(dev);
> > if (atomic_dec_and_test(&qdma->users)) {
> > + u32 val;
> > +
> > + /* Wait for TX to complete */
> > + err = read_poll_timeout(airoha_qdma_rr, val,
> > + !(val & GLOBAL_CFG_TX_DMA_BUSY_MASK),
> > + USEC_PER_MSEC, 100 * USEC_PER_MSEC,
> > + false, qdma, REG_QDMA_GLOBAL_CFG);
> > + if (err)
> > + return err;
> > ^^^^^^^^^^
>
> Can this leave the device in an inconsistent state? When read_poll_timeout()
> fails (TX DMA busy for more than 100ms), airoha_dev_stop() returns an error
> immediately. However, atomic_dec_and_test(&qdma->users) has already
> decremented qdma->users to 0, but airoha_qdma_stop_napi() was never called,
> so all NAPIs remain enabled.
>
> The networking core (__dev_close_many in net/core/dev.c) ignores the
> ndo_stop return value and marks the device as down regardless. When the
> device is later brought up again, airoha_dev_open() sees atomic_fetch_inc()
> return 0 (old value) and calls airoha_qdma_start_napi() -> napi_enable(),
> which hits BUG_ON(!test_bit(NAPI_STATE_SCHED, &val)) in napi_enable_locked()
> because the NAPI was never disabled.
>
> The early return also skips the netdev_tx_reset_subqueue() calls at the end
> of the function.
>
> Should the code either continue with cleanup on timeout (don't return), or
> ensure NAPI and DMA state are properly cleaned up before returning?
ack, I will fix it in v2.
Regards,
Lorenzo
>
> > +
> > + airoha_qdma_stop_napi(qdma);
> > airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> > GLOBAL_CFG_TX_DMA_EN_MASK |
> > GLOBAL_CFG_RX_DMA_EN_MASK);
>
> [ ... ]
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] mmc: dw_mmc: implement option for configuring DMA threshold
From: Shawn Lin @ 2026-04-14 8:50 UTC (permalink / raw)
To: Kaustabh Chakraborty, Ulf Hansson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaehoon Chung,
Krzysztof Kozlowski, Alim Akhtar
Cc: shawn.lin, linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc
In-Reply-To: <20260414-dwmmc-dma-thr-v2-1-4058078f5361@disroot.org>
在 2026/04/14 星期二 16:36, Kaustabh Chakraborty 写道:
> Some controllers, such as certain Exynos SDIO ones, are unable to
> perform DMA transfers of small amount of bytes properly. Following the
> device tree schema, implement the property to define the DMA transfer
> threshold (from a hard coded value of 16 bytes) so that lesser number of
> bytes can be transferred safely skipping DMA in such controllers. The
> value of 16 bytes stays as the default for controllers which do not
> define it. This value can be overridden by implementation-specific init
> sequences.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> drivers/mmc/host/dw_mmc.c | 5 +++--
> drivers/mmc/host/dw_mmc.h | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 20193ee7b73eb..9dd9fed4ccf49 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -40,7 +40,6 @@
> SDMMC_INT_RESP_ERR | SDMMC_INT_HLE)
> #define DW_MCI_ERROR_FLAGS (DW_MCI_DATA_ERROR_FLAGS | \
> DW_MCI_CMD_ERROR_FLAGS)
> -#define DW_MCI_DMA_THRESHOLD 16
>
> #define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
> #define DW_MCI_FREQ_MIN 100000 /* unit: HZ */
> @@ -821,7 +820,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
> * non-word-aligned buffers or lengths. Also, we don't bother
> * with all the DMA setup overhead for short transfers.
> */
> - if (data->blocks * data->blksz < DW_MCI_DMA_THRESHOLD)
> + if (data->blocks * data->blksz < host->dma_threshold)
> return -EINVAL;
>
> if (data->blksz & 3)
> @@ -3245,6 +3244,8 @@ int dw_mci_probe(struct dw_mci *host)
> goto err_clk_ciu;
> }
>
> + host->dma_threshold = 16;
I'd prefer to set it in dw_mci_alloc_host() instead of picking up
a random place to put it, for better code management.
> +
> if (host->rstc) {
> reset_control_assert(host->rstc);
> usleep_range(10, 50);
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 42e58be74ce09..fc7601fba849f 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -164,6 +164,8 @@ struct dw_mci {
> void __iomem *fifo_reg;
> u32 data_addr_override;
> bool wm_aligned;
> + /* Configurable data byte threshold value for DMA transfer. */
No here, there is a long section of comment before struct dw_mci{ } that
describes each member of it, please add it there.
> + u32 dma_threshold;
>
> struct scatterlist *sg;
> struct sg_mapping_iter sg_miter;
>
^ permalink raw reply
* [PATCH v2 1/2] mmc: dw_mmc: implement option for configuring DMA threshold
From: Kaustabh Chakraborty @ 2026-04-14 8:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jaehoon Chung, Shawn Lin, Krzysztof Kozlowski, Alim Akhtar
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, Kaustabh Chakraborty
In-Reply-To: <20260414-dwmmc-dma-thr-v2-0-4058078f5361@disroot.org>
Some controllers, such as certain Exynos SDIO ones, are unable to
perform DMA transfers of small amount of bytes properly. Following the
device tree schema, implement the property to define the DMA transfer
threshold (from a hard coded value of 16 bytes) so that lesser number of
bytes can be transferred safely skipping DMA in such controllers. The
value of 16 bytes stays as the default for controllers which do not
define it. This value can be overridden by implementation-specific init
sequences.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/mmc/host/dw_mmc.c | 5 +++--
drivers/mmc/host/dw_mmc.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 20193ee7b73eb..9dd9fed4ccf49 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -40,7 +40,6 @@
SDMMC_INT_RESP_ERR | SDMMC_INT_HLE)
#define DW_MCI_ERROR_FLAGS (DW_MCI_DATA_ERROR_FLAGS | \
DW_MCI_CMD_ERROR_FLAGS)
-#define DW_MCI_DMA_THRESHOLD 16
#define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
#define DW_MCI_FREQ_MIN 100000 /* unit: HZ */
@@ -821,7 +820,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
* non-word-aligned buffers or lengths. Also, we don't bother
* with all the DMA setup overhead for short transfers.
*/
- if (data->blocks * data->blksz < DW_MCI_DMA_THRESHOLD)
+ if (data->blocks * data->blksz < host->dma_threshold)
return -EINVAL;
if (data->blksz & 3)
@@ -3245,6 +3244,8 @@ int dw_mci_probe(struct dw_mci *host)
goto err_clk_ciu;
}
+ host->dma_threshold = 16;
+
if (host->rstc) {
reset_control_assert(host->rstc);
usleep_range(10, 50);
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 42e58be74ce09..fc7601fba849f 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -164,6 +164,8 @@ struct dw_mci {
void __iomem *fifo_reg;
u32 data_addr_override;
bool wm_aligned;
+ /* Configurable data byte threshold value for DMA transfer. */
+ u32 dma_threshold;
struct scatterlist *sg;
struct sg_mapping_iter sg_miter;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/2] Configuring DMA threshold value for DW-MMC controllers
From: Kaustabh Chakraborty @ 2026-04-14 8:35 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jaehoon Chung, Shawn Lin, Krzysztof Kozlowski, Alim Akhtar
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, Kaustabh Chakraborty
In Samsung Exynos 7870 devices with Broadcom Wi-Fi, it has been observed
that small sized DMA transfers are unreliable and are not written
properly, which renders the cache incoherent.
Experimental observations say that DMA transfer sizes of somewhere
around 64 to 512 are intolerable. We must thus implement a mechanism to
fall back to PIO transfer in this case. One such approach, which this
series implements is allowing the DMA transfer threshold, which is
already defined in the driver, to be configurable.
Note that this patch is likely to be labelled as a workaround. These
smaller transfers seem to be successful from downstream kernels,
however efforts to figure out how so went in vain. It is also very
possible that the downstream Broadcom Wi-Fi SDIO driver uses PIO
transfers as well.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Changes in v2:
- Remove dt-binding to set DMA threshold (Krzysztof Kozlowski)
- Add comment to describe struct dw_mci::dma_threshold (Shawn Lin)
- Set DMA threshold in Exynos 7870 DW-MMC driver (Krzysztof Kozlowski)
- Link to v1: https://lore.kernel.org/r/20260412-dwmmc-dma-thr-v1-0-75a2f658eee3@disroot.org
---
Kaustabh Chakraborty (2):
mmc: dw_mmc: implement option for configuring DMA threshold
mmc: dw_mmc: exynos: increase DMA threshold value for exynos7870
drivers/mmc/host/dw_mmc-exynos.c | 1 +
drivers/mmc/host/dw_mmc.c | 5 +++--
drivers/mmc/host/dw_mmc.h | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
---
base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
change-id: 20260412-dwmmc-dma-thr-1090d8285ea7
Best regards,
--
Kaustabh Chakraborty <kauschluss@disroot.org>
^ permalink raw reply
* [PATCH v2 2/2] mmc: dw_mmc: exynos: increase DMA threshold value for exynos7870
From: Kaustabh Chakraborty @ 2026-04-14 8:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jaehoon Chung, Shawn Lin, Krzysztof Kozlowski, Alim Akhtar
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, Kaustabh Chakraborty
In-Reply-To: <20260414-dwmmc-dma-thr-v2-0-4058078f5361@disroot.org>
Exynos 7870 compatible controllers, such as SDIO ones are not able to
perform DMA transfers for small sizes of data (~16 to ~512 bytes),
resulting in cache issues in subsequent transfers. Increase the DMA
transfer threshold to 512 to allow the shorter transfers to take place,
bypassing DMA.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/mmc/host/dw_mmc-exynos.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 261344d3a8cfe..4b76b997ddc15 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -141,6 +141,7 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
priv->ctrl_type == DW_MCI_TYPE_EXYNOS7870_SMU) {
/* Quirk needed for certain Exynos SoCs */
host->quirks |= DW_MMC_QUIRK_FIFO64_32;
+ host->dma_threshold = 512;
}
if (priv->ctrl_type == DW_MCI_TYPE_ARTPEC8) {
--
2.53.0
^ permalink raw reply related
* Re: [net-next] net: airoha: Wait for TX to complete in airoha_dev_stop()
From: Paolo Abeni @ 2026-04-14 8:29 UTC (permalink / raw)
To: lorenzo
Cc: edumazet, kuba, davem, netdev, andrew+netdev, pabeni,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20260411-airoha-fix-ndo_stop-v1-1-caddaa181739@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: airoha: Wait for TX to complete in airoha_dev_stop()
Wait for TX to complete in airoha_dev_stop routine before stopping the
TX DMA and run airoha_qdma_cleanup_tx_queue routine. Moreover,
start/stop TX/RX NAPIs in ndo_open()/ndo_stop() callbacks in order to be
sure the TX NAPIs have completed before stopping the TX DMA engine in
airoha_dev_stop routine.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 8e4b043af4bc..9e40c8f375c1 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -1684,18 +1686,26 @@ static int airoha_dev_stop(struct net_device *dev)
> struct airoha_qdma *qdma = port->qdma;
> int i, err;
>
> - netif_tx_disable(dev);
> err = airoha_set_vip_for_gdm_port(port, false);
> if (err)
> return err;
>
> - for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> - netdev_tx_reset_subqueue(dev, i);
> -
> airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> FE_PSE_PORT_DROP);
>
> + netif_tx_disable(dev);
> if (atomic_dec_and_test(&qdma->users)) {
> + u32 val;
> +
> + /* Wait for TX to complete */
> + err = read_poll_timeout(airoha_qdma_rr, val,
> + !(val & GLOBAL_CFG_TX_DMA_BUSY_MASK),
> + USEC_PER_MSEC, 100 * USEC_PER_MSEC,
> + false, qdma, REG_QDMA_GLOBAL_CFG);
> + if (err)
> + return err;
> ^^^^^^^^^^
Can this leave the device in an inconsistent state? When read_poll_timeout()
fails (TX DMA busy for more than 100ms), airoha_dev_stop() returns an error
immediately. However, atomic_dec_and_test(&qdma->users) has already
decremented qdma->users to 0, but airoha_qdma_stop_napi() was never called,
so all NAPIs remain enabled.
The networking core (__dev_close_many in net/core/dev.c) ignores the
ndo_stop return value and marks the device as down regardless. When the
device is later brought up again, airoha_dev_open() sees atomic_fetch_inc()
return 0 (old value) and calls airoha_qdma_start_napi() -> napi_enable(),
which hits BUG_ON(!test_bit(NAPI_STATE_SCHED, &val)) in napi_enable_locked()
because the NAPI was never disabled.
The early return also skips the netdev_tx_reset_subqueue() calls at the end
of the function.
Should the code either continue with cleanup on timeout (don't return), or
ensure NAPI and DMA state are properly cleaned up before returning?
> +
> + airoha_qdma_stop_napi(qdma);
> airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> GLOBAL_CFG_TX_DMA_EN_MASK |
> GLOBAL_CFG_RX_DMA_EN_MASK);
[ ... ]
^ permalink raw reply
* [PATCH v2] PCI: host-common: Request bus reassignment when not probe-only
From: Ratheesh Kannoth @ 2026-04-14 8:17 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-kernel, bhelgaas
Cc: will, lpieralisi, kwilczynski, mani, robh, vidyas,
Ratheesh Kannoth, Bjorn Helgaas
pci_host_common_init() is used by several generic ECAM host drivers.
After PCI core changes around pci_flags and preserve_config, these hosts
no longer opted into full bus number reassignment the way they did
before.
When PCI_PROBE_ONLY is not set, add PCI_REASSIGN_ALL_BUS so
pci_scan_bridge_extend() takes the reassignment path: bus numbers can be
assigned from firmware EA data (e.g. pci_ea_fixed_busnrs()). Skip the
flag in probe-only mode so existing assignments are not overridden.
CC: Bjorn Helgaas <helgaas@kernel.org>
CC: Vidya Sagar <vidyas@nvidia.com>
CC: Manivannan Sadhasivam <mani@kernel.org>
Fixes: 7246a4520b4b ("PCI: Use preserve_config in place of pci_flags")
Link: https://lore.kernel.org/netdev/adcXzcz2wWJFw4d7@rkannoth-OptiPlex-7090/
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
v1 -> v2 : https://lore.kernel.org/linux-pci/20260410142124.2673056-1-rkannoth@marvell.com/
---
drivers/pci/controller/pci-host-common.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index d6258c1cffe5..99952fb7189b 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -68,6 +68,10 @@ int pci_host_common_init(struct platform_device *pdev,
if (IS_ERR(cfg))
return PTR_ERR(cfg);
+ /* Do not reassign bus numbers if probe only */
+ if (!pci_has_flag(PCI_PROBE_ONLY))
+ pci_add_flags(PCI_REASSIGN_ALL_BUS);
+
bridge->sysdata = cfg;
bridge->ops = (struct pci_ops *)&ops->pci_ops;
bridge->enable_device = ops->enable_device;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 3/9] coresight: etm4x: fix leaked trace id
From: Jie Gan @ 2026-04-14 8:04 UTC (permalink / raw)
To: Yeoreum Yun, coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan
In-Reply-To: <20260413142003.3549310-4-yeoreum.yun@arm.com>
On 4/13/2026 10:19 PM, Yeoreum Yun wrote:
> If etm4_enable_sysfs() fails in cscfg_csdev_enable_active_config(),
> the trace ID may be leaked because it is not released.
>
> To address this, call etm4_release_trace_id() when etm4_enable_sysfs()
> fails in cscfg_csdev_enable_active_config().
>
LGTM.
Reviewed-by: Jie Gan <jie.gan@oss.qualcomm.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 8ebfd3924143..1bc9f13e33f7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -918,8 +918,10 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
> cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
> if (cfg_hash) {
> ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
> - if (ret)
> + if (ret) {
> + etm4_release_trace_id(drvdata);
> return ret;
> + }
> }
>
> raw_spin_lock(&drvdata->spinlock);
^ permalink raw reply
* Re: [PATCH v4 2/9] coresight: etm4x: exclude ss_status from drvdata->config
From: Jie Gan @ 2026-04-14 8:02 UTC (permalink / raw)
To: Yeoreum Yun, coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan
In-Reply-To: <20260413142003.3549310-3-yeoreum.yun@arm.com>
On 4/13/2026 10:19 PM, Yeoreum Yun wrote:
> The purpose of TRCSSCSRn register is to show status of
> the corresponding Single-shot Comparator Control and input supports.
> That means writable field's purpose for reset or restore from idle status
> not for configuration.
>
> Therefore, exclude ss_status from drvdata->config, move it to etm4x_caps.
> This includes remove TRCSSCRn from configurable item and
> remove saving in etm4_disable_hw().
>
LGTM.
Reviewed-by: Jie Gan <jie.gan@oss.qualcomm.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> .../hwtracing/coresight/coresight-etm4x-cfg.c | 1 -
> .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
> .../coresight/coresight-etm4x-sysfs.c | 7 ++-----
> drivers/hwtracing/coresight/coresight-etm4x.h | 4 +++-
> 4 files changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> index c302072b293a..d14d7c8a23e5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> @@ -86,7 +86,6 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata,
> off_mask = (offset & GENMASK(11, 5));
> do {
> CHECKREGIDX(TRCSSCCRn(0), ss_ctrl, idx, off_mask);
> - CHECKREGIDX(TRCSSCSRn(0), ss_status, idx, off_mask);
> CHECKREGIDX(TRCSSPCICRn(0), ss_pe_cmp, idx, off_mask);
> } while (0);
> } else if ((offset >= TRCCIDCVRn(0)) && (offset <= TRCVMIDCVRn(7))) {
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 6443f3717b37..8ebfd3924143 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -91,7 +91,7 @@ static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> const struct etmv4_caps *caps = &drvdata->caps;
>
> return (n < caps->nr_ss_cmp) && caps->nr_pe &&
> - (drvdata->config.ss_status[n] & TRCSSCSRn_PC);
> + (caps->ss_status[n] & TRCSSCSRn_PC);
> }
>
> u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit)
> @@ -571,11 +571,9 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
>
> for (i = 0; i < caps->nr_ss_cmp; i++) {
> - /* always clear status bit on restart if using single-shot */
> - if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> - config->ss_status[i] &= ~TRCSSCSRn_STATUS;
> etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
> - etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
> + /* always clear status and pending bits on restart if using single-shot */
> + etm4x_relaxed_write32(csa, caps->ss_status[i], TRCSSCSRn(i));
> if (etm4x_sspcicrn_present(drvdata, i))
> etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
> }
> @@ -1053,12 +1051,6 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
>
> etm4_disable_trace_unit(drvdata);
>
> - /* read the status of the single shot comparators */
> - for (i = 0; i < caps->nr_ss_cmp; i++) {
> - config->ss_status[i] =
> - etm4x_relaxed_read32(csa, TRCSSCSRn(i));
> - }
> -
> /* read back the current counter values */
> for (i = 0; i < caps->nr_cntr; i++) {
> config->cntr_val[i] =
> @@ -1501,8 +1493,8 @@ static void etm4_init_arch_data(void *info)
> */
> caps->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4);
> for (i = 0; i < caps->nr_ss_cmp; i++) {
> - drvdata->config.ss_status[i] =
> - etm4x_relaxed_read32(csa, TRCSSCSRn(i));
> + caps->ss_status[i] = etm4x_relaxed_read32(csa, TRCSSCSRn(i));
> + caps->ss_status[i] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING);
> }
> /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */
> caps->numcidc = FIELD_GET(TRCIDR4_NUMCIDC_MASK, etmidr4);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 50408215d1ac..dd62f01674cf 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1827,8 +1827,6 @@ static ssize_t sshot_ctrl_store(struct device *dev,
> raw_spin_lock(&drvdata->spinlock);
> idx = config->ss_idx;
> config->ss_ctrl[idx] = FIELD_PREP(TRCSSCCRn_SAC_ARC_RST_MASK, val);
> - /* must clear bit 31 in related status register on programming */
> - config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
> raw_spin_unlock(&drvdata->spinlock);
> return size;
> }
> @@ -1839,10 +1837,11 @@ static ssize_t sshot_status_show(struct device *dev,
> {
> unsigned long val;
> struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + const struct etmv4_caps *caps = &drvdata->caps;
> struct etmv4_config *config = &drvdata->config;
>
> raw_spin_lock(&drvdata->spinlock);
> - val = config->ss_status[config->ss_idx];
> + val = caps->ss_status[config->ss_idx];
> raw_spin_unlock(&drvdata->spinlock);
> return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> }
> @@ -1877,8 +1876,6 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev,
> raw_spin_lock(&drvdata->spinlock);
> idx = config->ss_idx;
> config->ss_pe_cmp[idx] = FIELD_PREP(TRCSSPCICRn_PC_MASK, val);
> - /* must clear bit 31 in related status register on programming */
> - config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
> raw_spin_unlock(&drvdata->spinlock);
> return size;
> }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 8168676f2945..8864cfb76bad 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -213,6 +213,7 @@
> #define TRCACATRn_EXLEVEL_MASK GENMASK(14, 8)
>
> #define TRCSSCSRn_STATUS BIT(31)
> +#define TRCSSCSRn_PENDING BIT(30)
> #define TRCSSCCRn_SAC_ARC_RST_MASK GENMASK(24, 0)
>
> #define TRCSSPCICRn_PC_MASK GENMASK(7, 0)
> @@ -861,6 +862,7 @@ enum etm_impdef_type {
> * @lpoverride: If the implementation can support low-power state over.
> * @skip_power_up: Indicates if an implementation can skip powering up
> * the trace unit.
> + * @ss_status: The status of the corresponding single-shot comparator.
> */
> struct etmv4_caps {
> u8 nr_pe;
> @@ -899,6 +901,7 @@ struct etmv4_caps {
> bool atbtrig : 1;
> bool lpoverride : 1;
> bool skip_power_up : 1;
> + u32 ss_status[ETM_MAX_SS_CMP];
> };
>
> /**
> @@ -977,7 +980,6 @@ struct etmv4_config {
> u32 res_ctrl[ETM_MAX_RES_SEL]; /* TRCRSCTLRn */
> u8 ss_idx;
> u32 ss_ctrl[ETM_MAX_SS_CMP];
> - u32 ss_status[ETM_MAX_SS_CMP];
> u32 ss_pe_cmp[ETM_MAX_SS_CMP];
> u8 addr_idx;
> u64 addr_val[ETM_MAX_SINGLE_ADDR_CMP];
^ permalink raw reply
* RE: [PATCH v7 0/4] PCI: Add support for resetting the Root Ports in a platform specific way
From: Hongxing Zhu @ 2026-04-14 7:59 UTC (permalink / raw)
To: Manivannan Sadhasivam, Brian Norris
Cc: manivannan.sadhasivam@oss.qualcomm.com, Bjorn Helgaas,
Mahesh J Salgaonkar, Oliver O'Halloran, Will Deacon,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Heiko Stuebner, Philipp Zabel, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-rockchip@lists.infradead.org,
Niklas Cassel, Wilfred Mallawa, Krishna Chaitanya Chundru,
Lukas Wunner, Wilson Ding, Miles Chen
In-Reply-To: <klzq2i2ne62hdri65gz7s5pxmvely277optr2lrkvdrrahl3ca@k3hdo6o4nkjz>
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: 2026年4月14日 0:35
> To: Brian Norris <briannorris@chromium.org>; Hongxing Zhu
> <hongxing.zhu@nxp.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>;
> manivannan.sadhasivam@oss.qualcomm.com; Bjorn Helgaas
> <bhelgaas@google.com>; Mahesh J Salgaonkar <mahesh@linux.ibm.com>;
> Oliver O'Halloran <oohall@gmail.com>; Will Deacon <will@kernel.org>;
> Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof Wilczyński
> <kwilczynski@kernel.org>; Rob Herring <robh@kernel.org>; Heiko Stuebner
> <heiko@sntech.de>; Philipp Zabel <p.zabel@pengutronix.de>;
> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-arm-msm@vger.kernel.org; linux-rockchip@lists.infradead.org; Niklas
> Cassel <cassel@kernel.org>; Wilfred Mallawa <wilfred.mallawa@wdc.com>;
> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>; Lukas
> Wunner <lukas@wunner.de>; Wilson Ding <dingwei@marvell.com>; Miles
> Chen <minhuachen@google.com>
> Subject: Re: [PATCH v7 0/4] PCI: Add support for resetting the Root Ports in a
> platform specific way
>
> Hi Brian,
>
> On Wed, Apr 08, 2026 at 06:58:34PM -0700, Brian Norris wrote:
> > Hi Richard and Mani,
> >
> > For the record, I've been using a form of an earlier version of this
> > patchset in my environment for some time now, and I've run across
> > problems that *might* relate to what Richard is reporting, but I'm not
> > quite sure at the moment. Details below.
> >
> > On Wed, Mar 25, 2026 at 07:06:49AM +0000, Hongxing Zhu wrote:
> > > Hi Mani:
> > > I've accidentally encountered a new issue based on the reset root port
> patch-set.
> > > After performing a few hot-reset operations, the PCIe link enters a
> continuous up/down cycling pattern.
> > >
> > > I found that calling pci_reset_secondary_bus() first in
> pcibios_reset_secondary_bus() appears to resolve this issue.
> > > Have you experienced a similar problem?
> > >
> > > "
> > > ...
> > > [ 141.897701] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000700)
> > > link up detected [ 142.086341] imx6q-pcie 4c300000.pcie: PCIe Gen.3
> > > x1 link up [ 142.092038] imx6q-pcie 4c300000.pcie:
> > > PCIe(LNK_STS:0x00000c00) link down detected ...
> > > "
> > >
> > > Platform: i.MX95 EVK board plus local Root Ports reset supports based
> on the #1 and #2 patches of v7 patch-set.
> > > Notes of the logs:
> > > - One Gen3 NVME device is connected.
> > > - "./memtool 4c341058=0;./memtool 4c341058=1;" is used to toggle the
> LTSSM_EN bit to trigger the link down.
> > > - Toggle BIT6 of Bridge Control Register to trigger hot reset by
> "./memtool 4c30003c=004001ff; ./memtool 4c30003c=000001ff;"
> > > - The Root Port reset patches works correctly at first.
> > > However, after several hot-reset triggers, the link enters a repeated
> down/up cycling state.
> > >
> > > Logs:
> > > [ 3.553188] imx6q-pcie 4c300000.pcie: host bridge
> /soc/pcie@4c300000 ranges:
> > > [ 3.560308] imx6q-pcie 4c300000.pcie: IO
> 0x006ff00000..0x006fffffff -> 0x0000000000
> > > [ 3.568525] imx6q-pcie 4c300000.pcie: MEM
> 0x0910000000..0x091fffffff -> 0x0010000000
> > > [ 3.577314] imx6q-pcie 4c300000.pcie: config reg[1] 0x60100000 ==
> cpu 0x60100000
> > > [ 3.796029] imx6q-pcie 4c300000.pcie: iATU: unroll T, 128 ob, 128 ib,
> align 4K, limit 1024G
> > > [ 4.003746] imx6q-pcie 4c300000.pcie: PCIe Gen.3 x1 link up
> > > [ 4.009553] imx6q-pcie 4c300000.pcie: PCI host bridge to bus 0000:00
> > > root@imx95evk:~#
> > > root@imx95evk:~#
> > > root@imx95evk:~# ./memtool 4c341058=0;./memtool 4c341058=1;
> Writing
> > > 32-bit value 0x0 to address 0x4C341058 Writing 32-bit v
> > > [ 87.265348] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000d01)
> link down detected
> > > alue 0x1 to adder
> > > [ 87.273106] imx6q-pcie 4c300000.pcie: Stop root bus and handle link
> down
> > > ss 0x4C341058
> > > [ 87.281264] pcieport 0000:00:00.0: Recovering Root Port due to Link
> Down
> > > [ 87.289245] pci 0000:01:00.0: AER: can't recover (no error_detected
> callback)
> > > root@imx95evk:~# [ 87.514216] imx6q-pcie 4c300000.pcie:
> PCIe(LNK_STS:0x00000700) link up detected
> > > [ 87.702968] imx6q-pcie 4c300000.pcie: PCIe Gen.3 x1 link up
> > > [ 87.834983] pcieport 0000:00:00.0: Root Port has been reset
> > > [ 87.840714] pcieport 0000:00:00.0: AER: device recovery failed
> > > [ 87.846592] imx6q-pcie 4c300000.pcie: Rescan bus after link up is
> detected
> > > [ 87.855947] pcieport 0000:00:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> >
> > I've seen this same line ("bridge configuration invalid") before, and
> > I believe that's because the saved state (pci_save_state(); more about
> > this below) is invalid -- it contains 0 values in places where they
> > should be non-zero. So when those values are restored
> > (pci_restore_state()), we get confused.
> >
> > I believe we've pinned down one reason this invalid state occurs --
> > it's because of an automatic (mis)feature in the DesignWare PCIe
> hardware.
> > Specifically, it's because of what the controller does during a
> > surprise link-down error.
> >
> > From the Designware docs:
> >
> > "[...] during normal operation, the link might fail and go down. After
> > this link-down event, the controller requests the DWC_pcie_clkrst.v
> > module to hot-reset the controller. There is no difference in the
> > handling of a link-down reset or a hot reset; the controller asserts
> > the link_req_rst_not output requesting the DWC_pcie_clkrst.v module
> to
> > reset the controller."
> >
> > In some of the adjacent documentation (and confirmed in local
> > testing), it suggests that this automatic reset will also reset
> > various DBI (i.e., PCIe config space) registers. It also seems as if
> > there's not really a good way to completely stop this automatic reset
> > -- the docs mention some SW methods prevent the reset, but they all
> seem racy or incomplete.
> >
> > Anyway, I think this implies that patch 1 is somewhat wrong [1]. It
> > includes some code like this:
> >
> > pci_save_state(dev);
> > ret = host->reset_root_port(host, dev);
> > if (ret)
> > pci_err(dev, "Failed to reset Root Port: %d\n", ret);
> > else
> > /* Now restore it on success */
> > pci_restore_state(dev);
> >
> > That first line (pci_save_state()) is prone to saving invalid state,
> > depending on whether the link-down event has finished flushing and
> > resetting the controller yet or not. The resulting impact is a bit
> > hard to judge, depending on what (mis)configuration you end up with.
> >
>
> Thanks a lot for your investigation. I think your observation makes sense and
> could be the culprit in saving the corrupted state. Even on non-DWC
> controllers, there is no guarantee that the Root Port config registers state will
> be preserved after LDn (before Root Port reset).
>
> > I also noticed commit a2f1e22390ac ("PCI/ERR: Ensure error
> > recoverability at all times") was merged recently. With that change, I
> > believe it is now safe to perform pci_restore_state() even without
> > pci_save_state() here.
> >
> > So ... can we remove pci_save_state() from
> > pcibios_reset_secondary_bus()? Might that help?
>
> I think so. I will also test it locally and report back soon.
>
> > It sounds like my above
> > observations *may* match Richard's reports, but I'm not sure. And
> > anyway, the documented hardware behavior is racy, so it's hard to
> > propose a foolproof solution.
> >
>
> @Richard: Can you confirm if removing 'pci_save_state(dev);' from
> pcibios_reset_secondary_bus() fixes your issue?
I have tested the hot reset trigger hundreds of times, and it works
consistently.
@Brian Norris @Manivannan Sadhasivam
Thanks a lot for your helps.
Best Regards
Richard Zhu
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v4 1/9] coresight: etm4x: introduce struct etm4_caps
From: Yeoreum Yun @ 2026-04-14 7:55 UTC (permalink / raw)
To: Leo Yan
Cc: coresight, linux-arm-kernel, linux-kernel, suzuki.poulose,
mike.leach, james.clark, alexander.shishkin, jie.gan
In-Reply-To: <20260413172104.GD356832@e132581.arm.com>
Hi Leo!
> On Mon, Apr 13, 2026 at 03:19:54PM +0100, Yeoreum Yun wrote:
> > Introduce struct etm4_caps to describe ETMv4 capabilities
> > and move capabilities information into it.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> LGTM:
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
Thanks.
>
> FWIW, two comments from Sashiko are valuable for me, please see below.
>
> > ---
> > .../coresight/coresight-etm4x-core.c | 234 +++++++++---------
> > .../coresight/coresight-etm4x-sysfs.c | 190 ++++++++------
> > drivers/hwtracing/coresight/coresight-etm4x.h | 176 ++++++-------
> > 3 files changed, 328 insertions(+), 272 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index d565a73f0042..6443f3717b37 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -88,8 +88,9 @@ static int etm4_probe_cpu(unsigned int cpu);
> > */
> > static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> > {
> > - return (n < drvdata->nr_ss_cmp) &&
> > - drvdata->nr_pe &&
> > + const struct etmv4_caps *caps = &drvdata->caps;
> > +
> > + return (n < caps->nr_ss_cmp) && caps->nr_pe &&
> > (drvdata->config.ss_status[n] & TRCSSCSRn_PC);
>
> As Sashiko suggests:
>
> "This isn't a regression introduced by this patch, but should this be
> checking caps->nr_pe_cmp instead of caps->nr_pe?"
>
> I confirmed the ETMv4 specification (ARM IHI0064H.b), the comment
> above is valid as the we should check caps->nr_pe_cmp instead.
>
> Could you first use a patch to fix the typo and then apply
> capabilities afterwards? This is helpful for porting to stable
> kernels.
Sashiko finds valid point. And I'm bit of surprised no body find it.
Okay. I'll send it next round.
>
> [...]
>
> > @@ -525,14 +530,14 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> > if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1))
> > dev_err(etm_dev,
> > "timeout while waiting for Idle Trace Status\n");
> > - if (drvdata->nr_pe)
> > + if (caps->nr_pe)
> > etm4x_relaxed_write32(csa, config->pe_sel, TRCPROCSELR);
> > etm4x_relaxed_write32(csa, config->cfg, TRCCONFIGR);
> > /* nothing specific implemented */
> > etm4x_relaxed_write32(csa, 0x0, TRCAUXCTLR);
> > etm4x_relaxed_write32(csa, config->eventctrl0, TRCEVENTCTL0R);
> > etm4x_relaxed_write32(csa, config->eventctrl1, TRCEVENTCTL1R);
> > - if (drvdata->stallctl)
> > + if (caps->stallctl)
> > etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> > etm4x_relaxed_write32(csa, config->ts_ctrl, TRCTSCTLR);
> > etm4x_relaxed_write32(csa, config->syncfreq, TRCSYNCPR);
> > @@ -542,17 +547,17 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> > etm4x_relaxed_write32(csa, config->vinst_ctrl, TRCVICTLR);
> > etm4x_relaxed_write32(csa, config->viiectlr, TRCVIIECTLR);
> > etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR);
> > - if (drvdata->nr_pe_cmp)
> > + if (caps->nr_pe_cmp)
> > etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR);
> > - for (i = 0; i < drvdata->nrseqstate - 1; i++)
> > + for (i = 0; i < caps->nrseqstate - 1; i++)
> > etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i));
>
> Sashiko's comment:
>
> "If the hardware does not implement a sequencer, caps->nrseqstate (a u8)
> will be 0. Does 0 - 1 evaluate to -1 as an int, which then gets promoted
> to ULONG_MAX against val (an unsigned long)?"
>
> This is a good catch. The condition check should be:
>
> for (i = 0; i < caps->nrseqstate; i++)
> ...;
>
> The issue is irrelevant to your patch, but could you use a patch to fix
> "nrseqstate - 1" first and then apply the cap refactoring on it? This
> would be friendly for porting to stable kernel.
Okay. I'll send it next round.
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH 0/3] arm-smmu-v3: Add PMCG child support and update PMU MMIO mapping
From: Peng Fan @ 2026-04-14 7:47 UTC (permalink / raw)
To: Robin Murphy
Cc: Will Deacon, Joerg Roedel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mark Rutland, linux-arm-kernel, iommu, devicetree,
linux-kernel, linux-perf-users, Peng Fan
In-Reply-To: <65629411-0e1c-4c9c-bc9f-6488097bd77f@arm.com>
Hi Robin,
On Fri, Apr 10, 2026 at 01:07:29PM +0100, Robin Murphy wrote:
>On 08/04/2026 2:47 pm, Peng Fan wrote:
>> On Wed, Apr 08, 2026 at 12:15:31PM +0100, Robin Murphy wrote:
>> > On 2026-04-08 8:51 am, Peng Fan (OSS) wrote:
>> > > This patch series adds proper support for describing and probing the
>> > > Arm SMMU v3 PMCG (Performance Monitor Control Group) as a child node of
>> > > the SMMU in Devicetree, and updates the relevant drivers accordingly.
>> > >
>> > > The SMMU v3 architecture allows an optional PMCG block, typically
>> > > associated with TCUs, to be implemented within the SMMU register
>> > > address space. For example, mmu700 PMCG is at the offset 0x2000 of the
>> > > TCU page 0.
>> >
>> > But what's wrong with the existing binding? Especially given that it even has
>> > an upstream user already:
>> >
>> > https://git.kernel.org/torvalds/c/aef9703dcbf8
>> >
>> > > Patch 1 updates the SMMU v3 Devicetree binding to allow PMCG child nodes,
>> > > referencing the existing arm,smmu-v3-pmcg binding.
>> > >
>> > > Patch 2 updates the arm-smmu-v3 driver to populate platform devices for
>> > > child nodes described in DT once the SMMU probe succeeds.
>> > >
>> > > Patch 3 updates the SMMUv3 PMU driver to correctly handle MMIO mapping when
>> > > PMCG is described as a child node. The PMCG registers occupy a sub-region
>> > > of the parent SMMU MMIO window, which is already requested by the SMMU
>> >
>> > That has not been the case since 52f3fab0067d ("iommu/arm-smmu-v3: Don't
>> > reserve implementation defined register space") nearly 6 years ago, where the
>> > whole purpose was to support Arm's PMCG implementation properly. What kernel
>> > is this based on?
>>
>> Seems I am wrong. I thought PMCG is in page 0, so there were resource
>> conflicts. I just retest without this patchset, all goes well.
>>
>> But from dt perspective, should the TCU PMCG node be child node of
>> SMMU node?
>
>No. PMCGs can be used entirely independently of the SMMU itself, and while
>most of the events do relate to SMMU translation and thus aren't necessarily
>meaningful if it's not in use, there are still some which can be useful for
>basic traffic counting, monitoring GPT/translation activity from _other_
>security states (if observation is delegated to Non-Secure) and possibly
>other things, even if the "main" Non-Secure SMMU interface isn't advertised
>at all. It would be unreasonable to require the SMMU node to be present and
>enabled *and* have a driver to populate PMCGs, to monitor events which are
>outside the scope of that driver.
Thanks for explaining this in detail.
Just have one more question, we are using mmu-700, but MMU-700 implementation
defined TCU and TBU events are not supported.
Should we introduce a compatible string saying "arm,mmu700-tcu-pmcg" or
"arm,mmu700-tbu-pmcg"? TBH, I have not checked MMU600(AE) or else.
Thanks,
Peng
>
>Thanks,
>Robin.
>
^ permalink raw reply
* Re: [PATCH] mm/arm: pgtable: remove young bit check for pte_valid_user
From: Brian Ruley @ 2026-04-14 7:44 UTC (permalink / raw)
To: Will Deacon
Cc: Russell King (Oracle), Steve Capper, linux-arm-kernel,
linux-kernel, catalin.marinas
In-Reply-To: <ad0Ky09tLcFx7JCa@zoo11.fihel.lab.ge-healthcare.net>
On Apr 13, Brian Ruley wrote:
>
> In the meanwhile, I'll see if I can add some instrumentation to
> verify this is the bug we're seeing.
The instrumentation worked (used the same ring buffer approach to track
dcache flushes). Here's the output:
```
kernel: [48629.557043] SIGILL at b6b80ac0 cpu 1 pid 32663 linux_pte=8eff659f hw_pte=8eff6e7e young=1 exec=1
kernel: [48629.557157] dcache flush START cpu0 pfn=8eff6 ts=48629557020154
kernel: [48629.557207] dcache flush FINISH cpu0 pfn=8eff6 ts=48629557036154
kernel: [48629.557230] dcache flush SKIPPED cpu1 pfn=8eff6 ts=48629557020154
audisp-syslog: type=ANOM_ABEND msg=audit(1776154637.460:15): auid=4294967295 uid=0 gid=0 ses=4294967295 pid=32663 comm="journalctl" exe="/usr/bin/journalctl" sig=4 res=1 AUID="unset" UID="root" GID="root"
```
BR,
Brian
^ permalink raw reply
* Re: [PATCH 3/5] ARM: configs: Drop redundant SND_ATMEL_SOC
From: Claudiu Beznea @ 2026-04-14 7:38 UTC (permalink / raw)
To: Krzysztof Kozlowski, Nicolas Ferre, Alexandre Belloni,
Bjorn Andersson, Dmitry Baryshkov, Dinh Nguyen,
Krzysztof Kozlowski
Cc: Arnd Bergmann, Linus Walleij, Drew Fustini, soc, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260412-b4-defconfig-multi-v7-v1-3-e76de035c2df@oss.qualcomm.com>
On 4/12/26 20:12, Krzysztof Kozlowski wrote:
> CONFIG_SND_ATMEL_SOC is gone since commit 4f30f84feb77 ("ASoC: atmel:
> Standardize ASoC menu") and can be simply dropped without effect.
>
> No impact on include/generated/autoconf.h.
>
> Signed-off-by: Krzysztof Kozlowski<krzysztof.kozlowski@oss.qualcomm.com>
Acked-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
^ permalink raw reply
* Re: [PATCH 10/10] drm: of: forbid bridge-only calls to drm_of_find_panel_or_bridge()
From: Luca Ceresoli @ 2026-04-14 7:02 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Xinliang Liu, Tian Tao,
Xinwei Kong, Sumit Semwal, Yongqin Liu, John Stultz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Tomi Valkeinen, Michal Simek,
Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel,
linux-arm-msm, freedreno, linux-arm-kernel
In-Reply-To: <nligqvm3lq6n556onglmb345arxztd4pc6fboo4yrs3bfu27eu@uiyu2xklnexb>
Hello Dmitry, Maxime,
On Mon Apr 13, 2026 at 8:04 PM CEST, Dmitry Baryshkov wrote:
> On Mon, Apr 13, 2026 at 03:58:42PM +0200, Luca Ceresoli wrote:
>> Up to now drm_of_find_panel_or_bridge() can be called with a bridge pointer
>> only, a panel pointer only, or both a bridge and a panel pointers. The
>> logic to handle all the three cases is somewhat complex to read however.
>>
>> Now all bridge-only callers have been converted to
>> of_drm_get_bridge_by_endpoint(), which is simpler and handles bridge
>> refcounting. So forbid new bridge-only users by mandating a non-NULL panel
>> pointer in the docs and in the sanity checks along with a warning.
>
> Are there remaining users which still use either the bridge or the
> panel? Would it be possible / better to drop the two-arg version?
Yes. I counted ~20 panel+bridge and 4 panel-only callers with this series
applied, and on top of those there are devm_drm_of_get_bridge() and
drmm_of_get_bridge() which to me are the real issue because they make it
impossible to correctly handle bridge lifetime.
We discussed this with both you and Maxime a while back. AFAIK Maxime has a
plan to make every panel automatically instantiate a panel_bridge. I think
that's the only reasonable approach to get rid of
drm_of_find_panel_or_bridge() + *_of_get_bridge() and make bridge lifetime
easier and safe.
@Maxime, do you have updates on that idea?
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] arm64/hwcap: Include kernel-hwcap.h in list of generated files
From: Anshuman Khandual @ 2026-04-14 6:55 UTC (permalink / raw)
To: Mark Brown, Catalin Marinas, Will Deacon
Cc: Marek Vasut, linux-arm-kernel, linux-kernel
In-Reply-To: <20260413-arm64-hwcap-gen-fix-v1-1-26c56aed6908@kernel.org>
On 13/04/26 9:14 PM, Mark Brown wrote:
> When adding generation for the kernel internal constants for hwcaps the
> generated file was not explicitly flagged as such in the build system,
> causing it to be regenerated on each build. This wasn't obvious when the
> series the change was included in was developed since it was all about
> changes that trigger rebuilds anyway.
>
> Fixes: abed23c3c44f5 (arm64/hwcap: Generate the KERNEL_HWCAP_ definitions for the hwcaps)
> Reported-by: Marek Vasut <marex@nabladev.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm64/include/asm/Kbuild | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index d2ff8f6c3231..31441790b808 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -17,4 +17,5 @@ generic-y += parport.h
> generic-y += user.h
>
> generated-y += cpucap-defs.h
> +generated-y += kernel-hwcap.h
> generated-y += sysreg-defs.h
>
> ---
> base-commit: abed23c3c44f565dc812563ac015be70dd61e97b
> change-id: 20260413-arm64-hwcap-gen-fix-ecb4bb6dbb91
>
> Best regards,
> --
> Mark Brown <broonie@kernel.org>
>
>
^ permalink raw reply
* [PATCH net v2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()
From: Lorenzo Bianconi @ 2026-04-14 6:50 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Lorenzo Bianconi
Cc: linux-arm-kernel, linux-mediatek, netdev
Similar to airoha_qdma_cleanup_rx_queue(), reset DMA TX descriptors in
airoha_qdma_cleanup_tx_queue routine. Moreover, reset TX_DMA_IDX to
TX_CPU_IDX to notify the NIC the QDMA TX ring is empty.
Fixes: 23020f0493270 ("net: airoha: Introduce ethernet support for EN7581 SoC")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Changes in v2:
- Move q->ndesc initialization at end of airoha_qdma_init_tx routine in
order to avoid any possible NULL pointer dereference in
airoha_qdma_cleanup_tx_queue()
- Check if q->tx_list is empty in airoha_qdma_cleanup_tx_queue()
- Link to v1: https://lore.kernel.org/r/20260410-airoha_qdma_cleanup_tx_queue-fix-net-v1-1-b7171c8f1e78@kernel.org
---
drivers/net/ethernet/airoha/airoha_eth.c | 41 ++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 9e995094c32a..3c1a2bc68c42 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -966,27 +966,27 @@ static int airoha_qdma_init_tx_queue(struct airoha_queue *q,
dma_addr_t dma_addr;
spin_lock_init(&q->lock);
- q->ndesc = size;
q->qdma = qdma;
q->free_thr = 1 + MAX_SKB_FRAGS;
INIT_LIST_HEAD(&q->tx_list);
- q->entry = devm_kzalloc(eth->dev, q->ndesc * sizeof(*q->entry),
+ q->entry = devm_kzalloc(eth->dev, size * sizeof(*q->entry),
GFP_KERNEL);
if (!q->entry)
return -ENOMEM;
- q->desc = dmam_alloc_coherent(eth->dev, q->ndesc * sizeof(*q->desc),
+ q->desc = dmam_alloc_coherent(eth->dev, size * sizeof(*q->desc),
&dma_addr, GFP_KERNEL);
if (!q->desc)
return -ENOMEM;
- for (i = 0; i < q->ndesc; i++) {
+ for (i = 0; i < size; i++) {
u32 val = FIELD_PREP(QDMA_DESC_DONE_MASK, 1);
list_add_tail(&q->entry[i].list, &q->tx_list);
WRITE_ONCE(q->desc[i].ctrl, cpu_to_le32(val));
}
+ q->ndesc = size;
/* xmit ring drop default setting */
airoha_qdma_set(qdma, REG_TX_RING_BLOCKING(qid),
@@ -1051,13 +1051,17 @@ static int airoha_qdma_init_tx(struct airoha_qdma *qdma)
static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
{
- struct airoha_eth *eth = q->qdma->eth;
- int i;
+ struct airoha_qdma *qdma = q->qdma;
+ struct airoha_eth *eth = qdma->eth;
+ int i, qid = q - &qdma->q_tx[0];
+ struct airoha_queue_entry *e;
+ u16 index = 0;
spin_lock_bh(&q->lock);
for (i = 0; i < q->ndesc; i++) {
- struct airoha_queue_entry *e = &q->entry[i];
+ struct airoha_qdma_desc *desc = &q->desc[i];
+ e = &q->entry[i];
if (!e->dma_addr)
continue;
@@ -1067,8 +1071,31 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
e->dma_addr = 0;
e->skb = NULL;
list_add_tail(&e->list, &q->tx_list);
+
+ /* Reset DMA descriptor */
+ WRITE_ONCE(desc->ctrl, 0);
+ WRITE_ONCE(desc->addr, 0);
+ WRITE_ONCE(desc->data, 0);
+ WRITE_ONCE(desc->msg0, 0);
+ WRITE_ONCE(desc->msg1, 0);
+ WRITE_ONCE(desc->msg2, 0);
+
q->queued--;
}
+
+ if (!list_empty(&q->tx_list)) {
+ e = list_first_entry(&q->tx_list, struct airoha_queue_entry,
+ list);
+ index = e - q->entry;
+ }
+ /* Set TX_DMA_IDX to TX_CPU_IDX to notify the hw the QDMA TX ring is
+ * empty.
+ */
+ airoha_qdma_rmw(qdma, REG_TX_CPU_IDX(qid), TX_RING_CPU_IDX_MASK,
+ FIELD_PREP(TX_RING_CPU_IDX_MASK, index));
+ airoha_qdma_rmw(qdma, REG_TX_DMA_IDX(qid), TX_RING_DMA_IDX_MASK,
+ FIELD_PREP(TX_RING_DMA_IDX_MASK, index));
+
spin_unlock_bh(&q->lock);
}
---
base-commit: 2cd7e6971fc2787408ceef17906ea152791448cf
change-id: 20260410-airoha_qdma_cleanup_tx_queue-fix-net-93375f5ee80f
Best regards,
--
Lorenzo Bianconi <lorenzo@kernel.org>
^ permalink raw reply related
* Re: [PATCH net] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()
From: Lorenzo Bianconi @ 2026-04-14 6:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260413225709.3648413-1-kuba@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3295 bytes --]
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()
>
> This commit resets DMA TX descriptors in airoha_qdma_cleanup_tx_queue
> and synchronizes TX_DMA_IDX to TX_CPU_IDX to notify the hardware that
> the ring is empty.
>
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 9285a68f435fe..963ab7b8d166b 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -1044,13 +1044,17 @@ static int airoha_qdma_init_tx(struct airoha_qdma *qdma)
> [ ... ]
> > spin_lock_bh(&q->lock);
> > for (i = 0; i < q->ndesc; i++) {
> > - struct airoha_queue_entry *e = &q->entry[i];
> > + struct airoha_qdma_desc *desc = &q->desc[i];
> >
> > + e = &q->entry[i];
>
> Can this result in a NULL pointer dereference?
>
> If airoha_qdma_init_tx_queue() sets q->ndesc before allocating q->entry and
> q->desc, and devm_kzalloc() fails, the error path jumps to
> airoha_hw_cleanup().
>
> Because q->ndesc > 0, this cleanup loop would execute and dereference
> q->entry[i] and q->desc[i], which may not have been allocated.
ack, I will fix it in v2.
>
> [ ... ]
> > + /* Reset DMA descriptor */
> > + WRITE_ONCE(desc->ctrl, 0);
> > + WRITE_ONCE(desc->addr, 0);
> > + WRITE_ONCE(desc->data, 0);
> > + WRITE_ONCE(desc->msg0, 0);
> > + WRITE_ONCE(desc->msg1, 0);
> > + WRITE_ONCE(desc->msg2, 0);
>
> Is it safe to modify these DMA descriptors and reset the ring indices below
> without waiting for the hardware DMA engine to become idle?
>
> In airoha_dev_stop(), the driver clears GLOBAL_CFG_TX_DMA_EN_MASK to request
> the hardware DMA engine to stop, but it does not appear to poll
> GLOBAL_CFG_TX_DMA_BUSY_MASK to ensure it has actually stopped.
>
> Modifying the DMA descriptors and internal ring indices while the hardware
> is still actively processing in-flight descriptors could potentially lead
> to corrupted DMA fetches or IOMMU faults.
This is already fixed here:
https://lore.kernel.org/netdev/20260411-airoha-fix-ndo_stop-v1-1-caddaa181739@kernel.org/
>
> > q->queued--;
> > }
> > +
> > + e = list_first_entry(&q->tx_list, struct airoha_queue_entry,
> > + list);
> > + index = e - q->entry;
> > + /* Set TX_DMA_IDX to TX_CPU_IDX to notify the hw the QDMA TX ring is
> > + * empty.
> > + */
> > + airoha_qdma_rmw(qdma, REG_TX_CPU_IDX(qid), TX_RING_CPU_IDX_MASK,
> > + FIELD_PREP(TX_RING_CPU_IDX_MASK, index));
> > + airoha_qdma_rmw(qdma, REG_TX_DMA_IDX(qid), TX_RING_DMA_IDX_MASK,
> > + FIELD_PREP(TX_RING_DMA_IDX_MASK, index));
>
> Could list_first_entry() be called on an empty list here?
>
> If q->tx_list is empty (for example, if allocation failed in
> airoha_qdma_init_tx_queue or if no entries had a valid dma_addr), calling
> list_first_entry() returns an invalid pointer.
>
> The subsequent pointer arithmetic would produce a garbage index value,
> which is then written to the REG_TX_CPU_IDX and REG_TX_DMA_IDX registers.
ack, I will fix it in v2.
Regards,
Lorenzo
> --
> pw-bot: cr
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] [v2] ARM: dts: bcm4709: fix bus range assignment
From: Arnd Bergmann @ 2026-04-14 6:47 UTC (permalink / raw)
To: Florian Fainelli, Hauke Mehrtens, Rafał Miłecki,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Rosen Penev
Cc: soc, Arnd Bergmann, Broadcom internal kernel review list,
linux-arm-kernel, devicetree, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The netgear r8000 dts file limits the bus range for the first host
bridge to exclude bus 0, but the two devices on the first bus are
explicitly assigned to bus 0, causing a build time warning:
/home/arnd/arm-soc/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts:142.3-27: Warning (pci_device_bus_num): /axi@18000000/pcie@13000/pcie@0/pcie@0,0/pcie@1,0:bus-range: PCI bus number 0 out of range, expected (1 - 255)
/home/arnd/arm-soc/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts:142.3-27: Warning (pci_device_bus_num): /axi@18000000/pcie@13000/pcie@0/pcie@0,0/pcie@2,0:bus-range: PCI bus number 0 out of range, expected (1 - 255)
As Rosen mentioned, the bus-range property was a mistake, so just
remove it and keep the reg values pointing to bus 0, which is
allowed by the default bus range of the SoC.
Suggested-by: Rosen Penev <rosenp@gmail.com>
Fixes: 893faf67438c ("ARM: dts: BCM5301X: add root pcie bridges")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
index d170c71cbd76..e85693fba16a 100644
--- a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
@@ -139,7 +139,6 @@ &pcie_bridge1 {
pcie@0,0 {
device_type = "pci";
reg = <0x0000 0 0 0 0>;
- bus-range = <0x01 0xff>;
#address-cells = <3>;
#size-cells = <2>;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 01/10] drm/bridge: add of_drm_get_bridge_by_endpoint()
From: Luca Ceresoli @ 2026-04-14 6:44 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Xinliang Liu, Tian Tao,
Xinwei Kong, Sumit Semwal, Yongqin Liu, John Stultz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Tomi Valkeinen, Michal Simek,
Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel,
linux-arm-msm, freedreno, linux-arm-kernel
In-Reply-To: <omlnswxukeqgnatzdvooaashgkfcacjevkvbkm6xt33itgua2k@jcmzll2w6kdq>
Hi Dmitry,
On Mon Apr 13, 2026 at 7:56 PM CEST, Dmitry Baryshkov wrote:
> On Mon, Apr 13, 2026 at 07:07:14PM +0200, Luca Ceresoli wrote:
>> Hi Dmitry, Maxime,
>>
>> thanks Dmitry for the quick feedback!
>>
>> On Mon Apr 13, 2026 at 4:58 PM CEST, Dmitry Baryshkov wrote:
>>
>> >> --- a/drivers/gpu/drm/drm_bridge.c
>> >> +++ b/drivers/gpu/drm/drm_bridge.c
>> >> @@ -1581,6 +1581,52 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
>> >> return bridge;
>> >> }
>> >> EXPORT_SYMBOL(of_drm_find_bridge);
>> >> +
>> >> +/**
>> >> + * of_drm_get_bridge_by_endpoint - return DRM bridge connected to a port/endpoint
>> >> + * @np: device tree node containing output ports
>> >> + * @port: port in the device tree node, or -1 for the first port found
>> >> + * @endpoint: endpoint in the device tree node, or -1 for the first endpoint found
>> >> + * @bridge: pointer to hold returned drm_bridge, must not be NULL
>> >> + *
>> >> + * Given a DT node's port and endpoint number, find the connected node and
>> >> + * return the associated drm_bridge device.
>> >> + *
>> >> + * The refcount of the returned bridge is incremented. Use drm_bridge_put()
>> >> + * when done with it.
>> >> + *
>> >> + * Returns zero (and sets *bridge to a valid bridge pointer) if successful,
>> >> + * or one of the standard error codes (and the value in *bridge is
>> >> + * unspecified) if it fails.
>> >
>> > Can we return drm_bridge or error cookie instead?
>>
>> (while replying I realized there is a design flaw in my implementation, but
>> see below)
>>
>> I initially thought I'd do it, but I don't like returning an error cookie
>> for functions getting a bridge pointer. The main reason is that with bridge
>> refcounting the __free() cleanup actions are handy in a lot of places, so we
>> are introdugin a lot of code like:
>>
>> struct drm_bridge *foo __free(drm_bridge_put) = some_func(...);
>>
>> Where some_func can be one of of_drm_find_bridge(),
>> drm_bridge_get_next_bridge(), drm_bridge_chain_get_{first,last}_bridge()
>> etc.
>
> This is fine even with the functions returning a cookie: the free
> function can explicitly check and return early if IS_ERR() pointer is
> passed to it.
>
>>
>> Such code is very handy exactly because these functions return either a
>> valid pointer or NULL, and thus the cleanup actions always does the right
>> thing. If an error cookie were returned, the caller would have to be very
>> careful in inhibiting the cleanup action by clearing the pointer before
>> returning. This originate for example this discussion: [0]
>>
>> [0] https://lore.kernel.org/lkml/4cd29943-a8d0-4706-b0c5-01d6b33863e4@bootlin.com/
>>
>> So I think never having a negative error value in the bridge pointer is
>> useful to prevent bugs slipping in drivers. For this we should take one of
>> these two opposite approaches:
>>
>> 1. don't return a bridge pointer which can be an ERR_PTR; return an int
>> with the error code and take a **drm_bridge and:
>> - on success, set the valid pointer in *bridge
>> - on failure, set *bridge = NULL (*)
>> 2. like the above-mentioned functions (of_drm_find_bridge(),
>> drm_bridge_get_next_bridge() etc) return a drm_bridge pointer which is
>> either a valid pointer or NULL
>
> 3. Return pointer or cookie, ignore cookie in the release function.
Ah, that's a good idea indeed!
It had been proposed recently by Laurent too, but in that case I didn't
take the suggestion because it was referring to a panel API which IIUC is
meant to be reworked anyway [0]. I must have archived the idea too much and
didn't think about using it now! :)
So yes, being of_drm_get_bridge_by_endpoint() a new API that is meant to
stay, I think it's worth doing.
Sadly, I guess that means I have to drop all the R-by you already gave to
various patches in the series.
[0] https://lore.kernel.org/all/DH624WYWKT14.5TSCJXZPVN0T@bootlin.com/
>> >> + */
>> >> +int of_drm_get_bridge_by_endpoint(const struct device_node *np,
>> >> + int port, int endpoint,
>> >> + struct drm_bridge **bridge)
>> >
>> > Nit: can it be drm_of_get_bridge_by_endpoint?
>>
>> Argh, this convention is changing periodically it seems! :-)
>>
>> I previous discussions I was asked to do either drm_of_ [1] of of_drm_ [2],
>> but since the latter was the last one requested I sticked on it.
>>
>> @Maxime, Dmitry, I'm OK with either, just let me know if I need to change.
>
> I missed Maxime's response, sorry. I'm fine with the suggested
> convention of using the first argument.
OK, no problem. Based on current discussion, in v2 the new API will be:
/**
* of_drm_get_bridge_by_endpoint - return DRM bridge connected to a port/endpoint
* @np: device tree node containing output ports
* @port: port in the device tree node, or -1 for the first port found
* @endpoint: endpoint in the device tree node, or -1 for the first endpoint found
*
* Given a DT node's port and endpoint number, find the connected node and
* return the associated drm_bridge device.
*
* The refcount of the returned bridge is incremented. Use drm_bridge_put()
* when done with it.
*
* Returns a pointer to the connected drm_bridge, or a negative error on failure
*/
struct drm_bridge *int of_drm_get_bridge_by_endpoint(const struct device_node *np,
int port, int endpoint);
It would be nice to agree on the API before v2, to avoid the need to rework
many patches and drop any review tags multiple times.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] drm: mxsfb: lcdif: enforce 64-byte pitch alignment for scanout
From: Advait Dhamorikar @ 2026-04-14 6:35 UTC (permalink / raw)
To: Philipp Zabel
Cc: marex, simona, imx, festevam, kernel, dri-devel, Frank.Li,
s.hauer, maarten.lankhorst, linux-kernel, mripard, stefan,
tzimmermann, airlied, linux-arm-kernel
In-Reply-To: <99fdddde59e1b0fe13d5014182b1c97de4fbec86.camel@pengutronix.de>
Hello Philipp,
> I wonder if setting 4320 bytes stride works if you reduce the burst
> size, for example by reverting commit 2215cb3be5c2 ("drm: lcdif: change
> burst size to 256B") to test.
Unfortunately, reverting the commit doesn't seem to fix the issue.
I tested with the reduced/default burst size as well as explicitly
setting it to 128 bytes, but a pitch of 4320 bytes still results in a
corrupted output(stretched and choppy display).
However, using a 64 byte aligned pitch works in all cases without having
to alter the burst size.
Best regards,
Advait
On Mon, Apr 13, 2026 at 7:16 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> Hi,
>
> On Mo, 2026-04-13 at 17:14 +0530, Advait Dhamorikar wrote:
> > The LCDIF controller expects framebuffer pitch to be aligned to a
> > 64 byte boundary for reliable scanout.
> >
> > While byte-granular pitches are
> > supported by the interface, the i.MX8MP reference manual
> > recommends 64-byte alignment for optimal operation.
> >
> > Corrupted output was observed with XR24 framebuffers where a pitch of
> > 4320 bytes caused visible corruption and choppy display, while an aligned
> > pitch of 4352 bytes worked correctly.
>
> This happens to be divisible by 256, which is is the burst size
> currently set in the undocumented CTRLDESCL0_3 register fields,
> according to the comment in lcdif_set_mode().
>
> I wonder if setting 4320 bytes stride works if you reduce the burst
> size, for example by reverting commit 2215cb3be5c2 ("drm: lcdif: change
> burst size to 256B") to test.
>
> If that is the case, it might be better to allow unaligned pitches but
> configure the burst size depending on pitch alignment.
>
> regards
> Philipp
^ 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