* [PATCH v2 2/5] arm64: dts: renesas: r9a09g077: Add DU node
From: Prabhakar @ 2026-06-15 11:54 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-renesas-soc, devicetree, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615115455.1412098-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add Display Unit (DU) node to SoC DTSI.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No change
---
arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
index dda7008acdd9..93e792d0c4a0 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
@@ -1377,6 +1377,30 @@ sdhi1_vqmmc: vqmmc-regulator {
};
};
+ du: display@920c0000 {
+ compatible = "renesas,r9a09g077-du";
+ reg = <0 0x920c0000 0 0x10000>;
+ interrupts = <GIC_SPI 781 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKAH>,
+ <&cpg CPG_MOD 1204>,
+ <&cpg CPG_CORE R9A09G077_LCDC_CLKD>;
+ clock-names = "aclk", "pclk", "vclk";
+ power-domains = <&cpg>;
+ renesas,vsps = <&vspd 0>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ du_out_rgb: endpoint {
+ };
+ };
+ };
+ };
+
fcpvd: fcp@920d0000 {
compatible = "renesas,r9a09g077-fcpvd", "renesas,fcpv";
reg = <0 0x920d0000 0 0x10000>;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/5] arm64: dts: renesas: r9a09g077: Add VSPD and FCPVD nodes
From: Prabhakar @ 2026-06-15 11:54 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-renesas-soc, devicetree, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615115455.1412098-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add VSPD and FCPVD nodes to RZ/T2H SoC DTSI.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No change
---
arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
index 40494159831d..dda7008acdd9 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
@@ -1376,6 +1376,28 @@ sdhi1_vqmmc: vqmmc-regulator {
status = "disabled";
};
};
+
+ fcpvd: fcp@920d0000 {
+ compatible = "renesas,r9a09g077-fcpvd", "renesas,fcpv";
+ reg = <0 0x920d0000 0 0x10000>;
+ clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKAH>,
+ <&cpg CPG_MOD 1204>,
+ <&cpg CPG_CORE R9A09G077_LCDC_CLKD>;
+ clock-names = "aclk", "pclk", "vclk";
+ power-domains = <&cpg>;
+ };
+
+ vspd: vsp@920e0000 {
+ compatible = "renesas,r9a09g077-vsp2", "renesas,r9a07g044-vsp2";
+ reg = <0 0x920e0000 0 0x8000>;
+ interrupts = <GIC_SPI 780 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKAH>,
+ <&cpg CPG_MOD 1204>,
+ <&cpg CPG_CORE R9A09G077_LCDC_CLKD>;
+ clock-names = "aclk", "pclk", "vclk";
+ power-domains = <&cpg>;
+ renesas,fcp = <&fcpvd>;
+ };
};
stmmac_axi_setup: stmmac-axi-config {
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/5] Add DU, VSPD and FCPVD support for RZ/T2H and RZ/N2H SoCs
From: Prabhakar @ 2026-06-15 11:54 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-renesas-soc, devicetree, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi all,
This patch series adds support for the Display Unit (DU) on the RZ/T2H
(r9a09g077) and RZ/N2H (r9a09g087) SoCs. The DU is a key component of
the display pipeline, responsible for driving the display output.
The patches include:
- Adding VSPD and FCPVD nodes to the SoC DTSI files for both RZ/T2H
and RZ/N2H.
- Adding the DU node to the SoC DTSI files, including clock and
interrupt configurations.
- Adding DT overlay support for enabling the DU/LCDC pipeline on the
RZ/T2H and RZ/N2H evaluation kits when fitted with a CN15/CN20
ADV7513 HDMI transmitter.
Note,
- DU driver patches have been merged into-next.
- FCP/VSP patches have been posted separately and are in Laurent's tree.
https://gitlab.freedesktop.org/linux-media/users/pinchartl/-/tree/for-next/media/renesas?ref_type=heads
- Clock changes have been posted separately and are pending review.
https://lore.kernel.org/all/20260615104845.4122868-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
- Patches apply on top of renesas-devel/renesas-dts-for-v7.3 (e0c6913891b1) branch.
v1->v2:
- Fixed typo in makefile for ARCH_R9A09G087
- Explicitly disabled the LED7 and key-1 in the overlay
- Dropped comment about DSW5[3] in patch#5
- Updated commit message in patch#5
- Dropped internal RB tag in patch#3
Cheers,
Prabhakar
Lad Prabhakar (5):
arm64: dts: renesas: r9a09g077: Add VSPD and FCPVD nodes
arm64: dts: renesas: r9a09g077: Add DU node
arm64: dts: renesas: r9a09g087: Add VSPD and FCPVD nodes
arm64: dts: renesas: r9a09g087: Add DU node
arm64: dts: renesas: Add LCDC overlays for RZ/T2H and RZ/N2H EVKs with
ADV7513
arch/arm64/boot/dts/renesas/Makefile | 6 +++
arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 46 +++++++++++++++++
.../renesas/r9a09g077m44-evk-cn15-lcdc.dtso | 40 +++++++++++++++
arch/arm64/boot/dts/renesas/r9a09g087.dtsi | 46 +++++++++++++++++
.../renesas/r9a09g087m44-evk-cn20-lcdc.dtso | 50 +++++++++++++++++++
.../dts/renesas/r9a09g087m44-rzn2h-evk.dts | 12 +++--
.../dts/renesas/rzt2h-n2h-evk-du-adv7513.dtsi | 50 +++++++++++++++++++
7 files changed, 246 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/boot/dts/renesas/r9a09g077m44-evk-cn15-lcdc.dtso
create mode 100644 arch/arm64/boot/dts/renesas/r9a09g087m44-evk-cn20-lcdc.dtso
create mode 100644 arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-du-adv7513.dtsi
--
2.54.0
^ permalink raw reply
* [PATCH v2] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
From: Udaya Kiran Challa @ 2026-06-15 11:53 UTC (permalink / raw)
To: tsbogend, robh, krzk+dt, conor+dt
Cc: skhan, me, linux-rtc, devicetree, linux-kernel,
Udaya Kiran Challa
Convert Microchip PIC32 SPI controller devicetree binding
from legacy text format to DT schema.
Signed-off-by: Udaya Kiran Challa <challauday369@gmail.com>
---
Changelog:
Changes since v1:
- Rename schema file to microchip,pic32mzda-spi.yaml
- Update subject prefix to match SPI DT binding conventions
Link to v1: https://lore.kernel.org/all/20260614175005.435826-1-challauday369@gmail.com/
---
.../bindings/spi/microchip,pic32mzda-spi.yaml | 78 +++++++++++++++++++
.../bindings/spi/microchip,spi-pic32.txt | 34 --------
2 files changed, 78 insertions(+), 34 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
diff --git a/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
new file mode 100644
index 000000000000..a0a182cdccbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/microchip,pic32mzda-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PIC32MZDA SPI Controller
+
+maintainers:
+ - Thomas Bogendoerfer <tsbogend@alpha.franken.de>
+
+allOf:
+ - $ref: spi-controller.yaml#
+
+properties:
+ compatible:
+ const: microchip,pic32mzda-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: Fault interrupt
+ - description: Receive interrupt
+ - description: Transmit interrupt
+
+ interrupt-names:
+ items:
+ - const: fault
+ - const: rx
+ - const: tx
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: mck0
+
+ dmas:
+ items:
+ - description: RX DMA channel
+ - description: TX DMA channel
+
+ dma-names:
+ items:
+ - const: spi-rx
+ - const: spi-tx
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi@1f821000 {
+ compatible = "microchip,pic32mzda-spi";
+ reg = <0x1f821000 0x200>;
+ interrupts = <109 IRQ_TYPE_LEVEL_HIGH>,
+ <110 IRQ_TYPE_LEVEL_HIGH>,
+ <111 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fault", "rx", "tx";
+ clocks = <&PBCLK2>;
+ clock-names = "mck0";
+ cs-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+ dmas = <&dma 134>, <&dma 135>;
+ dma-names = "spi-rx", "spi-tx";
+ };
diff --git a/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt b/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
deleted file mode 100644
index 79de379f4dc0..000000000000
--- a/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-Microchip PIC32 SPI Master controller
-
-Required properties:
-- compatible: Should be "microchip,pic32mzda-spi".
-- reg: Address and length of register space for the device.
-- interrupts: Should contain all three spi interrupts in sequence
- of <fault-irq>, <receive-irq>, <transmit-irq>.
-- interrupt-names: Should be "fault", "rx", "tx" in order.
-- clocks: Phandle of the clock generating SPI clock on the bus.
-- clock-names: Should be "mck0".
-- cs-gpios: Specifies the gpio pins to be used for chipselects.
- See: Documentation/devicetree/bindings/spi/spi-bus.txt
-
-Optional properties:
-- dmas: Two or more DMA channel specifiers following the convention outlined
- in Documentation/devicetree/bindings/dma/dma.txt
-- dma-names: Names for the dma channels. There must be at least one channel
- named "spi-tx" for transmit and named "spi-rx" for receive.
-
-Example:
-
-spi1: spi@1f821000 {
- compatible = "microchip,pic32mzda-spi";
- reg = <0x1f821000 0x200>;
- interrupts = <109 IRQ_TYPE_LEVEL_HIGH>,
- <110 IRQ_TYPE_LEVEL_HIGH>,
- <111 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "fault", "rx", "tx";
- clocks = <&PBCLK2>;
- clock-names = "mck0";
- cs-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
- dmas = <&dma 134>, <&dma 135>;
- dma-names = "spi-rx", "spi-tx";
-};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 3/3] arm64: dts: qcom: monaco-arduino-monza: add ina232 power sensor
From: Konrad Dybcio @ 2026-06-15 11:51 UTC (permalink / raw)
To: Loic Poulain, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-hwmon, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <20260611-monza-ina232-v2-3-e4375ce652d0@oss.qualcomm.com>
On 6/11/26 4:05 PM, Loic Poulain wrote:
> The Arduino Monza board has a TI INA232 current/power monitor connected
> on I2C12 at address 0x40. It is used to measure the board supply current
> through a 2 milliohms shunt resistor.
>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v9 1/6] dt-bindings: embedded-controller: Add Qualcomm reference device EC description
From: Anvesh Jain P @ 2026-06-15 11:49 UTC (permalink / raw)
To: Rob Herring
Cc: Sibi Sankar, Krzysztof Kozlowski, Conor Dooley, Hans de Goede,
Ilpo Järvinen, Bryan O'Donoghue, Bjorn Andersson,
Konrad Dybcio, Randy Dunlap, linux-arm-msm, devicetree,
linux-kernel, platform-driver-x86, Maya Matuszczyk,
Krzysztof Kozlowski
In-Reply-To: <20260612222259.GA1960900-robh@kernel.org>
On 6/13/2026 3:52 AM, Rob Herring wrote:
> Looks like this was missed, so I applied it. Please make sure your
> patches are applied. This should have been applied with the driver or
> .dts files.
>
> Rob
Thanks Rob for picking this up.
--
Best Regards,
Anvesh
^ permalink raw reply
* Re: [PATCH] of: property: Fix of_fwnode_get_reference_args() with negative index
From: Alban Bedel @ 2026-06-15 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, Rob Herring, Saravana Kannan, driver-core,
linux-kernel, Tommaso Merciai, Alban Bedel
In-Reply-To: <f21486ea-db76-4b71-9b68-86c861b0be53@kernel.org>
On Mon, 15 Jun 2026 11:54:01 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 15/06/2026 11:33, Alban Bedel wrote:
> > On Mon, 15 Jun 2026 08:33:32 +0200
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> On Thu, Jun 11, 2026 at 12:28:06PM +0200, Alban Bedel wrote:
> >>> fwnode_property_get_reference_args() should return -ENOENT when an out
> >>> of bound index is passed. An issue arised with the OF backend because
> >>> the OF API use signed indexes while the fwnode API use unsigned ones.
> >>> When an index value greater the INT_MAX was passed to the OF backend
> >>> it got casted to a negative value and it returned -EINVAL instead of
> >>
> >> INT_MAX is not out of bound for this function. It is invalid value,
> >> because OF code expects signed.
> >
> > But this is fwnode code, it use the OF API but it should implement the
> > fwnode API which, unlike the OF API, use unsigned index.
> >>> -ENOENT. This patch add a check to of_fwnode_get_reference_args() to
> >>> catch negative index before they are passed to the OF API and return
> >>> -ENOENT right away.
> >>
> >> I do not understand why are you fixing this issue that way. For this
> >> API, the INT_MAX is correct value, but you claim that it is wrong and
> >> should be ENOENT (even if there is entry).
> >>
> >> Fine, if this is not a correct value, then EINVAL.
> >
> > Indices larger than INT_MAX are valid in the fwnode API, so returning
> > -EINVAL is not appropriate here.
> >
>
> Then neither ENOENT are.
>
> But really, EINVAL is correct here. This is OF implementation, so this
> implementation decides what is EINVAL and what is right. Not fwnode API.
I think there is a missunderstanding here. The function we are talking
about, of_fwnode_get_reference_args(), is the OF backed implementation
of fwnode_property_get_reference_args(). As such it must follow the API
documented by fwnode_property_get_reference_args() which list the
following return values:
* Return: %0 on success
* %-ENOENT when the index is out of bounds, the index has an empty
* reference or the property was not found
* %-EINVAL on parse error
* %-ENOTCONN when the remote firmware node exists but has not been
* registered yet
It is not explicitly documented what should be returned if the index is
not representable in the backend, but considering it out of bound seems
to be the most sensible thing to do.
Alban
^ permalink raw reply
* Re: [PATCH 2/2] spi: qcom-geni: Add property to force GSI mode
From: Konrad Dybcio @ 2026-06-15 11:42 UTC (permalink / raw)
To: Pengyu Luo, Dmitry Baryshkov, Mukesh Kumar Savaliya
Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andy Gross, Bjorn Andersson, linux-arm-msm, linux-spi, devicetree,
linux-kernel
In-Reply-To: <CAH2e8h5Swn2mWoz+t_nrSw1=hQttwi1Hx=dg=cd9S3QRnHjpSQ@mail.gmail.com>
On 6/15/26 6:25 AM, Pengyu Luo wrote:
> On Mon, Jun 15, 2026 at 5:18 AM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>
>> On Sun, Jun 14, 2026 at 04:34:24PM +0800, Pengyu Luo wrote:
>>> Some devices (such as gaokun3) do not disable FIFO mode, causing the
>>> driver to fallback to FIFO mode by default. However, these platforms
>>> also support GSI mode, which is highly preferred for certain
>>> peripherals like SPI touchscreens to improve performance.
>>>
>>> Introduce the "qcom,force-gsi-mode" device property to hint and force
>>> the controller into GSI mode during initialization.
>>
>> Ideally, this should be decided by the SPI controller based on the
>> requirements. Another option would be to prefer GSI for all transfers if
>> it is available, ignoring the FIFO even if it is not disabled.
>>
>
> I have read reviews. Can we check if the compatible
> "qcom,sm6350-gpi-dma" is in DT? If the gpi dma controller is
> presented, then we enable GSI mode.
This hunk is very odd:
/*
* in case of failure to get gpi dma channel, we can still do the
* FIFO mode, so fallthrough
*/
dev_warn(mas->dev, "FIFO mode disabled, but couldn't get DMA, fall back to FIFO mode\n");
In my understanding, GSI DMA mode is always preferable. +Mukesh, do
you have any insights?
Konrad
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
From: me @ 2026-06-15 11:20 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Conor Dooley, github.com, dlechner, nuno.sa, andy, robh, krzk+dt,
conor+dt, tomasborquez13, masneyb, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260614205131.7449d180@jic23-huawei>
On 2026-06-14 21:51, Jonathan Cameron wrote:
> On Mon, 8 Jun 2026 18:49:13 +0100
> Conor Dooley <conor@kernel.org> wrote:
>
>> On Fri, Jun 05, 2026 at 09:28:03PM +0200, me@herrie.org wrote:
>> > On 2026-06-05 15:18, me@herrie.org wrote:
>> > > On 2026-06-05 15:04, Jonathan Cameron wrote:
>> > > > On Thu, 4 Jun 2026 18:01:08 +0100
>> > > > Conor Dooley <conor@kernel.org> wrote:
>> > > >
>> > > > > On Thu, Jun 04, 2026 at 12:06:16PM +0200, Herman van Hazendonk wrote:
>> > > > > > Document the new optional property that seeds the ISL29018 calibration
>> > > > > > scale factor at boot from firmware, allowing boards with tinted cover
>> > > > > > glass to ship with correct luminance readings without a userspace helper.
>> > > > > >
>> > > > > > The value is a positive integer (minimum 1, maximum 65535) that is
>> > > > > > multiplied with the raw lux reading. Userspace can still override it
>> > > > > > at runtime through in_illuminance0_calibscale.
>> > > > > >
>> > > > > > Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
>> > > > > > ---
>> > > > > > .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++
>> > > > > > 1 file changed, 13 insertions(+)
>> > > > > >
>> > > > > > diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.yaml b/Documentation/devicetree/bindings/iio/light/isl29018.yaml
>> > > > > > index 0ea278b07d1c..92ea2742bbd3 100644
>> > > > > > --- a/Documentation/devicetree/bindings/iio/light/isl29018.yaml
>> > > > > > +++ b/Documentation/devicetree/bindings/iio/light/isl29018.yaml
>> > > > > > @@ -34,6 +34,19 @@ properties:
>> > > > > > vcc-supply:
>> > > > > > description: Regulator that provides power to the sensor
>> > > > > >
>> > > > > > + isil,cover-comp-gain:
>> > > > > > + description: |
>> > > > > > + Multiplier applied to the ambient-light reading at startup to
>> > > > > > + compensate for optical loss in the board's cover glass. Boards
>> > > > > > + that mount the sensor under a tinted or coated window typically
>> > > > > > + need a value between a few and a few hundred.
>> > > >
>> > > > Is it useful to support decimal points on these values? The
>> > > > userspace interface
>> > > > does and you mention the 'right' answer might be only a few which
>> > > > means precision
>> > > > at that range will be terrible - less of an issue if 100s!
>> > > >
>> > > > Thanks
>> > > >
>> > > > Jonathan
>> > > >
>> > > Hard to say, my old HP TouchPad needs 100 as a value here (taken from
>> > > legacy 2.6.35
>> > > kernel and binaries). So we probably don't need precision, but I have no
>> > > other
>> > > references to substantiate.
>> > >
>> > > Thanks
>> > > Herman
>> > Scratch that. Did some more research. Proof is in the legacy webOS binaries:
>> >
>> > What legacy webOS actually does here:
>> >
>> > 1. Per-device calibration via factory token reads an ALSCal token from
>> > system storage
>> > containing JSON with calibration points at lux_0, lux_50, lux_100, lux_400
>> > (measured
>> > ADC counts at known reference illuminance levels).
>> >
>> > 2. Computes a floating-point ratio AlsToLux_Ratio_WhiteLED = average of
>> > expected_lux /
>> > measured_count across the JSON calibration points. This is a real number,
>> > not an integer.
>> >
>> > 3. Adjusts for light source spectrum at runtime detects illuminant type from
>> > ALS:IR
>> > ratio, then applies a fractional spectrum correction:
>> > Fluorescent above 100 counts: ratio x 0.4652
>> > Incandescent above 700 counts: ratio x 0.4
>> > Incandescent 50-100 counts: ratio x 0.9
>> > Fluorescent < 50 counts: ratio x (-0.000724·N + 0.7463)
>> >
>> > 4. Final lux = ALSCount / spectrum_corrected_ratio - a true floating-point
>> > division.
>> >
>> > Implications
>> >
>> > - The "right" cover-comp value is per-device factory-measured, not
>> > per-board. Different
>> > units off the same production line have different optical transmission due
>> > to coating
>> > tolerance.
>> > - The values are fractional by nature. Examples from the legacy code:
>> > 0.4652, 0.7463,
>> > 0.8333. None are integers.
>> >
>> > The ISL29023 datasheet itself says nothing about cover compensation - it's
>> > strictly
>> > board-level optical correction. So there's no "right" answer from the chip
>> > side;
>> > it's whatever the board's cover glass + coating attenuates.
>> >
>> > ALSCal values found on the particular device:
>> >
>> > {"lux_50":{"c":31}, "lux_100":{"c":58}, "lux_400":{"c":164}}
>> >
>> > This is device specific TouchPad's factory calibration:
>> > - At 50 lux, ADC reads 31 counts lux/count = 1.613
>> > - At 100 lux, ADC reads 58 counts lux/count = 1.724
>> > - At 400 lux, ADC reads 164 counts lux/count = 2.439
>> >
>> > Note the ratio isn't constant - the response is mildly non-linear, but per
>> > the legacy
>> > code the driver computes the average ratio as the calibration:
>> >
>> > ratio = (1.6129 + 1.7241 + 2.4390) / 3 = 1.9253 lux/count
>> >
>> > Independent verification:
>> > - At calibscale=34: lux = 1295
>> > - Implied raw count: 1295 / (34 x 0.015258) = 2496 counts
>> > - Applying legacy formula: 2496 / 1.9253 = 1296.4 = 1295
>> >
>> > The factory-calibrated value for this specific TouchPad is 34.04 (not 100).
>> > Per-point
>> > calibscale values from the ALSCal JSON:
>> >
>> > Cal point‚ lux/count ratio‚ Equivalent calibscale
>> > lux_50 ‚ 1.6129 ‚ 40.64
>> > lux_100 ‚ 1.7241 ‚ 38.01
>> > lux_400 ‚ 2.4390 ‚ 26.87
>> > average ‚ 1.9253 ‚ 34.04
>> >
>> > What this means concretely
>> >
>> > 1. Decimal precision is necessary, not nice-to-have. Real per-device factory
>> > values
>> > span 26.9 - 40.6 across the chip's response curve. A single scalar
>> > approximation costs
>> > precision; restricting to integer compounds it.
>> >
>> > 2. Updated v5 plan: switch to two-cell <int micro> for fractional values,
>> > and change
>> > the tenderloin DTS default from <100> to <34 040000> (or close to that).
>> >
>> > Thoughts on this?
>>
>> Instead of going 2-cell, it might be worth moving the property to be
>> based in some fractional unit to begin with. I just don't know what to
>> call that, milligain? I dunno.
>>
>>
>> Jonathan's call here I think
>
> If this is varying that a lot per device, then, unless you plan to
> have a bootloader inject it into DT, are we better off making this
> a userspace problem? There is a hybrid we've done before, which is
> to make it a firmware loading problem but that requires some way to
> generate a suitable file to load and typically a calibration sequence
> that goes with it.
>
> If it is 'fairly constant' for a given device then fine to be in DT
> - it will just be a case of picking the average of a bunch of measured
> devices and assuming it is good enough.
>
> If it does end up in dt, then a fractional type like Conor suggests
> is fine.
>
> Jonathan
Hi Jonathan,
I have about 8-10 devices in slightly different configurations using
this I guess,
so I can quickly run the numbers and see how the values look like, so we
have more
data to go by.
Thanks,
Herman
^ permalink raw reply
* [PATCH] dt-bindings: arm: xen: Convert xen.txt to YAML schema
From: Tejas @ 2026-06-15 11:26 UTC (permalink / raw)
To: devicetree; +Cc: linux-kernel, robh, krzk+dt, conor+dt, sstabellini, Tejas
Convert the Xen ARM device tree binding documentation from the legacy
plain-text format (Documentation/devicetree/bindings/arm/xen.txt) to
the YAML schema format, as required by the modern DT binding process.
The old xen.txt is removed as the YAML schema is now the authoritative
source.
The YAML schema:
- Validates compatible string format as "xen,xen-<major>.<minor>"
followed by the generic "xen,xen" string
- Documents reg as accepting 1..N regions (region 0 mandatory for
grant table mapping; regions 1..N optional extended regions)
- Documents the uefi subnode with correct types:
* xen,uefi-system-table: uint64 (guest PA of UEFI System Table)
* xen,uefi-mmap-start: uint64 (guest PA of UEFI memory map)
* xen,uefi-mmap-size: uint32 (size of UEFI memory map)
* xen,uefi-mmap-desc-size: uint32 (size of each mmap entry)
* xen,uefi-mmap-desc-ver: uint32 (mmap descriptor format version)
- 64-bit properties use /bits/ 64 <value> in the example, consistent
with other bindings carrying uint64 properties (e.g. opp-v2.yaml,
arm/mali-bifrost.yaml)
The uefi subnode was originally introduced through a multi-version review
series (v2..v7); the mistakes caught during those reviews (typos,
duplicated UEFI spec content, insufficient description of Xen-specific
hypercall semantics) are avoided with deliberate caution here.
Note: the example emits a dtc warning (unit_address_vs_reg) for the
/hypervisor node. Both the normative text in xen.txt ("Xen ARM virtual
platforms shall have a top-level 'hypervisor' node") and the example
in xen.txt mandate this exact node name — the $nodename: const:
hypervisor in the schema is a direct encoding of that pre-existing
requirement. A unit address is therefore impossible despite the presence
of reg. This warning is pre-existing and not introduced by this
conversion.
Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
---
Documentation/devicetree/bindings/arm/xen.txt | 62 ----------
.../devicetree/bindings/arm/xen.yaml | 113 ++++++++++++++++++
2 files changed, 113 insertions(+), 62 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/xen.txt
create mode 100644 Documentation/devicetree/bindings/arm/xen.yaml
diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
deleted file mode 100644
index f925290d4641..000000000000
--- a/Documentation/devicetree/bindings/arm/xen.txt
+++ /dev/null
@@ -1,62 +0,0 @@
-* Xen hypervisor device tree bindings
-
-Xen ARM virtual platforms shall have a top-level "hypervisor" node with
-the following properties:
-
-- compatible:
- compatible = "xen,xen-<version>", "xen,xen";
- where <version> is the version of the Xen ABI of the platform.
-
-- reg: specifies the base physical address and size of the regions in memory
- where the special resources should be mapped to, using an HYPERVISOR_memory_op
- hypercall.
- Region 0 is reserved for mapping grant table, it must be always present.
- The memory region is large enough to map the whole grant table (it is larger
- or equal to gnttab_max_grant_frames()).
- Regions 1...N are extended regions (unused address space) for mapping foreign
- GFNs and grants, they might be absent if there is nothing to expose.
-
-- interrupts: the interrupt used by Xen to inject event notifications.
- A GIC node is also required.
-
-To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
-under /hypervisor with following parameters:
-
-________________________________________________________________________________
-Name | Size | Description
-================================================================================
-xen,uefi-system-table | 64-bit | Guest physical address of the UEFI System
- | | Table.
---------------------------------------------------------------------------------
-xen,uefi-mmap-start | 64-bit | Guest physical address of the UEFI memory
- | | map.
---------------------------------------------------------------------------------
-xen,uefi-mmap-size | 32-bit | Size in bytes of the UEFI memory map
- | | pointed to in previous entry.
---------------------------------------------------------------------------------
-xen,uefi-mmap-desc-size | 32-bit | Size in bytes of each entry in the UEFI
- | | memory map.
---------------------------------------------------------------------------------
-xen,uefi-mmap-desc-ver | 32-bit | Version of the mmap descriptor format.
---------------------------------------------------------------------------------
-
-Example (assuming #address-cells = <2> and #size-cells = <2>):
-
-hypervisor {
- compatible = "xen,xen-4.3", "xen,xen";
- reg = <0 0xb0000000 0 0x20000>;
- interrupts = <1 15 0xf08>;
- uefi {
- xen,uefi-system-table = <0xXXXXXXXX>;
- xen,uefi-mmap-start = <0xXXXXXXXX>;
- xen,uefi-mmap-size = <0xXXXXXXXX>;
- xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
- xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
- };
-};
-
-The format and meaning of the "xen,uefi-*" parameters are similar to those in
-Documentation/arch/arm/uefi.rst, which are provided by the regular UEFI stub. However
-they differ because they are provided by the Xen hypervisor, together with a set
-of UEFI runtime services implemented via hypercalls, see
-http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html.
diff --git a/Documentation/devicetree/bindings/arm/xen.yaml b/Documentation/devicetree/bindings/arm/xen.yaml
new file mode 100644
index 000000000000..3117f6d3828c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/xen.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/xen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xen hypervisor
+
+maintainers:
+ - Stefano Stabellini <sstabellini@kernel.org>
+
+description: |
+ Xen ARM virtual platforms shall have a top-level "hypervisor" node with
+ the properties defined below.
+
+properties:
+ $nodename:
+ const: hypervisor
+
+ compatible:
+ description: |
+ Specifies the Xen hypervisor. The version of the Xen ABI is encoded
+ in the first item as "xen,xen-<version>", followed by the generic
+ "xen,xen" string.
+ items:
+ - pattern: "^xen,xen-[0-9]+\\.[0-9]+$"
+ - const: xen,xen
+
+ reg:
+ description: |
+ Base physical address and size of the regions in memory where special
+ resources should be mapped to, using an HYPERVISOR_memory_op hypercall.
+
+ Region 0 is reserved for mapping the grant table and must always be
+ present. The memory region must be large enough to map the whole grant
+ table (it is larger or equal to gnttab_max_grant_frames()).
+
+ Regions 1...N are extended regions (unused address space) for mapping
+ foreign GFNs and grants. They might be absent if there is nothing to
+ expose.
+ minItems: 1
+
+ interrupts:
+ description: |
+ The interrupt used by Xen to inject event notifications.
+ A GIC node is also required.
+ maxItems: 1
+
+ uefi:
+ type: object
+ description: |
+ Node populated by Xen to support UEFI on Xen ARM virtual platforms.
+ The format and meaning of the "xen,uefi-*" parameters are similar to
+ those in Documentation/arch/arm/uefi.rst, but are provided by the Xen
+ hypervisor together with a set of UEFI runtime services implemented via
+ hypercalls.
+ properties:
+ xen,uefi-system-table:
+ description: Guest physical address of the UEFI System Table.
+ $ref: /schemas/types.yaml#/definitions/uint64
+
+ xen,uefi-mmap-start:
+ description: Guest physical address of the UEFI memory map.
+ $ref: /schemas/types.yaml#/definitions/uint64
+
+ xen,uefi-mmap-size:
+ description: Size in bytes of the UEFI memory map pointed to by xen,uefi-mmap-start.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xen,uefi-mmap-desc-size:
+ description: Size in bytes of each entry in the UEFI memory map.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xen,uefi-mmap-desc-ver:
+ description: Version of the mmap descriptor format.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ / {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ gic: interrupt-controller {
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ };
+
+ hypervisor {
+ compatible = "xen,xen-4.3", "xen,xen";
+ reg = <0 0xb0000000 0 0x20000>;
+ interrupt-parent = <&gic>;
+ interrupts = <1 15 0xf08>;
+
+ uefi {
+ xen,uefi-system-table = /bits/ 64 <0x1301415>;
+ xen,uefi-mmap-start = /bits/ 64 <0x7591400>;
+ xen,uefi-mmap-size = <0x1800>;
+ xen,uefi-mmap-desc-size = <0x30>;
+ xen,uefi-mmap-desc-ver = <1>;
+ };
+ };
+ };
+...
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v8 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
From: Krzysztof Kozlowski @ 2026-06-15 11:26 UTC (permalink / raw)
To: Can Guo, bvanassche, beanhuo, peter.wang, martin.petersen, mani
Cc: linux-scsi, Alim Akhtar, Avri Altman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ram Kumar Dwivedi,
Zhaoming Luo,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20260615085027.2102882-2-can.guo@oss.qualcomm.com>
On 15/06/2026 10:50, Can Guo wrote:
> UFS v5.0/UFSHCI v5.0 adds HS-G6 support (46.6 Gbps/lane) via UniPro
> v3.0 and M-PHY v6.0. These specs define TX Equalization for all
> High-Speed Gears (not only HS-G6) to compensate channel loss and
> improve signal integrity at high speed.
>
> For HS-G6, M-PHY uses PAM4 1b1b line coding. Pre-Coding may also be
> required depending on channel characteristics.
>
> Document vendor-neutral properties in ufs-common.yaml:
> - txeq-preshoot-g[1-6]
> - txeq-deemphasis-g[1-6]
> - tx-precode-enable-g6
>
> Values are per-lane Host/Device tuples (2 values for x1, 4 values for
> x2). PreShoot/DeEmphasis range from 0..7, and Precode is 0/1.
>
> These are board-specific signal-integrity tuning values. They depend on
> channel SI/PHY characterization and validation (host PHY, device PHY,
> package, and board routing), and are determined by HW/PHY designers.
>
> Although UFSHCI v5.0 supports TX Equalization Training via UniPro v3.0,
> which allows host software to determine optimal TX Equalization at
> runtime, static board-specific TX Equalization settings in the Device
> Tree are still necessary because:
> - TX Equalization Training is not supported for HS-G3 and below
> - TX Equalization Training is disabled on some platforms
>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---
> .../devicetree/bindings/ufs/ufs-common.yaml | 55 +++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> index ed97f5682509..145a6416e1df 100644
> --- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> +++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> @@ -105,6 +105,61 @@ properties:
> Restricts the UFS controller to rate-a or rate-b for both TX and
> RX directions.
>
> + tx-precode-enable-g6:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + oneOf:
> + - items:
> + - description: Host_Lane0 precode
> + - description: Device_Lane0 precode
> + - items:
> + - description: Host_Lane0 precode
> + - description: Device_Lane0 precode
> + - description: Host_Lane1 precode
> + - description: Device_Lane1 precode
> + items:
> + enum: [0, 1]
> + description:
> + Static TX Precode enable values for HS-G6 only.
My bad here, that was my mistake. I wanted matrix, but gave you array
syntax. Proper code would be:
tx-precode-enable-g6:
$ref: /schemas/types.yaml#/definitions/uint32-matrix
minItems: 1
items:
- items:
- description: Host_Lane0 precode
enum: [0, 1]
- description: Device_Lane0 precode
enum: [0, 1]
- items:
- description: Host_Lane1 precode
enum: [0, 1]
- description: Device_Lane1 precode
enum: [0, 1]
description:
Static TX Precode enable values for HS-G6 only.
I tested it with DTS and seems to work fine. Please use the same for
other properties, but enum will be 0-7.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 8/8] arm64: dts: freescale: imx95: Add NXP neoisp device tree node
From: Francesco Dolcini @ 2026-06-15 11:25 UTC (permalink / raw)
To: Antoine Bouyer
Cc: Francesco Dolcini, 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,
ai.luthra, paul.elder, geert, sakari.ailus, hverkuil+cisco
In-Reply-To: <761f284a-1660-41d5-9625-9b25bf18aca5@nxp.com>
On Mon, Jun 15, 2026 at 11:56:15AM +0200, Antoine Bouyer wrote:
> On 6/14/26 11:05 AM, Francesco Dolcini wrote:
> > On Fri, Jun 12, 2026 at 03:20:39PM +0200, Antoine Bouyer wrote:
> > > Add neoisp device tree node to imx95.dtsi and enable it by default in
> > > 19x19 evk board.
> > >
> > > Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> >
> > ...
> >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > index d6c549c16047..5543a6cb1250 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > @@ -1867,6 +1867,17 @@ pmu@49252000 {
> > > };
> > > };
> > >
> > > + neoisp0: isp@4ae00000 {
> > > + compatible = "nxp,imx95-neoisp";
> > > + reg = <0x0 0x4ae00000 0x0 0x8000>,
> > > + <0x0 0x4afe0000 0x0 0x10000>;
> > > + interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&scmi_clk IMX95_CLK_CAMCM0>;
> > > + clock-names = "camcm0";
> > > + power-domains = <&scmi_devpd IMX95_PD_CAMERA>;
> > > + status = "disabled";
> > > + };
> >
> > Why the node is disabled? If the node is wholly described in
> > imx95.dtsi, it should be enabled.
>
> Actually, all nodes are disabled in the SoC dtsi, and enabled on the board
> dts file, even if fully described on the dtsi. So I used same approach for
> neoisp.
This is not correct. Please check what we do for the GPU/VPU[1] and NPU [2],
for example. Is there a reason to do it differently for the ISP?
Francesco
[1] arch/arm64/boot/dts/freescale/imx8mm.dtsi
[2] arch/arm64/boot/dts/freescale/imx93.dtsi
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: iio: st,st-sensors: add st,fullscale-mg
From: me @ 2026-06-15 11:16 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Herman van Hazendonk, linusw, denis.ciocca, robh, krzk+dt,
conor+dt, dlechner, nuno.sa, andy, sanjayembeddedse,
maudspierings, linux-iio, devicetree, linux-kernel
In-Reply-To: <20260614194442.61191f68@jic23-huawei>
On 2026-06-14 20:44, Jonathan Cameron wrote:
> On Fri, 5 Jun 2026 12:08:42 +0200
> Herman van Hazendonk <github.com@herrie.org> wrote:
>
>> Add an optional st,fullscale-mg property that selects the initial
>> full-scale range of an ST MEMS sensor at probe time, expressed in
>> milligauss for magnetometers (and analogous engineering units for
>> other ST sensor families that may grow this property in the future).
>>
>> The property is purely additive: if absent, drivers fall back to
>> their existing chip default, and if present but unsupported by the
>> specific sensor the driver warns and falls back. No existing in-tree
>> DTS is affected.
>>
>> The motivating case is the LSM303DLH magnetometer on the HP TouchPad
>> (apq8060 / tenderloin) where the kernel's chip-default +/-1.3 G range
>> saturates the X axis to the chip's 0xF000 overflow sentinel out of
>> probe, because the chip is mounted close to surrounding power planes
>> and picks up enough DC bias to exceed the smallest range. The driver
>> core hardcodes fs_avl[0] as the starting range, so userspace cannot
>> recover without racing the driver to write the in_magn_x_scale sysfs
>> attribute after probe. st,fullscale-mg lets the device tree declare
>> a wider initial range up-front and avoids the race entirely.
>
> I'm trying to understand what you mean here by racing.
>
> If we get this overflow condition the chip is wedged until reset, or
> userspace simply has to change the range to recover?
>
> I'm wondering if a UDEV rule is sufficient in theory to fix this.
>
> I'm not necessarily against having the range in DT as it is effectively
> hardware dependent but just want to make sure I fully understand the
> issue.
>
> Jonathan
>
>
>
>>
>> Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
>> ---
>> .../devicetree/bindings/iio/st,st-sensors.yaml | 18
>> ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
>> b/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
>> index a1a958215cdb..335f38e9f78f 100644
>> --- a/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
>> +++ b/Documentation/devicetree/bindings/iio/st,st-sensors.yaml
>> @@ -126,6 +126,24 @@ properties:
>> mount-matrix:
>> description: an optional 3x3 mounting rotation matrix.
>>
>> + st,fullscale-mg:
>> + description: |
>> + Selects the initial sensor full-scale at probe time, expressed
>> in
>> + milligauss for magnetometers (or analogous engineering units
>> for
>> + other sensor families that may grow this property in the
>> future).
>> + The value must match one of the sensor-specific full-scale
>> ranges
>> + supported by the chip; if the chip does not support the
>> requested
>> + range the driver falls back to its built-in default.
>> +
>> + This is intended for boards where the magnetometer chip picks
>> up
>> + enough DC bias from nearby PCB structures (power planes,
>> ferrous
>> + shields, etc.) that the kernel's chip-default
>> highest-sensitivity
>> + range saturates one or more axes to the chip's overflow
>> sentinel,
>> + and userspace observes that axis as permanently stuck.
>> Declaring
>> + a wider initial range avoids the saturation at the cost of a
>> + slightly coarser quantisation.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> allOf:
>> - if:
>> properties:
Hi Jonathan,
"racing" was loose wording on my part. The chip is not wedged. Once
userspace writes a wider range to in_magn_x_scale,
the next conversion comes back with sensible data and everything works.
A UDEV rule on add of the IIO device would, in principle, fix
the steady-state problem.
What I was clumsily pointing at is the probe-time window: the IIO
consumers in our stack (sensorfw's iio-sensors-adaptor, geomagnetic
/ orientation services) start polling in_magn_x_raw essentially the
moment the device node appears, and they treat the saturated
0xF000 sentinel as a legitimate sample rather than as overflow. Until
the UDEV rule fires and the kernel commits the new range, every
read returns the stuck sentinel, so orientation/compass features are
wrong from boot for some non-trivial number of samples (and on
slow-boot paths the consumer may have already cached a bogus calibration
baseline by the time UDEV catches up).
So the trade-off as I see it is:
- UDEV rule: works for steady state, fully out-of-tree, but the
per-board configuration ends up split between two places (DTS for
"this board has an LSM303DLH at i2c@... in this orientation"; UDEV for
"and by the way it needs a wider range or its raw readings are
nonsense"). The wider-range requirement is purely a property of how the
magnetometer is mounted on the board, which is what DT is for.
- st,fullscale-mg in DT: keeps all hardware-dependent calibration in one
place, available before any IIO consumer can open the device,
and harmless on boards that don't need it (absent property existing
chip-default behaviour, no DTS in tree changes).
I'm happy to drop the wording about "racing" in v2 and replace it with a
more accurate description of the early-consumer issue if that
reads better. And of course if you'd rather we ship a UDEV rule
downstream instead of adding a binding, I'll yield, but my read is that
this is hardware-dependent enough to belong in DT.
Thanks,
Herman
^ permalink raw reply
* Re: [PATCH v3 2/2] clk: amlogic: Add A9 peripherals clock controller driver
From: Jian Hu @ 2026-06-15 11:25 UTC (permalink / raw)
To: Jerome Brunet, Jian Hu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <1jecieftme.fsf@starbuckisacylon.baylibre.com>
On 6/10/2026 8:49 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mer. 10 juin 2026 at 16:14, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the peripherals clock controller driver for the Amlogic A9 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>> drivers/clk/meson/Kconfig | 15 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/a9-peripherals.c | 1925 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1941 insertions(+)
>>
[ ... ]
>> +
>> +/* Channel 6 is unconnected. */
>> +static u32 a9_glb_parents_val_table[] = { 0, 1, 2, 3, 4, 5, 7 };
>> +static struct clk_regmap a9_dspa;
> What is this ?
The peripheral clock definitions are ordered by register offset.
dspa is one of the parents of the glb clock, while the dsp clock
registers are located after the GLB clock registers.
Since glb references a9_dspa before its full definition appears, the
declaration
static struct clk_regmap a9_dspa;
is added as a forward declaration to satisfy the compiler.
Would it make sense to relax the register-offset ordering in this case?
By defining the DSP clock before the GLB clock, we could remove the
forward declaration of a9_dspa.
>> +
>> +static const struct clk_parent_data a9_glb_parents[] = {
>> + { .fw_name = "xtal", },
>> + { .hw = &a9_dspa.hw },
>> + { .fw_name = "fdiv3", },
>> + { .fw_name = "fdiv4", },
>> + { .fw_name = "fdiv5", },
>> + { .hw = &a9_isp.hw },
>> + { .fw_name = "rtc", }
>> +};
>> +
>> +static A9_COMP_SEL(glb, GLB_CLK_CTRL, 9, 0x7, a9_glb_parents,
>> + a9_glb_parents_val_table);
>> +static A9_COMP_DIV(glb, GLB_CLK_CTRL, 0, 7);
>> +static A9_COMP_GATE(glb, GLB_CLK_CTRL, 8, 0);
>> +
>> +static struct clk_regmap a9_usb_48m_dualdiv_in = {
>> + .data = &(struct clk_regmap_gate_data) {
>> + .offset = USB_CLK_CTRL,
>> + .bit_idx = 31,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("usb_48m_dualdiv_in", &a9_usb_48m_pre.hw,
>> + &clk_regmap_gate_ops, 0),
> Same comment as on the AO controller
Ok, I will drop CLK_HW_INIT*, and apply the same change to the other
clock definitions.
[ ... ]
>> +
>> +/* Channel 3 is unconnected. */
> You meant 3rd I guess but this is misleading and confusing with the
> table bellow. Channel 2 would be more appropriate I think, since those
> are 0-based.
You are right, The 3rd channel is unconnected. I will fix it in the
next version.
>> +static u32 a9_can_pe_parents_val_table[] = { 0, 1, 3 };
>> +static const struct clk_parent_data a9_can_pe_parents[] = {
>> + { .fw_name = "sys", },
>> + { .fw_name = "xtal", },
>> + { .fw_name = "fdiv5", }
>> +};
>> +
[ ... ]
>> +
>> +/*
>> + * Channel 3(ddr_dpll_pt_clk) is manged by the DDR module;
>> + * channel 12(msr_clk) is manged by clock measures module.
>> + * channel 16(audio_dac1_clk) is manged by audio module.
> Some why can't you expose those then ? gen clk is used for debugging
> AFAIK. The clock above are worth debugging I think
>
> Please be consistent with the CaSing.
For channel 3 (ddr_dpll_pt_clk), it is sourced from the DDR PLL clock
controller rather than
this clock controller. I will expose it in the DT.
For channel 12 (msr_clk), it depends on the clock measurement
configuration.
The measurement source must first be selected through freq_ctrl[20:26], and
only then does msr_clk become meaningful. In practice.
It is not a real clock but an output of the measurement logic, so it can
not be exposed as a clock.
https://elixir.bootlin.com/linux/v7.1-rc7/source/drivers/soc/amlogic/meson-clk-measure.c#L809
For channel 16 (audio_dac1_clk), I confirmed with the clock hardware
designer that
it is actually unconnected. It looks like there is an error in the
documentation.
I'll also fix the casing to keep it consistent throughout the comments.
>> + * Channel 10, 11, 13, 14 are not connected.
>> + */
>> +static u32 a9_gen_parents_val_table[] = { 0, 1, 2, 4, 5, 6, 7, 8, 9, 15, 17, 18,
>> + 19, 20, 21, 22, 23, 24, 25, 26};
>> +static struct clk_regmap a9_vid_pll;
>> +
>> +static const struct clk_parent_data a9_gen_parents[] = {
>> + { .fw_name = "xtal" },
>> + { .fw_name = "rtc" },
>> + { .fw_name = "sysplldiv16" },
>> + { .hw = &a9_vid_pll.hw },
>> + { .fw_name = "gp0" },
>> + { .fw_name = "hifi1" },
>> + { .fw_name = "hifi0" },
>> + { .fw_name = "gp1" },
>> + { .fw_name = "gp2" },
>> + { .fw_name = "dsudiv16" },
>> + { .fw_name = "cpudiv16" },
>> + { .fw_name = "a78div16" },
>> + { .fw_name = "fdiv2" },
>> + { .fw_name = "fdiv2p5" },
>> + { .fw_name = "fdiv3" },
>> + { .fw_name = "fdiv4" },
>> + { .fw_name = "fdiv5" },
>> + { .fw_name = "fdiv7" },
>> + { .fw_name = "mclk0" },
>> + { .fw_name = "mclk1" }
>> +};
>> +
>> +static A9_COMP_SEL(gen, GEN_CLK_CTRL, 12, 0x1f, a9_gen_parents,
>> + a9_gen_parents_val_table);
>> +static A9_COMP_DIV(gen, GEN_CLK_CTRL, 0, 11);
>> +static A9_COMP_GATE(gen, GEN_CLK_CTRL, 11, 0);
>> +
[ ... ]
>> +
>> +static struct clk_regmap a9_enc, a9_enc1;
> What is this again ?? and please come up with better names.
Same as the previous dspa clock declaration.
enc stands for encoder. I'll rename a9_enc to a9_encoder0 and a9_enc1 to
a9_encoder1 in the next version.
>> +
>> +static const struct clk_parent_data a9_vid_lock_parents[] = {
>> + { .fw_name = "xtal", },
>> + { .hw = &a9_enc.hw },
>> + { .hw = &a9_enc1.hw }
>> +};
>> +
>> +static A9_COMP_SEL(vid_lock, VID_LOCK_CLK_CTRL, 9, 0x7, a9_vid_lock_parents,
>> + NULL);
>> +static A9_COMP_DIV(vid_lock, VID_LOCK_CLK_CTRL, 0, 7);
>> +static A9_COMP_GATE(vid_lock, VID_LOCK_CLK_CTRL, 8, 0);
>> +
>> +static const struct clk_parent_data a9_vdin_meas_parents[] = {
>> + { .fw_name = "xtal", },
>> + { .fw_name = "fdiv4", },
>> + { .fw_name = "fdiv3", },
>> + { .fw_name = "fdiv5", }
>> +};
>> +
>> +static A9_COMP_SEL(vdin_meas, VDIN_MEAS_CLK_CTRL, 9, 0x7, a9_vdin_meas_parents,
>> + NULL);
>> +static A9_COMP_DIV(vdin_meas, VDIN_MEAS_CLK_CTRL, 0, 7);
>> +static A9_COMP_GATE(vdin_meas, VDIN_MEAS_CLK_CTRL, 8, 0);
>> +
>> +static struct clk_regmap a9_vid_pll_div = {
>> + .data = &(struct meson_vid_pll_div_data){
>> + .val = {
>> + .reg_off = VID_PLL_CLK_DIV,
>> + .shift = 0,
>> + .width = 15,
>> + },
>> + .sel = {
>> + .reg_off = VID_PLL_CLK_DIV,
>> + .shift = 16,
>> + .width = 2,
>> + },
>> + },
>> + .hw.init = CLK_HW_INIT_FW_NAME("vid_pll_div", "hdmiout2",
>> + &meson_vid_pll_div_ro_ops, 0),
>> +};
>> +
>> +static struct clk_regmap a9_vid_pll_sel = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = VID_PLL_CLK_DIV,
>> + .mask = 0x1,
>> + .shift = 18,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_DATA("vid_pll_sel",
>> + ((const struct clk_parent_data []) {
>> + { .hw = &a9_vid_pll_div.hw },
>> + { .fw_name = "hdmiout2" }
>> + }), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vid_pll = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_PLL_CLK_DIV,
>> + .bit_idx = 19,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vid_pll", &a9_vid_pll_sel.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vid_pll_vclk = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = HDMI_CLK_CTRL,
>> + .mask = 0x1,
>> + .shift = 15,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_DATA("vid_pll_vclk",
>> + ((const struct clk_parent_data []) {
>> + { .hw = &a9_vid_pll.hw },
>> + { .fw_name = "hdmipix" }
>> + }), &clk_regmap_mux_ops, 0),
>> +};
>> +
>> +static const struct clk_parent_data a9_vclk_parents[] = {
>> + { .hw = &a9_vid_pll_vclk.hw },
>> + { .fw_name = "pix0", },
>> + { .fw_name = "vid1", },
>> + { .fw_name = "pix1", },
>> + { .fw_name = "fdiv3", },
>> + { .fw_name = "fdiv4", },
>> + { .fw_name = "fdiv5", },
>> + { .fw_name = "vid2", }
>> +};
>> +
>> +static struct clk_regmap a9_vclk_sel = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = VID_CLK_CTRL,
>> + .mask = 0x7,
>> + .shift = 16,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_DATA("vclk_sel", a9_vclk_parents,
>> + &clk_regmap_mux_ops, 0),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_in = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_DIV,
>> + .bit_idx = 16,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_in", &a9_vclk_sel.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = VID_CLK_DIV,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div", &a9_vclk_in.hw,
>> + &clk_regmap_divider_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 19,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk", &a9_vclk_div.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div1_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 0,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div1_en", &a9_vclk.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div2_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 1,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div2_en", &a9_vclk.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
> Looks to me all this div_en / div repeating pattern would be easier to review
> with tiny macro .
Good point.
I tried to reduce the repeated div_en/div pattern using a helper macro.
It keeps the relationship between gate and fixed-factor clock more
compact and easier to review.
After using the helper macro, the div_en/div code can be simplified to
the following:
#define A9_VCLK(_name, _reg, _bit, _div, _parent) \
struct clk_regmap a9_##_name##_en = { \
.data = &(struct clk_regmap_gate_data){ \
.offset = _reg, \
.bit_idx = _bit, \
}, \
.hw.init = &(struct clk_init_data) { \
.name = #_name "_en", \
.ops = &clk_regmap_gate_ops, \
.parent_hws = (const struct clk_hw *[]) { _parent }, \
.num_parents = 1, \
.flags = CLK_SET_RATE_PARENT, \
}, \
}; \
\
struct clk_fixed_factor a9_##_name = { \
.mult = 1, \
.div = _div, \
.hw.init = &(struct clk_init_data){ \
.name = #_name, \
.ops = &clk_fixed_factor_ops, \
.parent_hws = (const struct clk_hw *[]) { \
&a9_##_name##_en.hw \
}, \
.num_parents = 1, \
.flags = CLK_SET_RATE_PARENT, \
}, \
}; \
static A9_VCLK(vclk_div2, VID_CLK_CTRL, 1, 2, &a9_vclk.hw);
static A9_VCLK(vclk_div4, VID_CLK_CTRL, 2, 4, &a9_vclk.hw);
static A9_VCLK(vclk_div6, VID_CLK_CTRL, 3, 6, &a9_vclk.hw);
static A9_VCLK(vclk_div6, VID_CLK_CTRL, 4, 12, &a9_vclk.hw);
static A9_VCLK(vclk2_div2, VIID_CLK_CTRL, 1, 2, &a9_vclk2.hw);
static A9_VCLK(vclk2_div4, VIID_CLK_CTRL, 2, 4, &a9_vclk2.hw);
static A9_VCLK(vclk2_div6, VIID_CLK_CTRL, 3, 6, &a9_vclk2.hw);
static A9_VCLK(vclk2_div6, VIID_CLK_CTRL, 4, 12, &a9_vclk2.hw);
If you think splitting it further into separate helper macros would
improve readability.
I can do that as well.
>> +
>> +static struct clk_fixed_factor a9_vclk_div2 = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = CLK_HW_INIT_HW("vclk_div2", &a9_vclk_div2_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div4_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 2,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div4_en", &a9_vclk.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk_div4 = {
>> + .mult = 1,
>> + .div = 4,
>> + .hw.init = CLK_HW_INIT_HW("vclk_div4", &a9_vclk_div4_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div6_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 3,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div6_en", &a9_vclk.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk_div6 = {
>> + .mult = 1,
>> + .div = 6,
>> + .hw.init = CLK_HW_INIT_HW("vclk_div6", &a9_vclk_div6_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk_div12_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL,
>> + .bit_idx = 4,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk_div12_en", &a9_vclk.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk_div12 = {
>> + .mult = 1,
>> + .div = 12,
>> + .hw.init = CLK_HW_INIT_HW("vclk_div12", &a9_vclk_div12_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_sel = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = VIID_CLK_CTRL,
>> + .mask = 0x7,
>> + .shift = 16,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_DATA("vclk2_sel", a9_vclk_parents,
>> + &clk_regmap_mux_ops, 0),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_in = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_DIV,
>> + .bit_idx = 16,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_in", &a9_vclk2_sel.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = VIID_CLK_DIV,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div", &a9_vclk2_in.hw,
>> + &clk_regmap_divider_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 19,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2", &a9_vclk2_div.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div1_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 0,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div1_en", &a9_vclk2.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div2_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 1,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div2_en", &a9_vclk2.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk2_div2 = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div2", &a9_vclk2_div2_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div4_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 2,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div4_en", &a9_vclk2.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk2_div4 = {
>> + .mult = 1,
>> + .div = 4,
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div4", &a9_vclk2_div4_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div6_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 3,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div6_en", &a9_vclk2.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk2_div6 = {
>> + .mult = 1,
>> + .div = 6,
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div6", &a9_vclk2_div6_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_vclk2_div12_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VIID_CLK_CTRL,
>> + .bit_idx = 4,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div12_en", &a9_vclk2.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_fixed_factor a9_vclk2_div12 = {
>> + .mult = 1,
>> + .div = 12,
>> + .hw.init = CLK_HW_INIT_HW("vclk2_div12", &a9_vclk2_div12_en.hw,
>> + &clk_fixed_factor_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +/* Channel 5, 6 and 7 are unconnected */
>> +static u32 a9_vid_parents_val_table[] = { 0, 1, 2, 3, 4, 8, 9, 10, 11, 12 };
>> +static const struct clk_hw *a9_vid_parents[] = {
>> + &a9_vclk_div1_en.hw,
>> + &a9_vclk_div2.hw,
>> + &a9_vclk_div4.hw,
>> + &a9_vclk_div6.hw,
>> + &a9_vclk_div12.hw,
>> + &a9_vclk2_div1_en.hw,
>> + &a9_vclk2_div2.hw,
>> + &a9_vclk2_div4.hw,
>> + &a9_vclk2_div6.hw,
>> + &a9_vclk2_div12.hw
>> +};
>> +
>> +static struct clk_regmap a9_vdac_sel = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = VIID_CLK_DIV,
>> + .mask = 0xf,
>> + .shift = 28,
>> + .table = a9_vid_parents_val_table,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_HW("vdac_sel", a9_vid_parents
>> + , &clk_regmap_mux_ops, 0),
>> +};
>> +
>> +static struct clk_regmap a9_vdac = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL2,
>> + .bit_idx = 4,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("vdac", &a9_vdac_sel.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
>> +static struct clk_regmap a9_enc_sel = {
> Should this be enc0 then ? for consistency ?
> Same applies to similar instance (it is the same discussion we already
> had on the T7 I believe)
Yes, that makes sense.
I'll rename them to a9_encoder0 and a9_encoder1 for consistency, and
I'll check for other similar instances as well.
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = VIID_CLK_DIV,
>> + .mask = 0xf,
>> + .shift = 12,
>> + .table = a9_vid_parents_val_table,
>> + },
>> + .hw.init = CLK_HW_INIT_PARENTS_HW("enc_sel", a9_vid_parents
>> + , &clk_regmap_mux_ops, 0),
>> +};
>> +
>> +static struct clk_regmap a9_enc = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = VID_CLK_CTRL2,
>> + .bit_idx = 10,
>> + },
>> + .hw.init = CLK_HW_INIT_HW("enc", &a9_enc_sel.hw,
>> + &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
>> +};
>> +
[ ... ]
> --
> Jerome
--
Jian
^ permalink raw reply
* Re: [PATCH V12 7/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
From: Andy Shevchenko @ 2026-06-15 11:21 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
Chris Morgan
In-Reply-To: <20260611202607.85376-8-macroalpha82@gmail.com>
On Thu, Jun 11, 2026 at 03:26:04PM -0500, Chris Morgan wrote:
> Add icm42607 accelerometer sensor for icm42607.
...
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
Please, please, use IWYU! So many headers are missing...
(Same comment to all files in this series.)
+ array_size.h
+ bits.h // BIT()
+ cleanup.h // guard()()
+ device/devres.h // devm_kasprintf()
+ err.h // -EINVAL, IS_ERR()
> +#include <linux/iio/iio.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
+ types.h // s16, __be16
Also you need to have
asm/byteorder.h // be16_to_cpup()
...
> +static int inv_icm42607_accel_read_odr(struct inv_icm42607_state *st,
> + int *val, int *val2)
> +{
> + unsigned int odr;
> + unsigned int i;
> +
> + guard(mutex)(&st->lock);
> +
> + odr = st->conf.accel.odr;
> +
> + for (i = 5; i < ARRAY_SIZE(inv_icm42607_accel_odr); ++i) {
Why pre-increment? Same for all other cases.
> + if (i == odr)
> + break;
> + }
> + if (i >= ARRAY_SIZE(inv_icm42607_accel_odr))
> + return -EINVAL;
> +
> + *val = inv_icm42607_accel_odr[i][0];
> + *val2 = inv_icm42607_accel_odr[i][1];
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/3] iio: common: st_sensors: honour channel endianness in read_axis_data
From: me @ 2026-06-15 11:12 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Herman van Hazendonk, linusw, denis.ciocca, robh, krzk+dt,
conor+dt, dlechner, nuno.sa, andy, sanjayembeddedse,
maudspierings, linux-iio, devicetree, linux-kernel
In-Reply-To: <20260614193908.522272ed@jic23-huawei>
On 2026-06-14 20:39, Jonathan Cameron wrote:
> On Fri, 5 Jun 2026 12:08:41 +0200
> Herman van Hazendonk <github.com@herrie.org> wrote:
>
>> st_sensors_read_axis_data() unconditionally decoded multi-byte
>> results with get_unaligned_le16() / get_unaligned_le24() regardless
>> of the channel's declared scan_type.endianness.
>>
>> For every ST sensor that has used this helper since it was introduced
>> this happened to be fine because the ST IMU/accel/gyro/pressure
>> families publish their data registers as little-endian and the
>> channel specs in those drivers declare IIO_LE accordingly.
>>
>> The LSM303DLH magnetometer however publishes its X/Y/Z output as a
>> pair of big-endian bytes (the H register sits at the lower address,
>> 0x03/0x05/0x07, and the L register immediately after), and its
>> channel specs in st_magn_core.c correctly declare IIO_BE -- but
>> read_axis_data() ignored that and decoded as little-endian, swapping
>> the high and low bytes of every magnetometer sample.
>>
>> The bug is most visible on a stationary chip: in earth's field the
>> true X reading is small and the high byte sits at 0x00, so swapping
>> the bytes pins sysfs X at exactly the low byte's pattern (e.g. 0x00F0
>> = 240). Y and Z still appear "to vary" because their magnitudes are
>> larger and the noise in the low byte produces big swings in the
>> swapped high byte:
>>
>> before (chip flat, sysfs in_magn_*_raw):
>> X=240 (stuck), Y= 12032..23296, Z=-16128..-9728
>>
>> after (direct i2c-dev big-endian decode, same chip same
>> orientation):
>> X≈-4096, Y≈210, Z≈80 (sensible values reflecting earth's
>> ambient field at low gauss range)
>>
>> Fix read_axis_data() to dispatch on ch->scan_type.endianness and
>> call get_unaligned_be16() / get_unaligned_be24() when the channel
>> declares IIO_BE. Existing IIO_LE consumers (st_accel, st_gyro,
>> st_pressure, st_lsm6dsx and others) are unaffected because their
>> channel specs already declare IIO_LE and the LE path is unchanged.
>>
> Hi Herman,
>
> Fixes tag please. We want to know how far to backport and provide
> info to those trying to work out if the bug affects their kernel
> trees.
>
> Thanks
>
> Jonathan
>
>> Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
Hi Jonathan,
I've been working with the HP TouchPad which uses the Qualcomm APQ8060
to bring this to mainline kernel, which was running a very ancient
2.6.35 kernel where I could compare things since it was working there
without issues, hence I could compare them side by side, check values
etc.
It seems this "bug" was introduced with 3.9:
Fixes: 23491b513bcd ("iio:common: Add STMicroelectronics common
library")
That's the commit that introduced st_sensors_read_axis_data() with the
unconditional get_unaligned_le16() decode.
I'll include that for the v2.
Thanks,
Herman
^ permalink raw reply
* Re: [PATCH v4 2/7] dt-bindings: display: bridge: Document Renesas R-Car V4H DSC bindings
From: Tomi Valkeinen @ 2026-06-15 11:07 UTC (permalink / raw)
To: Conor Dooley, Tomi Valkeinen
Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Marek Vasut,
Laurent Pinchart, Kieran Bingham, Philipp Zabel,
linux-renesas-soc, linux-clk, linux-kernel, dri-devel, devicetree
In-Reply-To: <20260615-undiluted-gallery-d7318905c8fe@wendy>
Hi,
On 15/06/2026 12:38, Conor Dooley wrote:
> On Mon, Jun 15, 2026 at 09:28:07AM +0300, Tomi Valkeinen wrote:
>> From: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>
>> The Renesas DSC Display Stream Compression is a bridge embedded in the
>> Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to
>> 8k or 400 Mpixel/s .
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> [tomi.valkeinen: fix the example]
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>> .../bindings/display/bridge/renesas,dsc.yaml | 99 ++++++++++++++++++++++
>> 1 file changed, 99 insertions(+)
>
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - renesas,r8a779g0-dsc
>> + - const: renesas,rcar-dsc
>
> I didn't provide an ack for this FYI, I was giving the ack for the
> file being called renesas,r8a779g0-dsc.yaml. Since Geert isn't happy
> with what's here, could you drop my ack when you send another version?
Yep, I took too much liberty there, sorry about that. I'll drop your
ack, unless I move to just renaming the file to "renesas,r8a779g0-dsc.yaml".
Tomi
^ permalink raw reply
* Re: [PATCH v4 2/7] dt-bindings: display: bridge: Document Renesas R-Car V4H DSC bindings
From: Tomi Valkeinen @ 2026-06-15 11:06 UTC (permalink / raw)
To: Geert Uytterhoeven, Tomi Valkeinen
Cc: Michael Turquette, Stephen Boyd, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Magnus Damm, Marek Vasut, Laurent Pinchart, Kieran Bingham,
Philipp Zabel, linux-renesas-soc, linux-clk, linux-kernel,
dri-devel, devicetree, Conor Dooley
In-Reply-To: <CAMuHMdVxwvDmRqsdbA_ReCnnaDWbzzTKGCXw-64U-bSJfgiy7g@mail.gmail.com>
Hi Geert,
On 15/06/2026 12:24, Geert Uytterhoeven wrote:
> Hi Tomi,
>
> On Mon, 15 Jun 2026 at 08:28, Tomi Valkeinen
> <tomi.valkeinen+renesas@ideasonboard.com> wrote:
>> From: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>
>> The Renesas DSC Display Stream Compression is a bridge embedded in the
>> Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to
>> 8k or 400 Mpixel/s .
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> [tomi.valkeinen: fix the example]
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> Thanks for the update!
>
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsc.yaml
>> @@ -0,0 +1,99 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/bridge/renesas,dsc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Renesas R-Car DSC Display Stream Compression
>> +
>> +maintainers:
>> + - Marek Vasut <marek.vasut+renesas@mailbox.org>
>> +
>> +description: |
>> + This binding describes the VESA DSC Display Stream Compression encoder
>> + embedded in the Renesas R-Car V4H SoC. The encoder supports all DSC1.1
>> + encoding mechanisms, configurable bits-per-pixel, resolution up to 8k.
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - renesas,r8a779g0-dsc
>> + - const: renesas,rcar-dsc
>
> Please do not introduce new "renesas,rcar" fallback compatible values,
> unless you are adding new support for a hardware block that is present
> in SoCs belonging to multiple (nowadays I would say at least three)
> R-Car generations. The DSC is only found in R-Car Gen4 SoCs, so
> "renesas,rcar-gen4-dsc" sounds more appropriate.
Ok.
> However, so far R-Car V4H is the only R-Car Gen4 SoC that has a DSC.
Right. Even "renesas,rcar-gen4-dsc" feels a bit too wide.
None of the other rcar du related bindings define fallback compatibles.
So maybe we can just keep "renesas,r8a779g0-dsc" and name the file
"renesas,r8a779g0-dsc.yaml" (which was the review comment I'm trying to
resolve with this change).
> Even the very similar R-Car V4M does not seem to have it.
> R-Car X5H has DSC-functionality integrated in its DisplayPort TX
> controller, so that seems to be a different implementation?
I'm not that familiar with DSC functionality, but my understanding is
that it's not DSI or DP specific, it just outputs packed parallel video.
I couldn't find details about X5H's DSC, it could even be the same block
(or not).
Tomi
^ permalink raw reply
* Re: [PATCH v3 1/5] clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations
From: sashiko-bot @ 2026-06-15 11:04 UTC (permalink / raw)
To: Prabhakar; +Cc: conor+dt, wsa+renesas, robh, devicetree
In-Reply-To: <20260615104845.4122868-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The documentation for `input_fref` explicitly states it should be in MHz, but the implementation assumes and requires Hz.
--
commit bcb3fbb3f1de21bfd1e1a632fe1e124e9c03de3b
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations
This commit introduces a per-SoC PLL reference input frequency parameter to
the rzv2h_pll_limits struct. It updates the PLL calculation logic to use
this dynamically provided reference frequency instead of a hardcoded 24MHz
constant, falling back to 24MHz if the field is not initialized.
> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
> index f6c47fb89bca5..513f8283c4796 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
[ ... ]
> @@ -237,6 +236,7 @@ struct rzv2h_plldsi_div_clk {
> bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
> struct rzv2h_pll_pars *pars, u64 freq_millihz)
> {
> + unsigned long input_fref = limits->input_fref ?: (24 * MEGA);
[ ... ]
> diff --git a/include/linux/clk/renesas.h b/include/linux/clk/renesas.h
> index c360df9fa735c..94ee70b20b44c 100644
> --- a/include/linux/clk/renesas.h
> +++ b/include/linux/clk/renesas.h
> @@ -53,6 +53,9 @@ static inline void rzg2l_cpg_dsi_div_set_divider(u8 divider, int target) { }
> * various parameters used to configure a PLL. These limits ensure
> * the PLL operates within valid and stable ranges.
> *
> + * @input_fref: Reference input frequency to the PLL (in MHz). If set
> + * to 0, a default value of 24MHz is used.
[Severity: Low]
The documentation here states that the frequency should be provided in MHz,
but the implementation in rzv2h_get_pll_pars() expects Hz, as it falls back
to (24 * MEGA) and doesn't scale input_fref.
If a developer configures a new SoC and sets .input_fref = 48 as documented,
will it be evaluated as 48 Hz instead of 48 MHz and cause the PLL limit checks
to fail? Should the kernel-doc be updated to request the frequency in Hz?
> *
> * @fout: Output frequency range (in MHz)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615104845.4122868-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1
^ permalink raw reply
* Re: [PATCH v3 5/5] clk: renesas: r9a09g077: Add LCDC and PLL3 clock support for RZ/T2H display pipeline
From: sashiko-bot @ 2026-06-15 11:02 UTC (permalink / raw)
To: Prabhakar; +Cc: robh, conor+dt, wsa+renesas, devicetree
In-Reply-To: <20260615104845.4122868-6-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] CCF statelessness contract violation corrupts driver state and hardware configuration.
- [High] Missing NULL check on `clk_hw_get_parent_by_index()` return value leads to kernel panic.
- [High] Division by zero panic in `r9a09g077_cpg_pll3_clk_recalc_rate` on uninitialized hardware.
- [Medium] Logic bug: Raw hardware value used directly as an array index in `recalc_rate`.
- [Medium] Integer truncation of the 64-bit rate parameter allows invalid requests to bypass boundary limits.
Pre-existing issues:
- [High] Use-After-Free (UAF) due to devm_kzalloc allocation assigned to a static pointer.
--
commit f2062943d79173a69b191f32a7edd6f6097d4c7d
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: clk: renesas: r9a09g077: Add LCDC and PLL3 clock support for RZ/T2H display pipeline
Add the clock definitions and PLL logic required to supply the LCDC
(VSPD/FCPVD/DU) blocks on the RZ/T2H (R9A09G077) SoC. The RZ/T2H display
subsystem depends on a dedicated PLL (PLL3) and a set of new derived
clocks.
> diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c
> index 93b15e06a19bc..3e73f2b6e5dcc 100644
> --- a/drivers/clk/renesas/r9a09g077-cpg.c
> +++ b/drivers/clk/renesas/r9a09g077-cpg.c
[ ... ]
> @@ -480,6 +576,276 @@ r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev,
> +static unsigned long r9a09g077_cpg_pll3_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct pll_clk *pll_clk = to_pll(hw);
> + u32 ctr0, ctr1;
> + u8 pdiv, sdiv;
> + u64 rate;
> + u16 mdiv;
> + s16 kdiv;
> +
> + ctr0 = readl(CPG_PLL3_VCO_CTR0(pll_clk->reg));
> + ctr1 = readl(CPG_PLL3_VCO_CTR1(pll_clk->reg));
> +
> + pdiv = FIELD_GET(CPG_PLL3_VCO_CTR0_PDIV, ctr0);
> + mdiv = FIELD_GET(CPG_PLL3_VCO_CTR0_MDIV, ctr0);
> + kdiv = (s16)FIELD_GET(CPG_PLL3_VCO_CTR1_KDIV, ctr1);
> + sdiv = FIELD_GET(CPG_PLL3_VCO_CTR1_SDIV, ctr1);
> +
> + rate = mul_u64_u32_shr(parent_rate, (mdiv << 16) + kdiv, 16 + sdiv);
> +
> + return DIV_ROUND_CLOSEST_ULL(rate, pdiv);
[Severity: High]
If the hardware comes out of reset with a value of 0 or the bootloader hasn't
configured the PLL, will pdiv be 0 here? If so, could this division trigger a
divide-by-zero kernel panic during boot when the CCF calls recalc_rate()?
> +}
> +
> +static int r9a09g077_cpg_pll3_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct pll_clk *pll_clk = to_pll(hw);
> + u64 rate_millihz;
> +
> + if (req->rate == pll_clk->cur_rate)
> + return 0;
> +
> + rate_millihz = mul_u32_u32(req->rate, MILLI);
[Severity: Medium]
Since req->rate is a 64-bit unsigned long, does casting it directly to a
32-bit integer via mul_u32_u32() implicitly truncate the rate? If a requested
rate exceeds the 32-bit bounds, could this truncation alias it into a lower
range and bypass boundary limits instead of rejecting the out-of-bounds
request?
> + if (!rzv2h_cpg_get_pll_pars(pll_clk->limits, &pll_clk->pll_parameters,
> + rate_millihz)) {
> + dev_dbg(pll_clk->dev,
> + "failed to determine rate for req->rate: %lu\n",
> + req->rate);
> + return -EINVAL;
> + }
> + req->rate = DIV_ROUND_CLOSEST_ULL(pll_clk->pll_parameters.freq_millihz, MILLI);
> + pll_clk->cur_rate = req->rate;
[Severity: High]
Does mutating pll_clk->cur_rate here violate the statelessness contract of
the Common Clock Framework? Since determine_rate is invoked during
clk_round_rate() purely to query hypothetical rates, saving the rate to the
device state here could corrupt the driver state if the queried rate is never
actually applied.
> +
> + return 0;
> +}
> +
> +static int r9a09g077_cpg_pll3_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct pll_clk *pll_clk = to_pll(hw);
> + struct rzv2h_pll_pars *params = &pll_clk->pll_parameters;
[Severity: High]
Because pll_parameters is updated in determine_rate() and used here instead
of calculating parameters based on the passed rate argument, does this mean
any rate query (which triggers determine_rate) will overwrite the state and
cause a subsequent set_rate() to program the wrong frequency?
[ ... ]
> +static int r9a09g077_cpg_lcdc_div_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
> + struct clk_hw *mux_hw = clk_hw_get_parent(hw);
> + u8 table[RZT2H_MAX_LCDC_DIV_TABLES] = { 0 };
> + struct rzv2h_pll_div_pars dsi_params;
> + const struct clk_div_table *div;
> + struct pll_clk *pll_clk;
> + unsigned int i = 0;
> + u64 freq_millihz;
> +
> + /* index 1 is always .pll3 in sel_clk_pll3[] */
> + pll_clk = to_pll(clk_hw_get_parent_by_index(mux_hw, 1));
[Severity: High]
Is there a missing NULL check here? If clk_hw_get_parent_by_index() fails to
resolve the parent clock and returns NULL, to_pll() will calculate an invalid
offset. Will this cause a fatal page fault when pll_clk->limits is
dereferenced later in this function?
> +
> + for (div = dsi_div->dtable; div->div; div++) {
> + if (i >= RZT2H_MAX_LCDC_DIV_TABLES)
> + return -EINVAL;
> + table[i++] = div->div;
> + }
> +
> + freq_millihz = mul_u32_u32(req->rate, MILLI);
[Severity: Medium]
Similar to the PLL3 determine_rate function, does using mul_u32_u32() here
truncate the 64-bit req->rate and allow out-of-bounds rate requests to bypass
limits?
> +
> + if (!rzv2h_cpg_get_pll_divs_pars(pll_clk->limits, &dsi_params, table,
> + i, freq_millihz)) {
> + dev_err(dsi_div->dev,
> + "LCDC divider failed to determine rate for req->rate: %lu\n",
> + req->rate);
> + return -EINVAL;
> + }
> +
> + req->rate = DIV_ROUND_CLOSEST_ULL(dsi_params.div.freq_millihz, MILLI);
> + req->best_parent_rate = req->rate * dsi_params.div.divider_value;
> + dsi_div->divider = dsi_params.div.divider_value;
> + pll_clk->cur_rate = req->best_parent_rate;
> + pll_clk->pll_parameters = dsi_params.pll;
[Severity: High]
Is it safe to mutate both the local divider state and the grandparent's PLL
state (pll_clk->cur_rate and pll_parameters) during a stateless query?
Like the issue in r9a09g077_cpg_pll3_determine_rate(), saving this
hypothetical state could overwrite the hardware configuration state if the
rate is just being queried.
[ ... ]
> +static unsigned long
> +r9a09g077_cpg_lcdc_div_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
> + u32 div;
> +
> + div = readl(dsi_div->reg);
> + div >>= GET_SHIFT(dsi_div->conf);
> + div &= clk_div_mask(GET_WIDTH(dsi_div->conf));
> + div = dsi_div->dtable[div].div;
[Severity: Medium]
Should div be used to iterate through the .val members of dtable rather
than being used directly as an array index? Although this might avoid a crash
if dtable_2_32 sequentially maps values 0-15 to indices 0-15, it seems
fragile and inconsistent with r9a09g077_cpg_lcdc_div_set_rate() which
properly iterates through the array to find the matching divider.
> +
> + return DIV_ROUND_CLOSEST_ULL(parent_rate, div);
> +}
This is a pre-existing issue, but I noticed a potential Use-After-Free in
r9a09g077_cpg_fselxspi_div_clk_register():
drivers/clk/renesas/r9a09g077-cpg.c:r9a09g077_cpg_fselxspi_div_clk_register() {
...
if (!xspi_div_ops) {
xspi_div_ops = devm_kzalloc(dev, sizeof(*xspi_div_ops), GFP_KERNEL);
...
init.ops = xspi_div_ops;
...
}
[Severity: High]
Since xspi_div_ops is a static pointer but allocated with device-managed
memory (devm_kzalloc), will this memory be freed if the driver is unbound?
On a subsequent rebind, does the static pointer retain its dangling value and
cause the clock framework to use freed memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615104845.4122868-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=5
^ permalink raw reply
* Re: [PATCH v3 4/5] clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library
From: sashiko-bot @ 2026-06-15 11:00 UTC (permalink / raw)
To: Prabhakar; +Cc: robh, wsa+renesas, devicetree, conor+dt
In-Reply-To: <20260615104845.4122868-5-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Arithmetic on `p.k` and `input_fref` produces incorrect results on 32-bit platforms when `p.k` is negative, due to signed-to-unsigned integer promotion and subsequent zero-extension.
--
commit 11b81f60e771c7c62903fe36e982968dc58b35e3
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library
This commit extracts the common PLL and divider calculation logic from the
RZ/V2H CPG driver into a standalone library. This restructuring allows sharing
the calculation infrastructure with upcoming platforms like RZ/T2H and RZ/N2H
without duplicating code.
> diff --git a/drivers/clk/renesas/rzv2h-cpg-lib.c b/drivers/clk/renesas/rzv2h-cpg-lib.c
> new file mode 100644
> index 0000000000000..124239c7327e4
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg-lib.c
[ ... ]
> + /* Compute (Ffvco * 65536) */
> + fvco = mul_u32_u32(p.m * 65536 + p.k, fref);
> + if (fvco < mul_u32_u32(limits->fvco.min, 65536) ||
> + fvco > mul_u32_u32(limits->fvco.max, 65536))
> + continue;
> +
> + /* PLL_M component of (output * 65536 * PLL_P) */
> + output = mul_u32_u32(p.m * 65536, input_fref);
> + /* PLL_K component of (output * 65536 * PLL_P) */
> + output += p.k * input_fref;
[Severity: High]
Will this calculation corrupt the output frequency on 32-bit platforms?
Since input_fref is unsigned long and p.k is s16, if p.k is negative on a
32-bit system, doesn't it get promoted to a 32-bit unsigned integer,
wrapping around to a large positive value?
When added to the u64 output, it would be zero-extended rather than
sign-extended, resulting in a massive positive value being added instead
of subtracting the correct proportional amount.
Should p.k be explicitly cast to s64 before the multiplication to prevent
this issue when used by 32-bit targets?
> + /* Make it in mHz */
> + output *= MILLI;
> + output = DIV_U64_ROUND_CLOSEST(output, 65536 * p.p * divider);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615104845.4122868-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=4
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: spi: snps,dw-apb-ssi: Add support for snps,dwc-ssi-2.00a
From: Conor Dooley @ 2026-06-15 10:57 UTC (permalink / raw)
To: Changhuang Liang
Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, linux-spi@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <ZQ4PR01MB12028430668DE1F9AE8645A9F2E62@ZQ4PR01MB1202.CHNPR01.prod.partner.outlook.cn>
[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]
On Mon, Jun 15, 2026 at 10:49:07AM +0000, Changhuang Liang wrote:
> Hi, Conor
>
> > On Sat, Jun 13, 2026 at 03:38:46AM +0000, Changhuang Liang wrote:
> > > Hi, Conor
> > >
> > > Thanks for the review.
> > >
> > > > On Fri, Jun 12, 2026 at 05:58:55AM -0700, Changhuang Liang wrote:
> > > > > Add a new compatible string "snps,dwc-ssi-2.00a" for the Synopsys
> > > > > DesignWare SSI controller version 2.00a.
> > > >
> > > > Two things. Firstly, driver patch suggests a fallback to 1.01a is
> > > > possible. Why haven't you added one?
> > >
> > > Will support fallback.
> > >
> > > > Secondly, I am going to expect that when your starfive user for this
> > > > appears in my inbox that it has a device-specific compatible, so you
> > > > may as well add that now.
> > >
> > > Our standard SPI control is completely based on the Synopsys DesignWare
> > SSI controller version 2.00a.
> > > However, for the controller that accesses the flash, a set of
> > > modifications has been made based on the Synopsys DesignWare SSI
> > controller version 2.00a, and the driver part also requires specific adaptation.
> > >
> > > Therefore, on the JHB100 SoC, the initial device tree source I have planned
> > looks like this:
> > >
> > > spi0: spi@17f20000 {
> > > compatible = "snps,dwc-ssi-2.00a";
> > > reg = <0x0 0x17f20000 0x0 0x10000>; };
> > >
> > > sfc0: spi@18000000 {
> > > compatible = "starfive,jhb100-spi";
> > > reg = <0x0 0x18000000 0x0 0x10000>; };
> > >
> > > Thus, I plan to add a new "starfive,jhb100-spi" compatible string for the
> > controller that accesses the flash.
> > > For now, I intend to keep these as two separate series.
> >
> > No, if you have two integrations of the designware IP with different
> > programming models on the jhb100 I expect to see two different soc-specific
> > compatibles used. I don't want to see bare synopsys compatibles on the
> > platforms I am looking after.
>
> Okay, I will change it to the following:
>
> spi0: spi@17f20000 {
> compatible = " starfive,jhb100-spi";
This one is compatible with "snps,dwc-ssi-2.00a" and "snps,dwc-ssi-1.00a",
so it should probably have those as fallbacks. The latter for sure, the
former if you know there's some non-discoverable differences between 2.00a
and 1.00a hardware.
Cheers,
Conor.
> reg = <0x0 0x17f20000 0x0 0x10000>;
> };
>
> sfc0: spi@18000000 {
> compatible = "starfive,jhb100-sfc";
> reg = <0x0 0x18000000 0x0 0x10000>;
> };
>
> Best Regards,
> Changhuang
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 7/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock
From: Krzysztof Kozlowski @ 2026-06-15 10:58 UTC (permalink / raw)
To: Zhang Yi; +Cc: alsa-devel, broonie, conor+dt, devicetree, krzk+dt, robh, tiwai
In-Reply-To: <20260615101423.19781-1-zhangyi@everest-semi.com>
On 15/06/2026 12:14, Zhang Yi wrote:
>>>>> Yes, setting mclk-from-sclk does not affect the MCLK connection.
>>>>
>>>> I am asking about wiring of the device. If MCLK is used from SCLK, but
>>>> SCLK is used as the internal clock, then how can you still have MCLK
>>>> connected?
>>>
>>> If MCLK is derived from SCLK, whether the 'MCLK' pin on the device is
>>> connected or not has no effect on operation.
>>
>> If MCLK is connected, why would you derive it from SCLK? That's the
>> point - your clocks property already might be telling everything needed
>> here.
>
> This is because sometimes the codec does not work properly
> when using the MCLK provided by the CPU.
OK.
>
>>>>>>> +
>>>>>>> + everest,hpfl:
>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint8
>>>>>>> + description:
>>>>>>> + the HPF value of ADCL.
>>>>>>
>>>>>> Is HPF value in dB? If so, use proper unit suffix and proper units.
>>>>>
>>>>> No, the values here correspond to the values in the registers.
>>>>> The value is not in dB
>>>>
>>>> What are the meanings of the register values?
>>>
>>> The value of everest,hpfl is equal to the value of the corresponding register,
>>> not the cutoff frequency of the HPF.
>>
>> You keep avoiding answers.
>>
>> I don't want you to encode standard units as register values.
>
> I think I understand what you mean.
> Do you want me to enter the physical values in DTS and then convert them to register values in the code?
If you have clear physical values then this should be rather used in DTS.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v7 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers
From: Mukesh Savaliya @ 2026-06-15 10:54 UTC (permalink / raw)
To: Bjorn Andersson
Cc: viken.dadhaniya, andi.shyti, robh, krzk+dt, conor+dt, vkoul,
Frank.Li, konradybcio, dmitry.baryshkov, linmq006, quic_jseerapu,
agross, linux-arm-msm, linux-i2c, devicetree, linux-kernel,
dmaengine, krzysztof.kozlowski, bartosz.golaszewski,
bjorn.andersson, konrad.dybcio
In-Reply-To: <9a0a2ba2-4f1b-425d-979b-fe59192bb2cd@oss.qualcomm.com>
Hi Bjorn, Please help next with the approach, accordingly i shall
update next patch.
On 5/25/2026 12:46 PM, Mukesh Savaliya wrote:
> Hi Bjorn, Thanks for the detailed review.
>
> On 5/22/2026 8:36 AM, Bjorn Andersson wrote:
>> On Thu, Apr 23, 2026 at 08:25:50PM +0530, Mukesh Kumar Savaliya wrote:
>>> On platforms where a GENI Serial Engine is shared with another system
>>> processor, selecting the "sleep" pinctrl state can disrupt ongoing
>>> transfers initiated by the other processor.
>>>
>>
>> Isn't it strange that the DeviceTree will define a sleep state for the
>> OS to select, but when this other property is set the OS should never
>> select this state?
>>
>
> The intent here is that for multi-owner configurations the
> “sleep” pinctrl state is not safe to use, since the pins may
> still be actively driven by another execution environment.
> Selecting the sleep state in such cases can disrupt transfers
> initiated by the other owner.
>
> You're right that this constraint is currently not described
> in the binding, which makes the behavior non-obvious.
>
> shall i update the DT binding to clarify that when
> "qcom,qup-multi-owner" is present ? The OS must not transition
> the pins to the "sleep" state, as the hardware is shared and
> may be active outside of Linux control.
>
> Alternatively, we can also consider relaxing the requirement
> to define a sleep state for such nodes if that aligns better
> with DT expectations.
>
>>> Teach geni_se_resources_off() to skip selecting the pinctrl sleep state
>>> when the Serial Engine is marked as shared, while still allowing the
>>> rest of the resource shutdown sequence to proceed.
>>>
>>> This is required for multi-owner configurations (described via
>>> DeviceTree
>>> with qcom,qup-multi-owner on the protocol controller node).
>>>
>>
>> The requirement as such is reasonable, but you don't define in the
>> binding that when this property is set, the sleep state must not be
>> selected by the OS...
>>
>
> Please let me know if you prefer second approach over the first, i shall
> update accordingly.
>
>> Regards,
>> Bjorn
>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
>>> ---
> [...]
>
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: spi: snps,dw-apb-ssi: Add support for snps,dwc-ssi-2.00a
From: Changhuang Liang @ 2026-06-15 10:49 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, linux-spi@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20260615-disallow-dosage-333752ccddc1@wendy>
Hi, Conor
> On Sat, Jun 13, 2026 at 03:38:46AM +0000, Changhuang Liang wrote:
> > Hi, Conor
> >
> > Thanks for the review.
> >
> > > On Fri, Jun 12, 2026 at 05:58:55AM -0700, Changhuang Liang wrote:
> > > > Add a new compatible string "snps,dwc-ssi-2.00a" for the Synopsys
> > > > DesignWare SSI controller version 2.00a.
> > >
> > > Two things. Firstly, driver patch suggests a fallback to 1.01a is
> > > possible. Why haven't you added one?
> >
> > Will support fallback.
> >
> > > Secondly, I am going to expect that when your starfive user for this
> > > appears in my inbox that it has a device-specific compatible, so you
> > > may as well add that now.
> >
> > Our standard SPI control is completely based on the Synopsys DesignWare
> SSI controller version 2.00a.
> > However, for the controller that accesses the flash, a set of
> > modifications has been made based on the Synopsys DesignWare SSI
> controller version 2.00a, and the driver part also requires specific adaptation.
> >
> > Therefore, on the JHB100 SoC, the initial device tree source I have planned
> looks like this:
> >
> > spi0: spi@17f20000 {
> > compatible = "snps,dwc-ssi-2.00a";
> > reg = <0x0 0x17f20000 0x0 0x10000>; };
> >
> > sfc0: spi@18000000 {
> > compatible = "starfive,jhb100-spi";
> > reg = <0x0 0x18000000 0x0 0x10000>; };
> >
> > Thus, I plan to add a new "starfive,jhb100-spi" compatible string for the
> controller that accesses the flash.
> > For now, I intend to keep these as two separate series.
>
> No, if you have two integrations of the designware IP with different
> programming models on the jhb100 I expect to see two different soc-specific
> compatibles used. I don't want to see bare synopsys compatibles on the
> platforms I am looking after.
Okay, I will change it to the following:
spi0: spi@17f20000 {
compatible = " starfive,jhb100-spi";
reg = <0x0 0x17f20000 0x0 0x10000>;
};
sfc0: spi@18000000 {
compatible = "starfive,jhb100-sfc";
reg = <0x0 0x18000000 0x0 0x10000>;
};
Best Regards,
Changhuang
^ 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