* [PATCH v2 0/3] Describe IMEM on Eliza
From: Alexander Koskovich @ 2026-04-16 9:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm, Alexander Koskovich
Add a compatible and describe the IMEM for the Eliza SoC.
Also sort nodes by unit address.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
Changes in v2:
- Fix sorting of nodes in eliza.dtsi
- Link to v1: https://lore.kernel.org/r/20260415-eliza-imem-v1-0-4a90e8683799@pm.me
---
Alexander Koskovich (3):
arm64: dts: qcom: eliza: Sort nodes by unit address
dt-bindings: sram: Document qcom,eliza-imem
arm64: dts: qcom: eliza: Add IMEM node
Documentation/devicetree/bindings/sram/sram.yaml | 1 +
arch/arm64/boot/dts/qcom/eliza.dtsi | 94 ++++++++++++++----------
2 files changed, 58 insertions(+), 37 deletions(-)
---
base-commit: 936c21068d7ade00325e40d82bfd2f3f29d9f659
change-id: 20260415-eliza-imem-e791f44abf1b
Best regards,
--
Alexander Koskovich <akoskovich@pm.me>
^ permalink raw reply
* [PATCH v2 1/3] arm64: dts: qcom: eliza: Sort nodes by unit address
From: Alexander Koskovich @ 2026-04-16 9:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm, Alexander Koskovich
In-Reply-To: <20260416-eliza-imem-v2-0-fb7a71123451@pm.me>
Qualcomm DTS uses sorting of MMIO nodes by the unit address, so move
few nodes in Eliza DTSI to fix that.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
arch/arm64/boot/dts/qcom/eliza.dtsi | 74 ++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
index 4a7a0ac40ce6..6fa5679c1a62 100644
--- a/arch/arm64/boot/dts/qcom/eliza.dtsi
+++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
@@ -662,16 +662,16 @@ &clk_virt SLAVE_QUP_CORE_2 QCOM_ICC_TAG_ALWAYS>,
};
};
- config_noc: interconnect@1600000 {
- compatible = "qcom,eliza-cnoc-cfg";
- reg = <0x0 0x01600000 0x0 0x5200>;
+ cnoc_main: interconnect@1500000 {
+ compatible = "qcom,eliza-cnoc-main";
+ reg = <0x0 0x01500000 0x0 0x16080>;
qcom,bcm-voters = <&apps_bcm_voter>;
#interconnect-cells = <2>;
};
- cnoc_main: interconnect@1500000 {
- compatible = "qcom,eliza-cnoc-main";
- reg = <0x0 0x01500000 0x0 0x16080>;
+ config_noc: interconnect@1600000 {
+ compatible = "qcom,eliza-cnoc-cfg";
+ reg = <0x0 0x01600000 0x0 0x5200>;
qcom,bcm-voters = <&apps_bcm_voter>;
#interconnect-cells = <2>;
};
@@ -862,13 +862,6 @@ tcsr: clock-controller@1fbf000 {
#reset-cells = <1>;
};
- lpass_ag_noc: interconnect@7e40000 {
- compatible = "qcom,eliza-lpass-ag-noc";
- reg = <0x0 0x07e40000 0x0 0xe080>;
- qcom,bcm-voters = <&apps_bcm_voter>;
- #interconnect-cells = <2>;
- };
-
lpass_lpiaon_noc: interconnect@7400000 {
compatible = "qcom,eliza-lpass-lpiaon-noc";
reg = <0x0 0x07400000 0x0 0x19080>;
@@ -883,6 +876,13 @@ lpass_lpicx_noc: interconnect@7420000 {
#interconnect-cells = <2>;
};
+ lpass_ag_noc: interconnect@7e40000 {
+ compatible = "qcom,eliza-lpass-ag-noc";
+ reg = <0x0 0x07e40000 0x0 0xe080>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ #interconnect-cells = <2>;
+ };
+
pdc: interrupt-controller@b220000 {
compatible = "qcom,eliza-pdc", "qcom,pdc";
reg = <0x0 0x0b220000 0x0 0x40000>,
@@ -1005,6 +1005,30 @@ spmi_bus1: spmi@c432000 {
};
};
+ tlmm: pinctrl@f100000 {
+ compatible = "qcom,eliza-tlmm";
+ reg = <0x0 0x0f100000 0x0 0xf00000>;
+
+ interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ gpio-ranges = <&tlmm 0 0 184>;
+ wakeup-parent = <&pdc>;
+
+ qup_uart14_default: qup-uart14-default-state {
+ /* TX, RX */
+ pins = "gpio18", "gpio19";
+ function = "qup2_se5";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
apps_smmu: iommu@15000000 {
compatible = "qcom,eliza-smmu-500", "qcom,smmu-500", "arm,mmu-500";
reg = <0x0 0x15000000 0x0 0x100000>;
@@ -1319,30 +1343,6 @@ cpufreq_hw: cpufreq@17d91000 {
#clock-cells = <1>;
};
- tlmm: pinctrl@f100000 {
- compatible = "qcom,eliza-tlmm";
- reg = <0x0 0x0f100000 0x0 0xf00000>;
-
- interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
-
- gpio-controller;
- #gpio-cells = <2>;
-
- interrupt-controller;
- #interrupt-cells = <2>;
-
- gpio-ranges = <&tlmm 0 0 184>;
- wakeup-parent = <&pdc>;
-
- qup_uart14_default: qup-uart14-default-state {
- /* TX, RX */
- pins = "gpio18", "gpio19";
- function = "qup2_se5";
- drive-strength = <2>;
- bias-pull-up;
- };
- };
-
gem_noc: interconnect@24100000 {
compatible = "qcom,eliza-gem-noc";
reg = <0x0 0x24100000 0x0 0x163080>;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: sram: Document qcom,eliza-imem
From: Alexander Koskovich @ 2026-04-16 9:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm, Alexander Koskovich
In-Reply-To: <20260416-eliza-imem-v2-0-fb7a71123451@pm.me>
Add compatible for Eliza SoC IMEM.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
Documentation/devicetree/bindings/sram/sram.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
index 8985f89170be..27e5e274c3cb 100644
--- a/Documentation/devicetree/bindings/sram/sram.yaml
+++ b/Documentation/devicetree/bindings/sram/sram.yaml
@@ -34,6 +34,7 @@ properties:
- nvidia,tegra186-sysram
- nvidia,tegra194-sysram
- nvidia,tegra234-sysram
+ - qcom,eliza-imem
- qcom,hawi-imem
- qcom,kaanapali-imem
- qcom,milos-imem
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/3] arm64: dts: qcom: eliza: Add IMEM node
From: Alexander Koskovich @ 2026-04-16 9:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm, Alexander Koskovich
In-Reply-To: <20260416-eliza-imem-v2-0-fb7a71123451@pm.me>
Add a node for the IMEM found on Eliza, which contains pil-reloc-info
and the modem tables for IPA, among others.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
arch/arm64/boot/dts/qcom/eliza.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
index 6fa5679c1a62..551df07e44c6 100644
--- a/arch/arm64/boot/dts/qcom/eliza.dtsi
+++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
@@ -1029,6 +1029,26 @@ qup_uart14_default: qup-uart14-default-state {
};
};
+ sram@14680000 {
+ compatible = "qcom,eliza-imem", "mmio-sram";
+ reg = <0x0 0x14680000 0x0 0x2c000>;
+ ranges = <0x0 0x0 0x14680000 0x2c000>;
+
+ no-memory-wc;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ pilreloc-sram@94c {
+ compatible = "qcom,pil-reloc-info";
+ reg = <0x94c 0xc8>;
+ };
+
+ ipa_modem_tables: modem-tables-sram@3000 {
+ reg = <0x3000 0x2000>;
+ };
+ };
+
apps_smmu: iommu@15000000 {
compatible = "qcom,eliza-smmu-500", "qcom,smmu-500", "arm,mmu-500";
reg = <0x0 0x15000000 0x0 0x100000>;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH RFT] arm64: dts: qcom: sm8650: Fix IPA IMEM slice
From: Konrad Dybcio @ 2026-04-16 9:41 UTC (permalink / raw)
To: Alexander Koskovich, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260415-fix-8650-ipa-modem-tables-v1-1-95f8f425e416@pm.me>
On 4/16/26 1:45 AM, Alexander Koskovich wrote:
> Downstream the IPA IMEM slice for SM8650 is described as:
> qcom,additional-mapping = <0x14683000 0x14683000 0x2000>;
>
> Update upstream ipa_modem_tables to reflect downstream.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
Thanks for catching this!
Fixes: 581fc5d5ade6 ("arm64: dts: qcom: sm8650: Explicitly describe the IPA IMEM slice")
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* RE: [PATCH] arm64: dts: renesas: rzt2h-n2h-evk: Configure eMMC/SDHI pins
From: Fabrizio Castro @ 2026-04-16 9:48 UTC (permalink / raw)
To: Fabrizio Castro, Geert Uytterhoeven, magnus.damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Biju Das, Prabhakar Mahadev Lad
In-Reply-To: <20260331145221.7974-1-fabrizio.castro.jz@renesas.com>
Hi Geert,
Are you happy with this patch?
Without this patch fast SD cards/eMMCs may fail on the RZ/T2H and RZ/N2H.
If it helps, we may also add a couple of fixes tags to the patch, to fast
track it:
Fixes: 4d7624fc85a2 ("arm64: dts: renesas: rzt2h-rzn2h-evk: Enable eMMC")
Fixes: dba8ee27c5de ("arm64: dts: renesas: rzt2h-rzn2h-evk: Enable MicroSD card slot")
I haven't added them to the patch because I wanted to avoid being selected
for stable, as it depends on the below series:
https://lore.kernel.org/linux-renesas-soc/20260319141515.2053556-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
Cheers,
Fab
> From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Sent: 31 March 2026 15:52
> To: Geert Uytterhoeven <geert+renesas@glider.be>; magnus.damm <magnus.damm@gmail.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>
> Cc: Fabrizio Castro <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Biju Das <biju.das.jz@bp.renesas.com>;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: [PATCH] arm64: dts: renesas: rzt2h-n2h-evk: Configure eMMC/SDHI pins
>
> The HW user manual for the Renesas RZ/T2H and the RZ/N2H state
> that for SDR104, SDR50, and HS200 to work properly the eMMC/SDHI
> interface pins have to be configured as specified below:
> * SDn_CLK pin - drive strength: Ultra High, slew rate: fast
> * Other SDn_* pins: drive strength: High, slew rate: fast,
> Schmitt trigger: disabled (not applicable to SDn_RST pins).
>
> Adjust the pin definitions accordingly.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> .../dts/renesas/rzt2h-n2h-evk-common.dtsi | 54 ++++++++++++++++---
> 1 file changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-common.dtsi b/arch/arm64/boot/dts/renesas/rzt2h-
> n2h-evk-common.dtsi
> index f87c2492f414..3fae950db603 100644
> --- a/arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-common.dtsi
> @@ -275,12 +275,28 @@ data-pins {
> <RZT2H_PORT_PINMUX(12, 7, 0x29)>, /* SD0_DATA5 */
> <RZT2H_PORT_PINMUX(13, 0, 0x29)>, /* SD0_DATA6 */
> <RZT2H_PORT_PINMUX(13, 1, 0x29)>; /* SD0_DATA7 */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> };
>
> - ctrl-pins {
> - pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>, /* SD0_CLK */
> - <RZT2H_PORT_PINMUX(12, 1, 0x29)>, /* SD0_CMD */
> - <RZT2H_PORT_PINMUX(13, 2, 0x29)>; /* SD0_RST# */
> + clk-pins {
> + pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>; /* SD0_CLK */
> + drive-strength-microamp = <11800>;
> + slew-rate = <1>;
> + };
> +
> + cmd-pins {
> + pinmux = <RZT2H_PORT_PINMUX(12, 1, 0x29)>; /* SD0_CMD */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> + };
> +
> + rst-pins {
> + pinmux = <RZT2H_PORT_PINMUX(13, 2, 0x29)>; /* SD0_RST# */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> };
> };
>
> @@ -299,12 +315,23 @@ data-pins {
> <RZT2H_PORT_PINMUX(12, 3, 0x29)>, /* SD0_DATA1 */
> <RZT2H_PORT_PINMUX(12, 4, 0x29)>, /* SD0_DATA2 */
> <RZT2H_PORT_PINMUX(12, 5, 0x29)>; /* SD0_DATA3 */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> + };
> +
> + clk-pins {
> + pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>; /* SD0_CLK */
> + drive-strength-microamp = <11800>;
> + slew-rate = <1>;
> };
>
> ctrl-pins {
> - pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>, /* SD0_CLK */
> - <RZT2H_PORT_PINMUX(12, 1, 0x29)>, /* SD0_CMD */
> + pinmux = <RZT2H_PORT_PINMUX(12, 1, 0x29)>, /* SD0_CMD */
> <RZT2H_PORT_PINMUX(22, 5, 0x29)>; /* SD0_CD */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> };
> };
>
> @@ -323,12 +350,23 @@ data-pins {
> <RZT2H_PORT_PINMUX(17, 0, 0x29)>, /* SD1_DATA1 */
> <RZT2H_PORT_PINMUX(17, 1, 0x29)>, /* SD1_DATA2 */
> <RZT2H_PORT_PINMUX(17, 2, 0x29)>; /* SD1_DATA3 */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> + };
> +
> + clk-pins {
> + pinmux = <RZT2H_PORT_PINMUX(16, 5, 0x29)>; /* SD1_CLK */
> + drive-strength-microamp = <11800>;
> + slew-rate = <1>;
> };
>
> ctrl-pins {
> - pinmux = <RZT2H_PORT_PINMUX(16, 5, 0x29)>, /* SD1_CLK */
> - <RZT2H_PORT_PINMUX(16, 6, 0x29)>, /* SD1_CMD */
> + pinmux = <RZT2H_PORT_PINMUX(16, 6, 0x29)>, /* SD1_CMD */
> <RZT2H_PORT_PINMUX(17, 4, 0x29)>; /* SD1_CD */
> + drive-strength-microamp = <9000>;
> + slew-rate = <1>;
> + input-schmitt-disable;
> };
> };
> };
> --
> 2.34.1
^ permalink raw reply
* Re: [PATCH 4/5] arm64: dts: qcom: add IPQ9650 SoC and rdp488 board support
From: Kathiravan Thirumoorthy @ 2026-04-16 9:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <20260416-khaki-goose-of-will-ec76bf@quoll>
On 4/16/2026 3:05 PM, Krzysztof Kozlowski wrote:
> On Wed, Apr 15, 2026 at 07:03:19PM +0530, Kathiravan Thirumoorthy wrote:
>> + xo_board: xo-board-clk {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + };
>> + };
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + CPU0: cpu@0 {
> Labels should be lowercase.
Ack.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* [PATCH v2 0/2] rtc: bq32000: Add settle delay for aggressive polling
From: Adriana Stancu @ 2026-04-16 9:57 UTC (permalink / raw)
To: alexandre.belloni
Cc: linux-rtc, devicetree, linux-kernel, robh, krzk+dt, conor+dt,
Adriana Stancu
In-Reply-To: <20260416092414.3210383-1-adriana@arista.com>
This series addresses a limitation in the TI BQ32000 RTC where aggressive
I2C polling (done by userspace tools like hwclock on systems where the
interrupt line is not connected to the CPU) can prevent the refresh of
RTC registers.
This results in stale data reads or select() timeouts in userspace. The
series introduces a configurable "settle delay" via device tree to ensure
that the hardware has sufficient idle time between read attempts.
Patch 1: Adds the "ti,read-settle-us" property to the YAML bindings.
Patch 2: Implements the delay in the driver using usleep_range.
Changes in v2:
- Expanded dt-binding property description to explain use case.
- Updated commit messages on dt change to describe the scenario when the
dt property would be necessary.
- Reword the commit messages to respect wrapping at 75 columns.
Adriana Stancu (2):
dt-bindings: rtc: ti,bq32k: Add delay on rtc reads
rtc: bq32000: add configurable delay between RTC reads
.../devicetree/bindings/rtc/ti,bq32000.yaml | 9 +++++
drivers/rtc/rtc-bq32k.c | 34 +++++++++++++++----
2 files changed, 37 insertions(+), 6 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: rtc: ti,bq32k: Add delay on rtc reads
From: Adriana Stancu @ 2026-04-16 9:57 UTC (permalink / raw)
To: alexandre.belloni
Cc: linux-rtc, devicetree, linux-kernel, robh, krzk+dt, conor+dt,
Adriana Stancu
In-Reply-To: <20260416095706.3212158-1-adriana@arista.com>
Add a configurable "ti,read-settle-us" property to resolve a limitation
where aggressive I2C polling prevents the BQ32000's internal register to
update. This ensures the hardware has sufficient idle time to update its
buffer, preventing stale data reads on systems where the "interrupts" are
not configured.
Signed-off-by: Adriana Stancu <adriana@arista.com>
---
Documentation/devicetree/bindings/rtc/ti,bq32000.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/rtc/ti,bq32000.yaml b/Documentation/devicetree/bindings/rtc/ti,bq32000.yaml
index bf9c1c4ddb7e..46403f0c85a5 100644
--- a/Documentation/devicetree/bindings/rtc/ti,bq32000.yaml
+++ b/Documentation/devicetree/bindings/rtc/ti,bq32000.yaml
@@ -29,6 +29,15 @@ properties:
trickle-diode-disable: true
+ ti,read-settle-us:
+ default: 0
+ description:
+ Delay in microseconds to wait before reading RTC registers.
+ Aggressive I2C polling on systems without an interrupt line
+ can prevent the BQ32000's internal refresh cycle, leading to
+ stale data. This delay ensures the hardware has sufficient
+ idle time to update its registers.
+
required:
- compatible
- reg
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/2] rtc: bq32000: add configurable delay between RTC reads
From: Adriana Stancu @ 2026-04-16 9:57 UTC (permalink / raw)
To: alexandre.belloni
Cc: linux-rtc, devicetree, linux-kernel, robh, krzk+dt, conor+dt,
Adriana Stancu
In-Reply-To: <20260416095706.3212158-1-adriana@arista.com>
When the RTC is used on systems without a interrupt line, userspace tools
like "hwclock" fall back to a frequent polling loop to synchronize with
the edge of the next second.
On the BQ32000, this aggressive polling can temporarly lock the register
refresh cycle, because the continuous transfers prevent the hardware from
updating the buffer. This results in stale data reads or select() timeouts
in userspace.
This patch introduces a configurable settle delay via "ti,read-settle-us"
property. If this property is specified, the driver uses a delay before
reading the RTC registers. This provides a sufficient idle time for the
hardware to sync with the register buffer.
Signed-off-by: Adriana Stancu <adriana@arista.com>
---
drivers/rtc/rtc-bq32k.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
index 7ad34539be4d..0cbfa0909732 100644
--- a/drivers/rtc/rtc-bq32k.c
+++ b/drivers/rtc/rtc-bq32k.c
@@ -16,6 +16,7 @@
#include <linux/kstrtox.h>
#include <linux/errno.h>
#include <linux/bcd.h>
+#include <linux/delay.h>
#define BQ32K_SECONDS 0x00 /* Seconds register address */
#define BQ32K_SECONDS_MASK 0x7F /* Mask over seconds value */
@@ -48,6 +49,11 @@ struct bq32k_regs {
uint8_t years;
};
+struct bq32k_data {
+ struct rtc_device *rtc;
+ u32 read_delay_us;
+};
+
static struct i2c_driver bq32k_driver;
static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len)
@@ -89,9 +95,17 @@ static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len)
static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
+ struct bq32k_data *bq32k = dev_get_drvdata(dev);
struct bq32k_regs regs;
int error;
+ /*
+ * When the device doesn't have the interrupt connected, prevent
+ * userpace from polling the RTC registers to frequently.
+ */
+ if (bq32k && bq32k->read_delay_us)
+ usleep_range(bq32k->read_delay_us, bq32k->read_delay_us + 50);
+
error = bq32k_read(dev, ®s, 0, sizeof(regs));
if (error)
return error;
@@ -253,13 +267,18 @@ static void bq32k_sysfs_unregister(struct device *dev)
static int bq32k_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
- struct rtc_device *rtc;
+ struct bq32k_data *bq32k;
uint8_t reg;
int error;
+ uint32_t settle_us = 0;
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
+ bq32k = devm_kzalloc(dev, sizeof(*bq32k), GFP_KERNEL);
+ if (!bq32k)
+ return -ENOMEM;
+
/* Check Oscillator Stop flag */
error = bq32k_read(dev, ®, BQ32K_SECONDS, 1);
if (!error && (reg & BQ32K_STOP)) {
@@ -280,10 +299,13 @@ static int bq32k_probe(struct i2c_client *client)
if (client->dev.of_node)
trickle_charger_of_init(dev, client->dev.of_node);
- rtc = devm_rtc_device_register(&client->dev, bq32k_driver.driver.name,
- &bq32k_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc))
- return PTR_ERR(rtc);
+ bq32k->rtc = devm_rtc_device_register(&client->dev, bq32k_driver.driver.name,
+ &bq32k_rtc_ops, THIS_MODULE);
+ if (IS_ERR(bq32k->rtc))
+ return PTR_ERR(bq32k->rtc);
+
+ device_property_read_u32(dev, "ti,read-settle-us", &settle_us);
+ bq32k->read_delay_us = settle_us;
error = bq32k_sysfs_register(&client->dev);
if (error) {
@@ -293,7 +315,7 @@ static int bq32k_probe(struct i2c_client *client)
}
- i2c_set_clientdata(client, rtc);
+ i2c_set_clientdata(client, bq32k);
return 0;
}
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v1 02/11] media: uapi: v4l2-isp: Add v4l2 ISP extensible statistics definitions
From: Jacopo Mondi @ 2026-04-16 10:03 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, jai.luthra, paul.elder
In-Reply-To: <20260413160331.2611829-3-antoine.bouyer@nxp.com>
Hello Antoine,
thanks for the update
On Mon, Apr 13, 2026 at 06:03:22PM +0200, Antoine Bouyer wrote:
> Extend the v4l2-isp extensible format introduced for isp parameters buffer
> to the statistics buffer as well.
>
> Like for ISP configuration purpose, that will help supporting various ISP
> hardware versions reporting different statistics data with less impact on
> userspace.
>
> The `v4l2_isp_stats_buffer` reuses the `v4l2_isp_params_buffer` container
> definitions, with similar header, versions and flags. V0 and V1 versions
> are provided to match with params versions. On the other side, ENABLE and
> DISABLE flags are not really meaningfull for statistics purpose. So VALID
> and INVALID flags are introduced. Purpose is to force ISP driver to
> validate a statistics buffer, before it is consumed by userspace.
Is this a leftover ?
I don't see VALID and INVALID in this patch and unless I've missed it
badly I don't see them in the next patches.
I'm fine without them, I'm not sure how you intend to use them to
force drivers to validate a statistics buffer.
>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
> include/uapi/linux/media/v4l2-isp.h | 148 +++++++++++++++++++---------
> 1 file changed, 100 insertions(+), 48 deletions(-)
>
> diff --git a/include/uapi/linux/media/v4l2-isp.h b/include/uapi/linux/media/v4l2-isp.h
> index 779168f9058e..e84476280d43 100644
> --- a/include/uapi/linux/media/v4l2-isp.h
> +++ b/include/uapi/linux/media/v4l2-isp.h
> @@ -13,25 +13,33 @@
> #include <linux/types.h>
>
> /**
> - * enum v4l2_isp_params_version - V4L2 ISP parameters versioning
> + * enum v4l2_isp_version - V4L2 ISP serialization format versioning
> *
> - * @V4L2_ISP_PARAMS_VERSION_V0: First version of the V4L2 ISP parameters format
> - * (for compatibility)
> - * @V4L2_ISP_PARAMS_VERSION_V1: First version of the V4L2 ISP parameters format
> + * @V4L2_ISP_VERSION_V0: First version of the V4L2 ISP serialization format
> + * (for compatibility)
> + * @V4L2_ISP_VERSION_V1: First version of the V4L2 ISP serialization format
> *
> * V0 and V1 are identical in order to support drivers compatible with the V4L2
> - * ISP parameters format already upstreamed which use either 0 or 1 as their
> - * versioning identifier. Both V0 and V1 refers to the first version of the
> - * V4L2 ISP parameters format.
> + * ISP format already upstreamed which use either 0 or 1 as their versioning
> + * identifier. Both V0 and V1 refers to the first version of the V4L2 ISP
> + * serialization format.
> *
> - * Future revisions of the V4L2 ISP parameters format should start from the
> + * Future revisions of the V4L2 ISP serialization format should start from the
> * value of 2.
> */
> -enum v4l2_isp_params_version {
> - V4L2_ISP_PARAMS_VERSION_V0 = 0,
> - V4L2_ISP_PARAMS_VERSION_V1
> +enum v4l2_isp_version {
> + V4L2_ISP_VERSION_V0 = 0,
> + V4L2_ISP_VERSION_V1
> };
>
> +/*
> + * Compatibility with existing users of v4l2_isp_params which pre-date the
> + * introduction of v4l2_isp_stats.
> + */
> +#define v4l2_isp_params_version v4l2_isp_version
> +#define V4L2_ISP_PARAMS_VERSION_V0 V4L2_ISP_VERSION_V0
> +#define V4L2_ISP_PARAMS_VERSION_V1 V4L2_ISP_VERSION_V1
> +
> #define V4L2_ISP_PARAMS_FL_BLOCK_DISABLE (1U << 0)
> #define V4L2_ISP_PARAMS_FL_BLOCK_ENABLE (1U << 1)
>
> @@ -39,64 +47,108 @@ enum v4l2_isp_params_version {
> * Reserve the first 8 bits for V4L2_ISP_PARAMS_FL_* flag.
> *
> * Driver-specific flags should be defined as:
> - * #define DRIVER_SPECIFIC_FLAG0 ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(0))
> - * #define DRIVER_SPECIFIC_FLAG1 ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(1))
> + * #define DRIVER_SPECIFIC_FLAG0 ((1U << V4L2_ISP_FL_DRIVER_FLAGS(0))
> + * #define DRIVER_SPECIFIC_FLAG1 ((1U << V4L2_ISP_FL_DRIVER_FLAGS(1))
> */
> -#define V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(n) ((n) + 8)
> +#define V4L2_ISP_FL_DRIVER_FLAGS(n) ((n) + 8)
>
> /**
> - * struct v4l2_isp_params_block_header - V4L2 extensible parameters block header
> - * @type: The parameters block type (driver-specific)
> + * struct v4l2_isp_block_header - V4L2 extensible block header
> + * @type: The parameters or statistics block type (driver-specific)
> * @flags: A bitmask of block flags (driver-specific)
> - * @size: Size (in bytes) of the parameters block, including this header
> + * @size: Size (in bytes) of the block, including this header
> *
> - * This structure represents the common part of all the ISP configuration
> - * blocks. Each parameters block shall embed an instance of this structure type
> - * as its first member, followed by the block-specific configuration data.
> + * This structure represents the common part of all the ISP configuration or
> + * statistic blocks. Each block shall embed an instance of this structure type
> + * as its first member, followed by the block-specific configuration or
> + * statistic data.
> *
> * The @type field is an ISP driver-specific value that identifies the block
> - * type. The @size field specifies the size of the parameters block.
> - *
> - * The @flags field is a bitmask of per-block flags V4L2_PARAMS_ISP_FL_* and
> - * driver-specific flags specified by the driver header.
> + * type. The @size field specifies the size of the block, including this
> + * header.
> */
> -struct v4l2_isp_params_block_header {
> +struct v4l2_isp_block_header {
> __u16 type;
> __u16 flags;
> __u32 size;
> } __attribute__((aligned(8)));
>
> /**
> - * struct v4l2_isp_params_buffer - V4L2 extensible parameters configuration
> - * @version: The parameters buffer version (driver-specific)
> - * @data_size: The configuration data effective size, excluding this header
> - * @data: The configuration data
> + * v4l2_isp_params_block_header - V4L2 extensible parameters block header
> *
> - * This structure contains the configuration parameters of the ISP algorithms,
> - * serialized by userspace into a data buffer. Each configuration parameter
> - * block is represented by a block-specific structure which contains a
> - * :c:type:`v4l2_isp_params_block_header` entry as first member. Userspace
> - * populates the @data buffer with configuration parameters for the blocks that
> - * it intends to configure. As a consequence, the data buffer effective size
> - * changes according to the number of ISP blocks that userspace intends to
> - * configure and is set by userspace in the @data_size field.
> - *
> - * The parameters buffer is versioned by the @version field to allow modifying
> - * and extending its definition. Userspace shall populate the @version field to
> - * inform the driver about the version it intends to use. The driver will parse
> - * and handle the @data buffer according to the data layout specific to the
> - * indicated version and return an error if the desired version is not
> + * This structure represents the common part of all the ISP configuration blocks
> + * and is identical to :c:type:`v4l2_isp_block_header`.
> + *
> + * The @flags field is a bitmask of per-block flags V4L2_ISP_PARAMS_FL_* and
> + * driver-specific flags specified by the driver header.
What if we move this to the documentation of struct v4l2_isp_block_header
and we only document the macro for compatibility reasons like you did
for `v4l2_isp_params_version` ?
We could add the above to the documentation of `struct
v4l2_isp_block_header`:
* The @flags field is a bitmask of per-block flags. If a block is used for
* configuration parameters this field can be a combination of
* V4L2_ISP_PARAMS_FL_ and driver-specific flags.
Depending on the answer on VALID/INVALID we can document the usage of
flags for stats as:
* The @flags field is a bitmask of per-block flags. If a block is used for
* configuration parameters this field can be a combination of
* V4L2_ISP_PARAMS_FL_ and driver-specific flags. If a block is used
* for statistics this fields is used to report optional
* driver-specific flags, if any.
> + */
> +#define v4l2_isp_params_block_header v4l2_isp_block_header
If you accept the above suggestion we can simply document this
/**
* v4l2_isp_params_block_header - V4L2 extensible parameters compatibility
*
* Compatibility with existing users of v4l2_isp_params_block_header
* which pre-date the introduction of v4l2_isp_block_header.
*.
> +
> +/**
> + * v4l2_isp_stats_block_header - V4L2 extensible statistics block header
> + *
> + * This structure represents the common part of all the ISP statistics blocks
> + * and is identical to :c:type:`v4l2_isp_block_header`.
> + *
> + * The @flags field is a bitmask of driver-specific flags specified by the
> + * driver header, as there is no generic flags for statistics.
> + */
> +#define v4l2_isp_stats_block_header v4l2_isp_block_header
Do we need this or should we use v4l2_isp_block_header unconditionally ?
> +
> +/**
> + * struct v4l2_isp_buffer - V4L2 extensible buffer
> + * @version: The extensible buffer version (driver-specific)
> + * @data_size: The data effective size, excluding this header
> + * @data: The configuration or statistics data
> + *
> + * This structure contains ISP configuration parameters or ISP hardware
> + * statistics serialized into a data buffer. Each block is represented by a
> + * block-specific structure which contains a :c:type:`v4l2_isp_block_header`
> + * entry as first member.
> + *
> + * For a parameters block, userspace populates the @data buffer with
Or:
* When used for ISP parameters userspace ..
> + * configuration parameters for the blocks that it intends to configure.
> + * As a consequence, the data buffer effective size changes according to the
> + * number of ISP blocks that userspace intends to configure and is set by
> + * userspace in the @data_size field.
> + *
> + * For a statistics block, behavior is the same as for parameters, except that
Or:
* When used to report ISP statistics the driver populates the
* @data buffer with statistics for each supported measurement
* block. The buffer effective size is set by the driver in the
* @data_size field.
> + * buffer is filled by the ISP driver.
> + *
> + * The buffer is versioned by the @version field to allow modifying
> + * and extending its definition. The writer shall populate the @version field
> + * to inform the reader about the version it intends to use. The reader will
> + * parse and handle the @data buffer according to the data layout specific to
> + * the indicated version and return an error if the desired version is not
> * supported.
Ack, I think using "writer" and "reader" is clear enough to support
both the params and stats use case. If we want more clarity we can add
to "driver" and "userspace" a "(role)" in the two previous paragraph.
Something like:
* When used for ISP parameters userspace (the writer) populates
* the @data buffer ...
> *
> - * For each ISP block that userspace wants to configure, a block-specific
> - * structure is appended to the @data buffer, one after the other without gaps
> - * in between. Userspace shall populate the @data_size field with the effective
> - * size, in bytes, of the @data buffer.
> + * For each ISP block, a block-specific structure is appended to the @data
> + * buffer, one after the other without gaps in between. The writer shall
> + * populate the @data_size field with the effective size, in bytes, of the
> + * @data buffer.
If we want to describe @data_size here we can remove it from the above
two paragraphs maybe. I think it's fine to have it here only.
> */
> -struct v4l2_isp_params_buffer {
> +struct v4l2_isp_buffer {
> __u32 version;
> __u32 data_size;
> __u8 data[] __counted_by(data_size);
> };
>
> +/**
> + * v4l2_isp_params_buffer - V4L2 extensible parameters configuration
s/configuration/compatibility
> + *
> + * This structure contains the configuration parameters of the ISP algorithms,
> + * serialized into a data buffer. It is identical to
> + * :c:type:`v4l2_isp_buffer`.
And here only
* Compatibility with existing users of v4l2_isp_params_buffer which
* pre-date the introduction of v4l2_isp_buffer
> + */
> +#define v4l2_isp_params_buffer v4l2_isp_buffer
> +
> +/**
> + * v4l2_isp_stats_buffer - V4L2 extensible statistics buffer
> + *
> + * This structure contains the statistics data from the ISP hardware,
> + * serialized into a data buffer. It is identical to
> + * :c:type:`v4l2_isp_buffer`.
> + */
> +#define v4l2_isp_stats_buffer v4l2_isp_buffer
Same question as per `v4l2_isp_stats_block_header`. Do we need it ?
Thanks
j
> +
> #endif /* _UAPI_V4L2_ISP_H_ */
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: rtc: ti,bq32k: Add delay on rtc reads
From: Alexandre Belloni @ 2026-04-16 10:03 UTC (permalink / raw)
To: Adriana Stancu
Cc: linux-rtc, devicetree, linux-kernel, robh, krzk+dt, conor+dt
In-Reply-To: <20260416095706.3212158-2-adriana@arista.com>
On 16/04/2026 02:57:05-0700, Adriana Stancu wrote:
> Add a configurable "ti,read-settle-us" property to resolve a limitation
> where aggressive I2C polling prevents the BQ32000's internal register to
> update. This ensures the hardware has sufficient idle time to update its
> buffer, preventing stale data reads on systems where the "interrupts" are
> not configured.
>
Why does it need to be configured?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 1/3] arm64: dts: qcom: eliza: Sort nodes by unit address
From: Krzysztof Kozlowski @ 2026-04-16 10:04 UTC (permalink / raw)
To: Alexander Koskovich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <20260416-eliza-imem-v2-1-fb7a71123451@pm.me>
On 16/04/2026 11:39, Alexander Koskovich wrote:
> Qualcomm DTS uses sorting of MMIO nodes by the unit address, so move
> few nodes in Eliza DTSI to fix that.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> arch/arm64/boot/dts/qcom/eliza.dtsi | 74 ++++++++++++++++++-------------------
> 1 file changed, 37 insertions(+), 37 deletions(-)
>
One patchset per 24h.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 3/8] wifi: ath10k: snoc: support powering on the device via pwrseq
From: Luca Weiss @ 2026-04-16 10:06 UTC (permalink / raw)
To: Dmitry Baryshkov, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski,
Marcel Holtmann, Luiz Augusto von Dentz, Jeff Johnson,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam, Vinod Koul,
Balakrishna Godavarthi, Matthias Kaehlcke
Cc: linux-arm-msm, linux-kernel, devicetree, linux-bluetooth,
linux-wireless, ath10k, linux-pm, Krzysztof Kozlowski,
Bartosz Golaszewski
In-Reply-To: <20260119-wcn3990-pwrctl-v3-3-948df19f5ec2@oss.qualcomm.com>
Hi Dmitry,
On Mon Jan 19, 2026 at 6:07 PM CET, Dmitry Baryshkov wrote:
> The WCN39xx family of WiFi/BT chips incorporates a simple PMU, spreading
> voltages over internal rails. Implement support for using powersequencer
> for this family of ATH10k devices in addition to using regulators.
>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/net/wireless/ath/ath10k/snoc.c | 53 ++++++++++++++++++++++++++++++++--
> drivers/net/wireless/ath/ath10k/snoc.h | 3 ++
> 2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
> index b3f6424c17d3..f72f236fb9eb 100644
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: ISC
> /*
> * Copyright (c) 2018 The Linux Foundation. All rights reserved.
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> */
>
> #include <linux/bits.h>
> @@ -11,6 +12,7 @@
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/property.h>
> +#include <linux/pwrseq/consumer.h>
> #include <linux/regulator/consumer.h>
> #include <linux/remoteproc/qcom_rproc.h>
> #include <linux/of_reserved_mem.h>
> @@ -1023,10 +1025,14 @@ static int ath10k_hw_power_on(struct ath10k *ar)
>
> ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
>
> - ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
> + ret = pwrseq_power_on(ar_snoc->pwrseq);
> if (ret)
> return ret;
>
> + ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
> + if (ret)
> + goto pwrseq_off;
> +
> ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
> if (ret)
> goto vreg_off;
> @@ -1035,18 +1041,28 @@ static int ath10k_hw_power_on(struct ath10k *ar)
>
> vreg_off:
> regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> +pwrseq_off:
> + pwrseq_power_off(ar_snoc->pwrseq);
> +
> return ret;
> }
>
> static int ath10k_hw_power_off(struct ath10k *ar)
> {
> struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
> + int ret_seq = 0;
> + int ret_vreg;
>
> ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
>
> clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
>
> - return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> + ret_vreg = regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> +
> + if (ar_snoc->pwrseq)
> + ret_seq = pwrseq_power_off(ar_snoc->pwrseq);
> +
> + return ret_vreg ? : ret_seq;
> }
>
> static void ath10k_snoc_wlan_disable(struct ath10k *ar)
> @@ -1762,7 +1778,38 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
> goto err_release_resource;
> }
>
> - ar_snoc->num_vregs = ARRAY_SIZE(ath10k_regulators);
> + /*
> + * devm_pwrseq_get() can return -EPROBE_DEFER in two cases:
> + * - it is not supposed to be used
> + * - it is supposed to be used, but the driver hasn't probed yet.
> + *
> + * There is no simple way to distinguish between these two cases, but:
> + * - if it is not supposed to be used, then regulator_bulk_get() will
> + * return all regulators as expected, continuing the probe
> + * - if it is supposed to be used, but wasn't probed yet, we will get
> + * -EPROBE_DEFER from regulator_bulk_get() too.
> + *
> + * For backwards compatibility with DTs specifying regulators directly
> + * rather than using the PMU device, ignore the defer error from
> + * pwrseq.
> + */
> + ar_snoc->pwrseq = devm_pwrseq_get(&pdev->dev, "wlan");
> + if (IS_ERR(ar_snoc->pwrseq)) {
> + ret = PTR_ERR(ar_snoc->pwrseq);
> + ar_snoc->pwrseq = NULL;
> + if (ret != -EPROBE_DEFER)
> + goto err_free_irq;
I'm fairly sure this is now broken with CONFIG_POWER_SEQUENCING=n since
then pwrseq_get() is returning ERR_PTR(-ENOSYS) which is not handled
here.
I'm observing my ath10k_snoc is now failing to probe "with error -38"
which definitely seems to be related, but I haven't debugged it further
yet.
Regards
Luca
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: eliza: Add IMEM node
From: Krzysztof Kozlowski @ 2026-04-16 10:09 UTC (permalink / raw)
To: Alexander Koskovich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <20260416-eliza-imem-v2-3-fb7a71123451@pm.me>
On 16/04/2026 11:40, Alexander Koskovich wrote:
> Add a node for the IMEM found on Eliza, which contains pil-reloc-info
> and the modem tables for IPA, among others.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> arch/arm64/boot/dts/qcom/eliza.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
> index 6fa5679c1a62..551df07e44c6 100644
> --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
> @@ -1029,6 +1029,26 @@ qup_uart14_default: qup-uart14-default-state {
> };
> };
>
> + sram@14680000 {
> + compatible = "qcom,eliza-imem", "mmio-sram";
> + reg = <0x0 0x14680000 0x0 0x2c000>;
> + ranges = <0x0 0x0 0x14680000 0x2c000>;
> +
> + no-memory-wc;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + pilreloc-sram@94c {
> + compatible = "qcom,pil-reloc-info";
> + reg = <0x94c 0xc8>;
> + };
> +
> + ipa_modem_tables: modem-tables-sram@3000 {
> + reg = <0x3000 0x2000>;
I don't think these two should be in the main SoC DTSI. The non-modem
version obviously does not have modem-tables.
Either this is part of board or common DTSI for SMxxxx SoC. Whatever is
chosen here, should be consistent with other platforms.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 02/11] media: uapi: v4l2-isp: Add v4l2 ISP extensible statistics definitions
From: Jacopo Mondi @ 2026-04-16 10:13 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Antoine Bouyer, julien.vuillaumier, alexi.birlinger,
daniel.baluta, peng.fan, frank.li, laurent.pinchart, mchehab,
robh, krzk+dt, conor+dt, michael.riesch, anthony.mcgivern,
linux-media, linux-kernel, devicetree, imx, jai.luthra,
paul.elder
In-Reply-To: <aeCscV9eJbfswiAY@zed>
Hi Antoine
On Thu, Apr 16, 2026 at 12:03:24PM +0200, Jacopo Mondi wrote:
> Hello Antoine,
> thanks for the update
>
> On Mon, Apr 13, 2026 at 06:03:22PM +0200, Antoine Bouyer wrote:
> > Extend the v4l2-isp extensible format introduced for isp parameters buffer
> > to the statistics buffer as well.
> >
> > Like for ISP configuration purpose, that will help supporting various ISP
> > hardware versions reporting different statistics data with less impact on
> > userspace.
> >
> > The `v4l2_isp_stats_buffer` reuses the `v4l2_isp_params_buffer` container
> > definitions, with similar header, versions and flags. V0 and V1 versions
> > are provided to match with params versions. On the other side, ENABLE and
> > DISABLE flags are not really meaningfull for statistics purpose. So VALID
> > and INVALID flags are introduced. Purpose is to force ISP driver to
> > validate a statistics buffer, before it is consumed by userspace.
>
> Is this a leftover ?
>
> I don't see VALID and INVALID in this patch and unless I've missed it
> badly I don't see them in the next patches.
>
> I'm fine without them, I'm not sure how you intend to use them to
> force drivers to validate a statistics buffer.
>
> >
> > Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> > ---
> > include/uapi/linux/media/v4l2-isp.h | 148 +++++++++++++++++++---------
> > 1 file changed, 100 insertions(+), 48 deletions(-)
> >
> > diff --git a/include/uapi/linux/media/v4l2-isp.h b/include/uapi/linux/media/v4l2-isp.h
> > index 779168f9058e..e84476280d43 100644
> > --- a/include/uapi/linux/media/v4l2-isp.h
> > +++ b/include/uapi/linux/media/v4l2-isp.h
> > @@ -13,25 +13,33 @@
> > #include <linux/types.h>
> >
> > /**
> > - * enum v4l2_isp_params_version - V4L2 ISP parameters versioning
> > + * enum v4l2_isp_version - V4L2 ISP serialization format versioning
> > *
> > - * @V4L2_ISP_PARAMS_VERSION_V0: First version of the V4L2 ISP parameters format
> > - * (for compatibility)
> > - * @V4L2_ISP_PARAMS_VERSION_V1: First version of the V4L2 ISP parameters format
> > + * @V4L2_ISP_VERSION_V0: First version of the V4L2 ISP serialization format
> > + * (for compatibility)
> > + * @V4L2_ISP_VERSION_V1: First version of the V4L2 ISP serialization format
> > *
> > * V0 and V1 are identical in order to support drivers compatible with the V4L2
> > - * ISP parameters format already upstreamed which use either 0 or 1 as their
> > - * versioning identifier. Both V0 and V1 refers to the first version of the
> > - * V4L2 ISP parameters format.
> > + * ISP format already upstreamed which use either 0 or 1 as their versioning
> > + * identifier. Both V0 and V1 refers to the first version of the V4L2 ISP
> > + * serialization format.
> > *
> > - * Future revisions of the V4L2 ISP parameters format should start from the
> > + * Future revisions of the V4L2 ISP serialization format should start from the
> > * value of 2.
> > */
> > -enum v4l2_isp_params_version {
> > - V4L2_ISP_PARAMS_VERSION_V0 = 0,
> > - V4L2_ISP_PARAMS_VERSION_V1
> > +enum v4l2_isp_version {
> > + V4L2_ISP_VERSION_V0 = 0,
> > + V4L2_ISP_VERSION_V1
> > };
> >
> > +/*
> > + * Compatibility with existing users of v4l2_isp_params which pre-date the
> > + * introduction of v4l2_isp_stats.
> > + */
> > +#define v4l2_isp_params_version v4l2_isp_version
> > +#define V4L2_ISP_PARAMS_VERSION_V0 V4L2_ISP_VERSION_V0
> > +#define V4L2_ISP_PARAMS_VERSION_V1 V4L2_ISP_VERSION_V1
> > +
> > #define V4L2_ISP_PARAMS_FL_BLOCK_DISABLE (1U << 0)
> > #define V4L2_ISP_PARAMS_FL_BLOCK_ENABLE (1U << 1)
> >
> > @@ -39,64 +47,108 @@ enum v4l2_isp_params_version {
> > * Reserve the first 8 bits for V4L2_ISP_PARAMS_FL_* flag.
> > *
> > * Driver-specific flags should be defined as:
> > - * #define DRIVER_SPECIFIC_FLAG0 ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(0))
> > - * #define DRIVER_SPECIFIC_FLAG1 ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(1))
> > + * #define DRIVER_SPECIFIC_FLAG0 ((1U << V4L2_ISP_FL_DRIVER_FLAGS(0))
> > + * #define DRIVER_SPECIFIC_FLAG1 ((1U << V4L2_ISP_FL_DRIVER_FLAGS(1))
> > */
> > -#define V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(n) ((n) + 8)
> > +#define V4L2_ISP_FL_DRIVER_FLAGS(n) ((n) + 8)
> >
> > /**
> > - * struct v4l2_isp_params_block_header - V4L2 extensible parameters block header
> > - * @type: The parameters block type (driver-specific)
> > + * struct v4l2_isp_block_header - V4L2 extensible block header
> > + * @type: The parameters or statistics block type (driver-specific)
> > * @flags: A bitmask of block flags (driver-specific)
> > - * @size: Size (in bytes) of the parameters block, including this header
> > + * @size: Size (in bytes) of the block, including this header
> > *
> > - * This structure represents the common part of all the ISP configuration
> > - * blocks. Each parameters block shall embed an instance of this structure type
> > - * as its first member, followed by the block-specific configuration data.
> > + * This structure represents the common part of all the ISP configuration or
> > + * statistic blocks. Each block shall embed an instance of this structure type
> > + * as its first member, followed by the block-specific configuration or
> > + * statistic data.
> > *
> > * The @type field is an ISP driver-specific value that identifies the block
> > - * type. The @size field specifies the size of the parameters block.
> > - *
> > - * The @flags field is a bitmask of per-block flags V4L2_PARAMS_ISP_FL_* and
> > - * driver-specific flags specified by the driver header.
> > + * type. The @size field specifies the size of the block, including this
> > + * header.
> > */
> > -struct v4l2_isp_params_block_header {
> > +struct v4l2_isp_block_header {
> > __u16 type;
> > __u16 flags;
> > __u32 size;
> > } __attribute__((aligned(8)));
> >
> > /**
> > - * struct v4l2_isp_params_buffer - V4L2 extensible parameters configuration
> > - * @version: The parameters buffer version (driver-specific)
> > - * @data_size: The configuration data effective size, excluding this header
> > - * @data: The configuration data
> > + * v4l2_isp_params_block_header - V4L2 extensible parameters block header
> > *
> > - * This structure contains the configuration parameters of the ISP algorithms,
> > - * serialized by userspace into a data buffer. Each configuration parameter
> > - * block is represented by a block-specific structure which contains a
> > - * :c:type:`v4l2_isp_params_block_header` entry as first member. Userspace
> > - * populates the @data buffer with configuration parameters for the blocks that
> > - * it intends to configure. As a consequence, the data buffer effective size
> > - * changes according to the number of ISP blocks that userspace intends to
> > - * configure and is set by userspace in the @data_size field.
> > - *
> > - * The parameters buffer is versioned by the @version field to allow modifying
> > - * and extending its definition. Userspace shall populate the @version field to
> > - * inform the driver about the version it intends to use. The driver will parse
> > - * and handle the @data buffer according to the data layout specific to the
> > - * indicated version and return an error if the desired version is not
> > + * This structure represents the common part of all the ISP configuration blocks
> > + * and is identical to :c:type:`v4l2_isp_block_header`.
> > + *
> > + * The @flags field is a bitmask of per-block flags V4L2_ISP_PARAMS_FL_* and
> > + * driver-specific flags specified by the driver header.
>
> What if we move this to the documentation of struct v4l2_isp_block_header
> and we only document the macro for compatibility reasons like you did
> for `v4l2_isp_params_version` ?
>
> We could add the above to the documentation of `struct
> v4l2_isp_block_header`:
>
> * The @flags field is a bitmask of per-block flags. If a block is used for
> * configuration parameters this field can be a combination of
> * V4L2_ISP_PARAMS_FL_ and driver-specific flags.
>
> Depending on the answer on VALID/INVALID we can document the usage of
> flags for stats as:
>
> * The @flags field is a bitmask of per-block flags. If a block is used for
> * configuration parameters this field can be a combination of
> * V4L2_ISP_PARAMS_FL_ and driver-specific flags. If a block is used
Just one note, please use V4L2_ISP_PARAMS_FL_* as otherwise building
documentation will raise warnings.
> * for statistics this fields is used to report optional
> * driver-specific flags, if any.
>
>
> > + */
> > +#define v4l2_isp_params_block_header v4l2_isp_block_header
>
> If you accept the above suggestion we can simply document this
>
> /**
> * v4l2_isp_params_block_header - V4L2 extensible parameters compatibility
> *
> * Compatibility with existing users of v4l2_isp_params_block_header
> * which pre-date the introduction of v4l2_isp_block_header.
> *.
>
> > +
> > +/**
> > + * v4l2_isp_stats_block_header - V4L2 extensible statistics block header
> > + *
> > + * This structure represents the common part of all the ISP statistics blocks
> > + * and is identical to :c:type:`v4l2_isp_block_header`.
> > + *
> > + * The @flags field is a bitmask of driver-specific flags specified by the
> > + * driver header, as there is no generic flags for statistics.
> > + */
> > +#define v4l2_isp_stats_block_header v4l2_isp_block_header
>
> Do we need this or should we use v4l2_isp_block_header unconditionally ?
>
> > +
> > +/**
> > + * struct v4l2_isp_buffer - V4L2 extensible buffer
> > + * @version: The extensible buffer version (driver-specific)
> > + * @data_size: The data effective size, excluding this header
> > + * @data: The configuration or statistics data
> > + *
> > + * This structure contains ISP configuration parameters or ISP hardware
> > + * statistics serialized into a data buffer. Each block is represented by a
> > + * block-specific structure which contains a :c:type:`v4l2_isp_block_header`
> > + * entry as first member.
> > + *
> > + * For a parameters block, userspace populates the @data buffer with
>
> Or:
>
> * When used for ISP parameters userspace ..
>
> > + * configuration parameters for the blocks that it intends to configure.
> > + * As a consequence, the data buffer effective size changes according to the
> > + * number of ISP blocks that userspace intends to configure and is set by
> > + * userspace in the @data_size field.
> > + *
> > + * For a statistics block, behavior is the same as for parameters, except that
>
> Or:
>
> * When used to report ISP statistics the driver populates the
> * @data buffer with statistics for each supported measurement
> * block. The buffer effective size is set by the driver in the
> * @data_size field.
>
> > + * buffer is filled by the ISP driver.
> > + *
> > + * The buffer is versioned by the @version field to allow modifying
> > + * and extending its definition. The writer shall populate the @version field
> > + * to inform the reader about the version it intends to use. The reader will
> > + * parse and handle the @data buffer according to the data layout specific to
> > + * the indicated version and return an error if the desired version is not
> > * supported.
>
> Ack, I think using "writer" and "reader" is clear enough to support
> both the params and stats use case. If we want more clarity we can add
> to "driver" and "userspace" a "(role)" in the two previous paragraph.
> Something like:
>
> * When used for ISP parameters userspace (the writer) populates
> * the @data buffer ...
>
> > *
> > - * For each ISP block that userspace wants to configure, a block-specific
> > - * structure is appended to the @data buffer, one after the other without gaps
> > - * in between. Userspace shall populate the @data_size field with the effective
> > - * size, in bytes, of the @data buffer.
> > + * For each ISP block, a block-specific structure is appended to the @data
> > + * buffer, one after the other without gaps in between. The writer shall
> > + * populate the @data_size field with the effective size, in bytes, of the
> > + * @data buffer.
>
> If we want to describe @data_size here we can remove it from the above
> two paragraphs maybe. I think it's fine to have it here only.
>
> > */
> > -struct v4l2_isp_params_buffer {
> > +struct v4l2_isp_buffer {
> > __u32 version;
> > __u32 data_size;
> > __u8 data[] __counted_by(data_size);
> > };
> >
> > +/**
> > + * v4l2_isp_params_buffer - V4L2 extensible parameters configuration
>
> s/configuration/compatibility
>
> > + *
> > + * This structure contains the configuration parameters of the ISP algorithms,
> > + * serialized into a data buffer. It is identical to
> > + * :c:type:`v4l2_isp_buffer`.
>
> And here only
>
> * Compatibility with existing users of v4l2_isp_params_buffer which
> * pre-date the introduction of v4l2_isp_buffer
>
> > + */
> > +#define v4l2_isp_params_buffer v4l2_isp_buffer
> > +
> > +/**
> > + * v4l2_isp_stats_buffer - V4L2 extensible statistics buffer
> > + *
> > + * This structure contains the statistics data from the ISP hardware,
> > + * serialized into a data buffer. It is identical to
> > + * :c:type:`v4l2_isp_buffer`.
> > + */
> > +#define v4l2_isp_stats_buffer v4l2_isp_buffer
>
> Same question as per `v4l2_isp_stats_block_header`. Do we need it ?
>
> Thanks
> j
>
> > +
> > #endif /* _UAPI_V4L2_ISP_H_ */
> > --
> > 2.51.0
> >
^ permalink raw reply
* Re: [PATCH v3 07/12] rvtrace: Add trace ramsink driver
From: Eric Lin @ 2026-04-16 10:14 UTC (permalink / raw)
To: Anup Patel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
Paul Walmsley, Greg KH, Alexander Shishkin, Ian Rogers,
Alexandre Ghiti, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Jiri Olsa, Adrian Hunter, Liang Kan,
Mayuresh Chitale, Anup Patel, Atish Patra, Andrew Jones,
Sunil V L, linux-riscv, devicetree, linux-kernel,
Mayuresh Chitale, Nick Hu, Vincent Chen, Greentime Hu
In-Reply-To: <20260225062448.4027948-8-anup.patel@oss.qualcomm.com>
On Wed, Feb 25, 2026 at 11:54:43AM +0530, Anup Patel wrote:
Hi Anup,
> From: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>
>
> Add initial implementation of RISC-V trace ramsink driver. The ramsink
> is defined in the RISC-V Trace Control Interface specification.
>
> Co-developed-by: Anup Patel <anup.patel@oss.qualcomm.com>
> Signed-off-by: Anup Patel <anup.patel@oss.qualcomm.com>
> Signed-off-by: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>
> ---
> drivers/hwtracing/rvtrace/Kconfig | 9 +
> drivers/hwtracing/rvtrace/Makefile | 1 +
> drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 322 ++++++++++++++++++++
> 3 files changed, 332 insertions(+)
> create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c
>
> diff --git a/drivers/hwtracing/rvtrace/Kconfig b/drivers/hwtracing/rvtrace/Kconfig
> index ba35c05f3f54..0577f9acb858 100644
> --- a/drivers/hwtracing/rvtrace/Kconfig
> +++ b/drivers/hwtracing/rvtrace/Kconfig
> @@ -21,3 +21,12 @@ config RVTRACE_ENCODER
> default y
> help
> This driver provides support for RISC-V Trace Encoder component.
> +
> +config RVTRACE_RAMSINK
> + tristate "RISC-V Trace Ramsink driver"
> + depends on RVTRACE
> + select DMA_SHARED_BUFFER
> + default y
> + help
> + This driver provides support for Risc-V E-Trace Ramsink
> + component.
> diff --git a/drivers/hwtracing/rvtrace/Makefile b/drivers/hwtracing/rvtrace/Makefile
> index f320693a1fc5..122e575da9fb 100644
> --- a/drivers/hwtracing/rvtrace/Makefile
> +++ b/drivers/hwtracing/rvtrace/Makefile
> @@ -3,3 +3,4 @@
> obj-$(CONFIG_RVTRACE) += rvtrace.o
> rvtrace-y := rvtrace-core.o rvtrace-platform.o
> obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
> +obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
> diff --git a/drivers/hwtracing/rvtrace/rvtrace-ramsink.c b/drivers/hwtracing/rvtrace/rvtrace-ramsink.c
> new file mode 100644
> index 000000000000..5393423c8f28
> --- /dev/null
> +++ b/drivers/hwtracing/rvtrace/rvtrace-ramsink.c
> @@ -0,0 +1,322 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2026 Qualcomm Technologies, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/rvtrace.h>
> +#include <linux/types.h>
> +#include <linux/sizes.h>
> +
> +#define RVTRACE_RAMSINK_STARTLOW_OFF 0x010
> +#define RVTRACE_RAMSINK_STARTHIGH_OFF 0x014
> +#define RVTRACE_RAMSINK_LIMITLOW_OFF 0x018
> +#define RVTRACE_RAMSINK_LIMITHIGH_OFF 0x01c
> +#define RVTRACE_RAMSINK_WPLOW_OFF 0x020
> +#define RVTRACE_RAMSINK_WPHIGH_OFF 0x024
> +#define RVTRACE_RAMSINK_WPLOW_WRAP 0x1
> +#define RVTRACE_RAMSINK_CTRL_MODE_SHIFT 0x4
> +#define RVTRACE_RAMSINK_CTRL_STP_WRAP_SHIFT 0x8
> +
> +enum rvtrace_ramsink_mode {
> + MODE_SRAM,
> + MODE_SMEM
> +};
> +
> +struct rvtrace_ramsink_priv {
> + size_t size;
> + void *va;
> + dma_addr_t start;
> + dma_addr_t end;
> + enum rvtrace_ramsink_mode mode;
> + bool stop_on_wrap;
> + int mem_acc_width;
> +};
> +
> +struct trace_buf {
> + void *base;
> + long cur;
> + size_t len;
> +};
> +
> +static int rvtrace_ramsink_start(struct rvtrace_component *comp)
> +{
> + int ret;
> + u32 val;
> +
> + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> + val |= BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
> + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> + ret = rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> + RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 1,
> + comp->pdata->control_poll_timeout_usecs);
> + if (ret)
> + dev_err(&comp->dev, "failed to start ramsink.\n");
> +
> + return ret;
> +}
> +
> +static int rvtrace_ramsink_stop(struct rvtrace_component *comp)
> +{
> + int ret;
> + u32 val;
> +
> + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> + val &= ~BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
> + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> + ret = rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> + RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 0,
> + comp->pdata->control_poll_timeout_usecs);
> + if (ret) {
> + dev_err(&comp->dev, "failed to stop ramsink.\n");
> + return ret;
> + }
> +
> + return rvtrace_comp_poll_empty(comp);
> +}
> +
> +static void tbuf_to_pbuf_copy(struct trace_buf *src, struct trace_buf *dst, size_t size)
> +{
> + int bytes_dst, bytes_src, bytes;
> + void *dst_addr, *src_addr;
> +
> + while (size) {
> + src_addr = src->base + src->cur;
> + dst_addr = dst->base + dst->cur;
> +
> + /* Ensure that there are no OOB memory accesses */
> + if (dst->len - dst->cur < size)
> + bytes_dst = dst->len - dst->cur;
> + else
> + bytes_dst = size;
> +
> + if (src->len - src->cur < size)
> + bytes_src = src->len - src->cur;
> + else
> + bytes_src = size;
> + bytes = bytes_dst < bytes_src ? bytes_dst : bytes_src;
> + memcpy(dst_addr, src_addr, bytes);
> + dst->cur = (dst->cur + bytes) % dst->len;
> + src->cur = (src->cur + bytes) % src->len;
> + size -= bytes;
> + }
> +}
> +
> +static size_t rvtrace_ramsink_copyto_auxbuf(struct rvtrace_component *comp,
> + struct rvtrace_perf_auxbuf *buf)
> +{
> + struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
> + size_t size_wp_end = 0, size_start_wp = 0;
> + struct trace_buf src, dst;
> + u32 wp_low, wp_high, trram_ctrl;
> + u64 buf_cur_head;
> +
> + dst.base = buf->base;
> + dst.len = buf->length;
> + dst.cur = buf->pos;
> + src.base = priv->va;
> + src.len = priv->size;
> + wp_low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPLOW_OFF);
> + wp_high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPHIGH_OFF);
> + buf_cur_head = (u64)(wp_high) << 32 | wp_low;
> + trram_ctrl = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
The trram_ctrl is not used. I think we should remove it.
> + if (buf_cur_head & 0x1) {
> + buf_cur_head &= ~RVTRACE_RAMSINK_WPLOW_WRAP;
> + rvtrace_write32(comp->pdata, lower_32_bits(priv->start),
> + RVTRACE_RAMSINK_WPLOW_OFF);
> + rvtrace_write32(comp->pdata, upper_32_bits(priv->start),
> + RVTRACE_RAMSINK_WPHIGH_OFF);
> + src.cur = buf_cur_head - priv->start;
> + size_wp_end = priv->end - buf_cur_head;
> + tbuf_to_pbuf_copy(&src, &dst, size_wp_end);
> + }
> +
> + src.cur = 0;
> + size_start_wp = buf_cur_head - priv->start;
> + tbuf_to_pbuf_copy(&src, &dst, size_start_wp);
> + dev_dbg(&comp->dev, "Copied %zu bytes\n", size_wp_end + size_start_wp);
Currently, rvtrace_ramsink_copyto_auxbuf() only resets the RAM sink
Write Pointer (WP) if a trace buffer wrap has occurred. If no wrap
occurs, it copies the trace data from the buffer's start address to
the current WP, but the WP is not reset to buffer's start address.
This causes an issue during process context switches. When a traced
program is context-switched out, the trace data is copied to the
perf AUX buffer. If the WP is not reset at this point, the next
time the program is scheduled and subsequently switched out, the
function will once again copy from the start of the buffer up to the
new WP. This unintentionally re-copies the old trace data from the
previous scheduled slice.
Regards,
Eric Lin
> + return (size_wp_end + size_start_wp);
> +}
> +
> +static int rvtrace_ramsink_setup_buf(struct rvtrace_component *comp,
> + struct rvtrace_ramsink_priv *priv)
> +{
> + struct device *pdev = comp->pdata->dev;
> + u64 start_min, limit_max, end;
> + u32 low, high;
> + int ret;
> +
> + /* Probe min and max values for start and limit registers */
> + rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTLOW_OFF);
> + rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTHIGH_OFF);
> + low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTLOW_OFF);
> + high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTHIGH_OFF);
> + start_min = (u64)(high) << 32 | low;
> +
> + rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITLOW_OFF);
> + rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITHIGH_OFF);
> + low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
> + high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
> + limit_max = (u64)(high) << 32 | low;
> +
> + /* Set DMA mask based on the maximum allowed limit address */
> + ret = dma_set_mask_and_coherent(pdev, DMA_BIT_MASK(fls64(limit_max)));
> + if (ret)
> + return ret;
> +
> + priv->va = dma_alloc_coherent(pdev, priv->size, &priv->start, GFP_KERNEL);
> + if (!priv->va)
> + return -ENOMEM;
> +
> + priv->end = priv->start + priv->size;
> + if (priv->end <= start_min || priv->start >= limit_max) {
> + dma_free_coherent(pdev, priv->size, priv->va, priv->start);
> + dev_err(&comp->dev, "DMA memory not addressable by device\n");
> + return -EINVAL;
> + }
> +
> + /* Setup ram sink start addresses */
> + if (priv->start < start_min) {
> + dev_warn(&comp->dev, "Ramsink start address updated from %pad to %pad\n",
> + &priv->start, &start_min);
> + priv->va += start_min - priv->start;
> + priv->start = start_min;
> + }
> +
> + rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_STARTLOW_OFF);
> + rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_STARTHIGH_OFF);
> + rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_WPLOW_OFF);
> + rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_WPHIGH_OFF);
> + /* Setup ram sink limit addresses */
> + if (priv->end > limit_max) {
> + dev_warn(&comp->dev, "Ramsink limit address updated from %pad to %pad\n",
> + &priv->end, &limit_max);
> + priv->end = limit_max;
> + priv->size = priv->end - priv->start;
> + }
> +
> + /* Limit address needs to be set to end - mem_access_width to avoid overflow */
> + end = priv->end - priv->mem_acc_width;
> + rvtrace_write32(comp->pdata, lower_32_bits(end), RVTRACE_RAMSINK_LIMITLOW_OFF);
> + rvtrace_write32(comp->pdata, upper_32_bits(end), RVTRACE_RAMSINK_LIMITHIGH_OFF);
> + low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
> + high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
> + end = (u64)(high) << 32 | low;
> + if (end != (priv->end - 4)) {
> + dev_warn(&comp->dev, "Ramsink limit address updated from %pad to %pad\n",
> + &priv->end, &end);
> + priv->end = end;
> + priv->size = priv->end - priv->start;
> + }
> +
> + return 0;
> +}
> +
> +static int rvtrace_ramsink_setup(struct rvtrace_component *comp)
> +{
> + struct rvtrace_ramsink_priv *priv;
> + u32 trram_ctrl;
> + int ret;
> +
> + priv = devm_kzalloc(&comp->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + /* Derive RAM sink memory size based on component implementation ID */
> + switch (comp->pdata->impid) {
> + default:
> + priv->size = SZ_1M;
> + priv->mode = MODE_SMEM;
> + priv->stop_on_wrap = false;
> + priv->mem_acc_width = 4;
> + break;
> + }
> +
> + trram_ctrl = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> + trram_ctrl |= priv->mode << RVTRACE_RAMSINK_CTRL_MODE_SHIFT;
> + rvtrace_write32(comp->pdata, trram_ctrl, RVTRACE_COMPONENT_CTRL_OFFSET);
> + trram_ctrl = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> + dev_dbg(&comp->dev, "mode: %s\n", (trram_ctrl >> RVTRACE_RAMSINK_CTRL_MODE_SHIFT) & 0x1 ?
> + "SMEM" : "SRAM");
> +
> + trram_ctrl |= priv->stop_on_wrap << RVTRACE_RAMSINK_CTRL_STP_WRAP_SHIFT;
> + rvtrace_write32(comp->pdata, trram_ctrl, RVTRACE_COMPONENT_CTRL_OFFSET);
> +
> + ret = rvtrace_ramsink_setup_buf(comp, priv);
> + if (!ret)
> + dev_set_drvdata(&comp->dev, priv);
> +
> + return ret;
> +}
> +
> +static void rvtrace_ramsink_cleanup(struct rvtrace_component *comp)
> +{
> + struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
> +
> + dma_free_coherent(comp->pdata->dev, priv->size, priv->va, priv->start);
> +}
> +
> +static int rvtrace_ramsink_probe(struct rvtrace_component *comp)
> +{
> + int ret;
> +
> + ret = rvtrace_ramsink_setup(comp);
> + if (ret)
> + return dev_err_probe(&comp->dev, ret, "failed to setup ramsink.\n");
> +
> + ret = rvtrace_enable_component(comp->pdata);
> + if (ret)
> + return dev_err_probe(&comp->dev, ret, "failed to enable ramsink.\n");
> +
> + return ret;
> +}
> +
> +static void rvtrace_ramsink_remove(struct rvtrace_component *comp)
> +{
> + int ret;
> +
> + ret = rvtrace_disable_component(comp->pdata);
> + if (ret)
> + dev_err(&comp->dev, "failed to disable ramsink.\n");
> +
> + rvtrace_ramsink_cleanup(comp);
> +}
> +
> +static struct rvtrace_component_id rvtrace_ramsink_ids[] = {
> + { .type = RVTRACE_COMPONENT_TYPE_RAMSINK,
> + .version = rvtrace_component_mkversion(1, 0), },
> + {},
> +};
> +
> +static struct rvtrace_driver rvtrace_ramsink_driver = {
> + .id_table = rvtrace_ramsink_ids,
> + .copyto_auxbuf = rvtrace_ramsink_copyto_auxbuf,
> + .stop = rvtrace_ramsink_stop,
> + .start = rvtrace_ramsink_start,
> + .probe = rvtrace_ramsink_probe,
> + .remove = rvtrace_ramsink_remove,
> + .driver = {
> + .name = "rvtrace-ramsink",
> + },
> +};
> +
> +static int __init rvtrace_ramsink_init(void)
> +{
> + return rvtrace_register_driver(&rvtrace_ramsink_driver);
> +}
> +
> +static void __exit rvtrace_ramsink_exit(void)
> +{
> + rvtrace_unregister_driver(&rvtrace_ramsink_driver);
> +}
> +
> +module_init(rvtrace_ramsink_init);
> +module_exit(rvtrace_ramsink_exit);
> +
> +/* Module information */
> +MODULE_AUTHOR("Mayuresh Chitale");
> +MODULE_DESCRIPTION("RISC-V Trace Ramsink Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: eliza: Add IMEM node
From: Alexander Koskovich @ 2026-04-16 10:15 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <9592f205-7467-462b-874e-7fc599e5277a@kernel.org>
On Thursday, April 16th, 2026 at 6:09 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 16/04/2026 11:40, Alexander Koskovich wrote:
> > Add a node for the IMEM found on Eliza, which contains pil-reloc-info
> > and the modem tables for IPA, among others.
> >
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> > arch/arm64/boot/dts/qcom/eliza.dtsi | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
> > index 6fa5679c1a62..551df07e44c6 100644
> > --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
> > @@ -1029,6 +1029,26 @@ qup_uart14_default: qup-uart14-default-state {
> > };
> > };
> >
> > + sram@14680000 {
> > + compatible = "qcom,eliza-imem", "mmio-sram";
> > + reg = <0x0 0x14680000 0x0 0x2c000>;
> > + ranges = <0x0 0x0 0x14680000 0x2c000>;
> > +
> > + no-memory-wc;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + pilreloc-sram@94c {
> > + compatible = "qcom,pil-reloc-info";
> > + reg = <0x94c 0xc8>;
> > + };
> > +
> > + ipa_modem_tables: modem-tables-sram@3000 {
> > + reg = <0x3000 0x2000>;
>
> I don't think these two should be in the main SoC DTSI. The non-modem
> version obviously does not have modem-tables.
>
> Either this is part of board or common DTSI for SMxxxx SoC. Whatever is
> chosen here, should be consistent with other platforms.
Would you want the IPA node, MPSS remoteproc, etc to follow same pattern? Can
just throw them in a sm7550.dtsi.
Since sm7550.dtsi wouldn't have any consumers until I push my board dts, I guess should hold off on this until then?
The node sort part of this patchset can be applied separately.
>
> Best regards,
> Krzysztof
>
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: eliza: Add IMEM node
From: Konrad Dybcio @ 2026-04-16 10:19 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alexander Koskovich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <9592f205-7467-462b-874e-7fc599e5277a@kernel.org>
On 4/16/26 12:09 PM, Krzysztof Kozlowski wrote:
> On 16/04/2026 11:40, Alexander Koskovich wrote:
>> Add a node for the IMEM found on Eliza, which contains pil-reloc-info
>> and the modem tables for IPA, among others.
>>
>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>> ---
>> arch/arm64/boot/dts/qcom/eliza.dtsi | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
>> index 6fa5679c1a62..551df07e44c6 100644
>> --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
>> @@ -1029,6 +1029,26 @@ qup_uart14_default: qup-uart14-default-state {
>> };
>> };
>>
>> + sram@14680000 {
>> + compatible = "qcom,eliza-imem", "mmio-sram";
>> + reg = <0x0 0x14680000 0x0 0x2c000>;
>> + ranges = <0x0 0x0 0x14680000 0x2c000>;
>> +
>> + no-memory-wc;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + pilreloc-sram@94c {
>> + compatible = "qcom,pil-reloc-info";
>> + reg = <0x94c 0xc8>;
>> + };
>> +
>> + ipa_modem_tables: modem-tables-sram@3000 {
>> + reg = <0x3000 0x2000>;
>
> I don't think these two should be in the main SoC DTSI. The non-modem
> version obviously does not have modem-tables.
That's not quite right, IMEM is partitioned to have it either way, even
if it stays unused. You'll notice this slice is there even on platforms
that were designed with no modem in any SKU
Konrad
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: eliza: Add IMEM node
From: Konrad Dybcio @ 2026-04-16 10:20 UTC (permalink / raw)
To: Alexander Koskovich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <20260416-eliza-imem-v2-3-fb7a71123451@pm.me>
On 4/16/26 11:40 AM, Alexander Koskovich wrote:
> Add a node for the IMEM found on Eliza, which contains pil-reloc-info
> and the modem tables for IPA, among others.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> arch/arm64/boot/dts/qcom/eliza.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
> index 6fa5679c1a62..551df07e44c6 100644
> --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
> @@ -1029,6 +1029,26 @@ qup_uart14_default: qup-uart14-default-state {
> };
> };
>
> + sram@14680000 {
> + compatible = "qcom,eliza-imem", "mmio-sram";
> + reg = <0x0 0x14680000 0x0 0x2c000>;
> + ranges = <0x0 0x0 0x14680000 0x2c000>;
> +
> + no-memory-wc;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + pilreloc-sram@94c {
Since the node below has more than one dash, I'd expect this name
not to be squished too
Otherwise
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
From: Laurentiu Palcu @ 2026-04-16 10:20 UTC (permalink / raw)
To: Marek Vasut
Cc: Marco Felsch, Liu Ying, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Marek Vasut, dri-devel, devicetree, linux-kernel
In-Reply-To: <bc7d5955-600a-48eb-b897-3928ace275a5@nabladev.com>
Hi Marco, Marek, Ying,
On Tue, Mar 31, 2026 at 01:22:19AM +0200, Marek Vasut wrote:
> On 3/30/26 8:29 AM, Marco Felsch wrote:
>
> Hello Marco,
>
> > > > On 26-03-29, Liu Ying wrote:
> > > > > LDB's parent device could be a syscon which doesn't allow a reg property
> > > > > to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
> > > > > has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
> > > > > without a reg property(LDB is also a child device of the Media blk-ctrl).
> > > > > To make the LDB schema be able to describe LDBs without the reg property
> > > > > like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> > > >
> > > > NACK, we want to describe the HW and from HW PoV the LDB is and was
> > > > always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
> > > >
> > > > > Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> > > >
> > > > Therefore I would just revert this patch completely.
> > > Last time, I pointed out the hardware is part of syscon, but as a subnode
> > > and therefore with reg properties. What is the problem there ?
> >
> > To quote the DT spec here:
> >
> > """
> > The reg property describes the address of the device’s resources within
> > the address space defined by its parent bus.
> > """
>
> That parent bus would be the syscon, wouldn't it.
>
> > The parent bus is not the parent iomuxc (i.MX6X) nor the blk-ctrl
> > (i.MX8MP/93) device. Therefore this is wrong IMHO and should be dropped.
>
> How so ? What is the parent bus ?
It looks like the discussion is stuck on 2 things:
1. DT spec argument hasn't been fully addressed: Marek asked "what is
the parent bus if not the syscon?". That question is still open. Syscon
children carrying 'reg' to express their offset within the parent's MMIO
range is a common upstream pattern. Marco, can you explain why syscon
doesn't qualify as the address space provider here?
2. Regardless of (1), removing 'reg' from the imx6sx/imx8mp DT nodes is
an ABI break, those nodes are already upstream. Ying's patch is
the minimal fix that respects that constraint while unblocking imx93.
Marco, a broader cleanup of 'reg' from imx6sx/imx8mp would need to be a
separate patch with an explicit plan for the ABI impact... So, for now, my
suggestion is to move forward with Ying's solution.
--
Thanks,
Laurentiu
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: sram: Document qcom,eliza-imem
From: Krzysztof Kozlowski @ 2026-04-16 10:21 UTC (permalink / raw)
To: Alexander Koskovich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <20260416-eliza-imem-v2-2-fb7a71123451@pm.me>
On 16/04/2026 11:40, Alexander Koskovich wrote:
> Add compatible for Eliza SoC IMEM.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> Documentation/devicetree/bindings/sram/sram.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/5] clk: qcom: camcc-milos: Declare icc path dependency for CAMSS_TOP_GDSC
From: Konrad Dybcio @ 2026-04-16 10:22 UTC (permalink / raw)
To: Luca Weiss, Georgi Djakov, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio
Cc: ~postmarketos/upstreaming, phone-devel, linux-pm, linux-kernel,
linux-arm-msm, linux-clk, devicetree
In-Reply-To: <20260116-milos-camcc-icc-v1-4-400b7fcd156a@fairphone.com>
On 1/16/26 2:17 PM, Luca Weiss wrote:
> This GDSC requires an interconnect path to be enabled, otherwise the
> GDSC will be stuck on 'off' and can't be enabled.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Andrea della Porta @ 2026-04-16 10:30 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andrea della Porta, linux-pwm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, devicetree,
linux-rpi-kernel, linux-arm-kernel, linux-kernel, Naushir Patuck,
Stanimir Varbanov, mbrugger
In-Reply-To: <adkrHkANCzxO8KUP@monoceros>
Hi Uwe,
On 19:31 Fri 10 Apr , Uwe Kleine-König wrote:
> Hello Andrea,
>
> nice work for a v2!
Thanks!
>
> On Fri, Apr 10, 2026 at 04:09:58PM +0200, Andrea della Porta wrote:
<...snip...>
> > +#define RP1_PWM_GLOBAL_CTRL 0x000
> > +#define RP1_PWM_CHANNEL_CTRL(x) (0x014 + ((x) * 0x10))
> > +#define RP1_PWM_RANGE(x) (0x018 + ((x) * 0x10))
> > +#define RP1_PWM_PHASE(x) (0x01C + ((x) * 0x10))
> > +#define RP1_PWM_DUTY(x) (0x020 + ((x) * 0x10))
> > +
> > +/* 8:FIFO_POP_MASK + 0:Trailing edge M/S modulation */
> > +#define RP1_PWM_CHANNEL_DEFAULT (BIT(8) + BIT(0))
>
> Please add a #define for BIT(8) and then use that and
> FIELD_PREP(RP1_PWM_MODE, RP1_PWM_MODE_SOMENICENAME) to define the
> constant. Also I would define it below the register defines.
Ack.
>
> > +#define RP1_PWM_CHANNEL_ENABLE(x) BIT(x)
> > +#define RP1_PWM_POLARITY BIT(3)
> > +#define RP1_PWM_SET_UPDATE BIT(31)
> > +#define RP1_PWM_MODE_MASK GENMASK(1, 0)
>
> s/_MASK// please
>
> It would be great if the bitfield's names started with the register
> name.
Ack.
>
> > +
> > +#define RP1_PWM_NUM_PWMS 4
> > +
> > +struct rp1_pwm {
> > + struct regmap *regmap;
> > + struct clk *clk;
> > + unsigned long clk_rate;
> > + bool clk_enabled;
> > +};
> > +
> > +struct rp1_pwm_waveform {
> > + u32 period_ticks;
> > + u32 duty_ticks;
> > + bool enabled;
> > + bool inverted_polarity;
> > +};
> > +
> > +static const struct regmap_config rp1_pwm_regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x60,
>
> I'm not a fan of aligning the = in a struct, still more if it fails like
> here. Please consistently align all =s, or even better, use a single
> space before each =. (Same for the struct definitions above, but I won't
> insist.)
Let's use the single space.
>
> > +};
> > +
> > +static void rp1_pwm_apply_config(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + u32 value;
> > +
> > + /* update the changed registers on the next strobe to avoid glitches */
> > + regmap_read(rp1->regmap, RP1_PWM_GLOBAL_CTRL, &value);
> > + value |= RP1_PWM_SET_UPDATE;
> > + regmap_write(rp1->regmap, RP1_PWM_GLOBAL_CTRL, value);
>
> I assume there is a glitch if I update two channels and the old
> configuration of the first channel ends while I'm in the middle of
> configuring the second?
The configuration registers are per-channel but the update flag is global.
I don't have details of the hw insights, my best guess is that anything that
you set in the registers before updating the flag will take effect, so there
should be no glitches.
>
> > +}
> > +
> > +static int rp1_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > +
> > + /* init channel to reset defaults */
> > + regmap_write(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), RP1_PWM_CHANNEL_DEFAULT);
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_round_waveform_tohw(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const struct pwm_waveform *wf,
> > + void *_wfhw)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u64 clk_rate = rp1->clk_rate;
> > + u64 ticks;
>
> if (!wf->period_length_ns)
> wfhw->enabled = false
> return 0;
>
> > + ticks = mul_u64_u64_div_u64(wf->period_length_ns, clk_rate, NSEC_PER_SEC);
>
> To ensure this doesn't overflow please fail to probe the driver if
> clk_rate > 1 GHz with an explaining comment. (Or alternatively calculate
> the length of period_ticks = U32_MAX and skip the calculation if
> wf->period_length_ns is bigger.)
Ack.
>
> > + if (ticks > U32_MAX)
> > + ticks = U32_MAX;
> > + wfhw->period_ticks = ticks;
>
> What happens if wf->period_length_ns > 0 but ticks == 0?
I've added a check, returning 1 to signal teh round-up, and a minimum tick of 1
in this case.
>
> > + if (wf->duty_offset_ns + wf->duty_length_ns >= wf->period_length_ns) {
>
> The maybe surprising effect here is that in the two cases
>
> wf->duty_offset_ns == wf->period_length_ns and wf->duty_length_ns == 0
>
> and
>
> wf->duty_length_ns == wf->period_length_ns and wf->duty_offset_ns == 0
>
> you're configuring inverted polarity. I doesn't matter technically
> because the result is the same, but for consumers still using pwm_state
> this is irritating. That's why pwm-stm32 uses inverted polarity only if
> also wf->duty_length_ns and wf->duty_offset_ns are non-zero.
Ack.
>
> > + ticks = mul_u64_u64_div_u64(wf->period_length_ns - wf->duty_length_ns,
> > + clk_rate, NSEC_PER_SEC);
>
> The rounding is wrong here. You should pick the biggest duty_length not
> bigger than wf->duty_length_ns, so you have to use
>
> ticks = wfhw->period_ticks - mul_u64_u64_div_u64(wf->duty_length_ns, clk_rate, NSEC_PER_SEC):
>
> . I see this is a hole in the pwmtestperf coverage.
Ack.
>
> > + wfhw->inverted_polarity = true;
> > + } else {
> > + ticks = mul_u64_u64_div_u64(wf->duty_length_ns, clk_rate, NSEC_PER_SEC);
> > + wfhw->inverted_polarity = false;
> > + }
> > +
> > + if (ticks > wfhw->period_ticks)
> > + ticks = wfhw->period_ticks;
>
> You can and should assume that wf->duty_length_ns <=
> wf->period_length_ns. Then the if condition can never become true.
Ack.
>
> > + wfhw->duty_ticks = ticks;
> > +
> > + wfhw->enabled = !!wfhw->duty_ticks;
> > +
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_round_waveform_fromhw(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const void *_wfhw,
> > + struct pwm_waveform *wf)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + const struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u64 clk_rate = rp1->clk_rate;
> > + u32 ticks;
> > +
> > + memset(wf, 0, sizeof(*wf));
>
> wf = (struct pwm_waveform){ };
>
> is usually more efficient.
Ack.
>
> > + if (!wfhw->enabled)
> > + return 0;
> > +
> > + wf->period_length_ns = DIV_ROUND_UP_ULL((u64)wfhw->period_ticks * NSEC_PER_SEC, clk_rate);
> > +
> > + if (wfhw->inverted_polarity) {
> > + wf->duty_length_ns = DIV_ROUND_UP_ULL((u64)wfhw->duty_ticks * NSEC_PER_SEC,
> > + clk_rate);
> > + } else {
> > + wf->duty_offset_ns = DIV_ROUND_UP_ULL((u64)wfhw->duty_ticks * NSEC_PER_SEC,
> > + clk_rate);
> > + ticks = wfhw->period_ticks - wfhw->duty_ticks;
> > + wf->duty_length_ns = DIV_ROUND_UP_ULL((u64)ticks * NSEC_PER_SEC, clk_rate);
> > + }
>
> This needs adaption after the rounding issue in tohw is fixed.
Ack.
>
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_write_waveform(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const void *_wfhw)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + const struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u32 value;
> > +
> > + /* set period and duty cycle */
> > + regmap_write(rp1->regmap,
> > + RP1_PWM_RANGE(pwm->hwpwm), wfhw->period_ticks);
> > + regmap_write(rp1->regmap,
> > + RP1_PWM_DUTY(pwm->hwpwm), wfhw->duty_ticks);
> > +
> > + /* set polarity */
> > + regmap_read(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), &value);
> > + if (!wfhw->inverted_polarity)
> > + value &= ~RP1_PWM_POLARITY;
> > + else
> > + value |= RP1_PWM_POLARITY;
> > + regmap_write(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), value);
> > +
> > + /* enable/disable */
> > + regmap_read(rp1->regmap, RP1_PWM_GLOBAL_CTRL, &value);
> > + if (wfhw->enabled)
> > + value |= RP1_PWM_CHANNEL_ENABLE(pwm->hwpwm);
> > + else
> > + value &= ~RP1_PWM_CHANNEL_ENABLE(pwm->hwpwm);
> > + regmap_write(rp1->regmap, RP1_PWM_GLOBAL_CTRL, value);
>
> You can exit early if wfhw->enabled is false after clearing the channel
> enable bit.
Ack.
>
> > + rp1_pwm_apply_config(chip, pwm);
> > +
> > + return 0;
> > +}
> > +
<,...snip...>
> > + }
> > +
> > + return 0;
> > +
> > +err_disable_clk:
> > + clk_disable_unprepare(rp1->clk);
> > +
> > + return ret;
> > +}
>
> On remove you miss to balance the call to clk_prepare_enable() (if no
> failed call to clk_prepare_enable() in rp1_pwm_resume() happend).
Since this driver now exports a syscon, it's only builtin (=Y) so
it cannot be unloaded.
I've also avoided the .remove callback via .suppress_bind_attrs.
>
> > +
> > +static int rp1_pwm_suspend(struct device *dev)
> > +{
> > + struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > +
> > + if (rp1->clk_enabled) {
> > + clk_disable_unprepare(rp1->clk);
> > + rp1->clk_enabled = false;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_resume(struct device *dev)
> > +{
> > + struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(rp1->clk);
> > + if (ret) {
> > + dev_err(dev, "Failed to enable clock on resume: %d\n", ret);
>
> Please use %pe for error codes.
Ack.
Best regards,
Andrea
>
> > + return ret;
> > + }
> > +
> > + rp1->clk_enabled = true;
> > +
> > + return 0;
> > +}
>
> Best regards
> Uwe
^ permalink raw reply
* Re: [PATCH v1 01/11] media: Documentation: uapi: Update V4L2 ISP for extensible stats
From: Jacopo Mondi @ 2026-04-16 10:27 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, jai.luthra, paul.elder
In-Reply-To: <20260413160331.2611829-2-antoine.bouyer@nxp.com>
Hi Antoine
On Mon, Apr 13, 2026 at 06:03:21PM +0200, Antoine Bouyer wrote:
> Add driver documentation for V4L2 ISP generic statistics format, mainly
> copied from the generic parameters one.
>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
> .../userspace-api/media/v4l/v4l2-isp.rst | 39 +++++++++++++++++--
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/v4l2-isp.rst b/Documentation/userspace-api/media/v4l/v4l2-isp.rst
> index facf6dba1ca7..9e9c71bfc0df 100644
> --- a/Documentation/userspace-api/media/v4l/v4l2-isp.rst
> +++ b/Documentation/userspace-api/media/v4l/v4l2-isp.rst
> @@ -24,7 +24,7 @@ correctly populate the buffer header with the generic parameters format version
> and with the size (in bytes) of the binary data buffer where it will store the
> ISP blocks configuration.
>
> -Each *ISP configuration block* is preceded by an header implemented by the
> +Each *ISP configuration block* is preceded by a header implemented by the
> :c:type:`v4l2_isp_params_block_header` structure, followed by the configuration
I would update all occurences of v4l2_isp_params_block_header with
v4l2_isp_block_header (same for the stats counterpart).
The same goes for v4l2_isp_params_buffer and v4l2_isp_stats_buffer to be
replaced with v4l2_isp_buffer.
The compatibilty types should only be there to allow existing
applications to continue working.
> parameters for that specific block, defined by the ISP driver specific data
> types.
> @@ -32,8 +32,8 @@ types.
> Userspace applications are responsible for correctly populating each block's
> header fields (type, flags and size) and the block-specific parameters.
>
> -ISP block enabling, disabling and configuration
> ------------------------------------------------
> +ISP parameters block enabling, disabling and configuration
> +----------------------------------------------------------
>
> When userspace wants to configure and enable an ISP block it shall fully
> populate the block configuration and set the V4L2_ISP_PARAMS_FL_BLOCK_ENABLE
> @@ -59,7 +59,38 @@ definition without invalidating the existing ones.
> ISP statistics
> ==============
>
> -Support for generic statistics format is not yet implemented in Video4Linux2.
> +The generic ISP statistics format is similar to the generic ISP configuration
Similar or identical ? :)
> +parameters format. It is realized by defining a C structure that contains a
> +header, followed by binary buffer where the ISP driver copies a variable number
> +of ISP statistics block.
> +
> +The :c:type:`v4l2_isp_stats_buffer` structure defines the buffer header which
In this case I would say:
Extensible statistics buffers have :c:type:`v4l2_isp_buffer` header
followed by ...
> +is followed by a binary buffer of ISP statistics data. ISP drivers shall
> +correctly populate the buffer header with the generic statistics format version
s/generic statistics format version/serialization format version/
Please check if this has to be changed for paramters as well
> +and with the size (in bytes) of the binary data buffer where it will store the
> +ISP statistics data.
and with the size (in bytes) of the binary data buffer where ISP statistics
data are serialized.
> +
> +Each *ISP statistics block* is preceded by a header implemented by the
> +:c:type:`v4l2_isp_stats_block_header` structure, followed by the statistics
Use v4l2_isp_block_header
> +data for that specific block, defined by the ISP driver specific data types.
> +
> +Drivers are responsible for correctly populating each block's header fields
> +(type and size) and the block-specific statistics data. The flags field can be
> +left empty, it is not relevant for statistics data.
I would say that
The flags field can be populated with driver-specific flags, if any.
> +
> +ISP statistics block configuration
> +----------------------------------
> +
> +When an ISP driver wants to share statistics from an ISP block, it shall fully
> +populate the block statistics.
> +
> +When ISP driver wants userspace to ignore statistics from an ISP block, it can
> +either simply omit the full block, or omit the additional data after header.
> +In second case, block header's `size` shall be filled with header structure's
> +size only.
Mmmm, I would not do that. Drivers should only report stats blocks if
populated. Is there a use case for reporting only the header ? (we
allow this for params as userspace can enable/disable blocks).
> +
> +Extension to the statistics format can be implemented by adding new blocks
> +definition without invalidating the existing ones.
Thanks!
j
>
> V4L2 ISP uAPI data types
> ========================
> --
> 2.51.0
>
>
^ 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