* Re: [PATCH RFC v4 02/12] dt-bindings: clk: zte: Add zx297520v3 matrix clock and reset bindings
From: Conor Dooley @ 2026-06-17 16:11 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260616-zx29clk-v4-2-ca994bd22e9d@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]
On Tue, Jun 16, 2026 at 11:26:22PM +0300, Stefan Dösinger wrote:
> I split matrixclk into its own controller again because syscon/regmap
> deals poorly with device nodes that have more than one memory region. As
> a consequence I am passing all PLL outputs generated on Topclk down to
> Matrixclk.
This type of commentary FWIW can go below the --- line and instead just
write a normal commit message.
I do appreciate though that you put the information in the individual
patch.
> The syscon is used to generate the regmap shared between the clock and
> auxiliary reset drivers. The register space also contains at least one
> extra block of functionality, hardware spinlocks, that I expect will be
> necessary to communicate correctly with the LTE DSP firmware blob.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
> ---
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +
> + topclk: clock-controller@13b000 {
> + compatible = "zte,zx297520v3-topclk", "syscon";
> + reg = <0x0013b000 0x400>;
> + clocks = <&osc26m>, <&osc32k>;
> + clock-names = "osc26m", "osc32k";
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
This should be removed from here, the tooling will satisfy the topclk
references, just as it has done for osc26m and osc32k. The example
should just contain the node the binding documents (and its children).
pw-bot: changes-requested
Looks fine otherwise.
Cheers,
Conor.
> +
> + clock-controller@1306000 {
> + compatible = "zte,zx297520v3-matrixclk", "syscon";
> + reg = <0x01306000 0x400>;
> + clocks = <&osc26m>, <&osc32k>,
> + <&topclk ZX297520V3_MPLL>, <&topclk ZX297520V3_MPLL_D2>,
> + <&topclk ZX297520V3_MPLL_D3>, <&topclk ZX297520V3_MPLL_D4>,
> + <&topclk ZX297520V3_MPLL_D5>, <&topclk ZX297520V3_MPLL_D6>,
> + <&topclk ZX297520V3_MPLL_D8>, <&topclk ZX297520V3_MPLL_D12>,
> + <&topclk ZX297520V3_MPLL_D16>, <&topclk ZX297520V3_MPLL_D26>,
> + <&topclk ZX297520V3_UPLL>, <&topclk ZX297520V3_UPLL_D2>,
> + <&topclk ZX297520V3_UPLL_D3>, <&topclk ZX297520V3_UPLL_D4>,
> + <&topclk ZX297520V3_UPLL_D5>, <&topclk ZX297520V3_UPLL_D6>,
> + <&topclk ZX297520V3_UPLL_D8>, <&topclk ZX297520V3_UPLL_D12>,
> + <&topclk ZX297520V3_UPLL_D16>,
> + <&topclk ZX297520V3_DPLL>, <&topclk ZX297520V3_DPLL_D2>,
> + <&topclk ZX297520V3_DPLL_D3>, <&topclk ZX297520V3_DPLL_D4>,
> + <&topclk ZX297520V3_DPLL_D5>, <&topclk ZX297520V3_DPLL_D6>,
> + <&topclk ZX297520V3_DPLL_D8>, <&topclk ZX297520V3_DPLL_D12>,
> + <&topclk ZX297520V3_DPLL_D16>,
> + <&topclk ZX297520V3_GPLL>, <&topclk ZX297520V3_GPLL_D2>,
> + <&topclk ZX297520V3_GPLL_D3>, <&topclk ZX297520V3_GPLL_D4>,
> + <&topclk ZX297520V3_GPLL_D5>, <&topclk ZX297520V3_GPLL_D6>,
> + <&topclk ZX297520V3_GPLL_D8>, <&topclk ZX297520V3_GPLL_D12>,
> + <&topclk ZX297520V3_GPLL_D16>;
> + clock-names = "osc26m", "osc32k", "mpll", "mpll_d2", "mpll_d3",
> + "mpll_d4", "mpll_d5", "mpll_d6", "mpll_d8", "mpll_d12",
> + "mpll_d16", "mpll_d26", "upll", "upll_d2", "upll_d3",
> + "upll_d4", "upll_d5", "upll_d6", "upll_d8", "upll_d12",
> + "upll_d16", "dpll", "dpll_d2", "dpll_d3", "dpll_d4",
> + "dpll_d5", "dpll_d6", "dpll_d8", "dpll_d12", "dpll_d16",
> + "gpll", "gpll_d2", "gpll_d3", "gpll_d4", "gpll_d5",
> + "gpll_d6", "gpll_d8", "gpll_d12", "gpll_d16";
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings
From: Conor Dooley @ 2026-06-17 16:08 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260616-zx29clk-v4-1-ca994bd22e9d@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5389 bytes --]
On Tue, Jun 16, 2026 at 11:26:21PM +0300, Stefan Dösinger wrote:
> These SoCs have 3 clock and reset controllers: Top, Matrix and LSP. The
> separation of concerns between Top and Matrix and the interface between
> them is poorly defined in the hardware, so the bindings list all
> potential PLL clocks that might be passed between them.
>
> Generally every device has two clocks (one work clock, and one that
> connects it to the bus, I call it PCLK), two reset bits (I don't know
> what the difference is - sometimes asserting one is enough to reset the
> device, sometimes both need to be asserted). PCLK and WCLK are
> controlled by individual gates. Some devices have a mux and/or a
> divider for their work clock. Some devices, like the GPIO controller,
> only have reset bits and no clocks.
>
> The top clock controller is fed by a 26mhz external oscillator and has 4
> PLLs to generate other clock rates. ZTE's kernel mostly relies on the
> boot ROM to set up PLLs, but one LTE-Related PLL is not configured
> on some boards. Therefore my driver contains code to program PLLs. It
> produces identical settings as the boot ROM for the pre-programmed
> frequencies.
>
> Not all clocks will have an explicit user in the end. I am defining a
> lot of them simply to shut them off. The boot loader sets up a few of
> the proprietary timers, which will send regular IRQs (although the
> kernel of course doesn't need to listen to them). I don't plan to add a
> driver for the proprietary timer as I see no use for them - the ARM arch
> timer works just fine. I will add a driver for the very similar
> proprietary watchdog though.
>
> The clock list in this patch is pretty complete but not exhaustive.
> There are other bits that are enabled, but I couldn't deduce what they
> are controlling by trial and error. Some of them seem to do nothing.
> Others cause an instant hang of the board when disabled. It is quite
> likely that a handful more clocks will be added in the future, but not a
> large number.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
> ---
> .../bindings/clock/zte,zx297520v3-topclk.yaml | 70 ++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/clock/zte,zx297520v3-clk.h | 118 +++++++++++++++++++++
> 3 files changed, 190 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/zte,zx297520v3-topclk.yaml b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topclk.yaml
> new file mode 100644
> index 000000000000..374f63891288
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topclk.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/zte,zx297520v3-topclk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ZTE zx297520v3 SoC top clock and reset controller
> +
> +maintainers:
> + - Stefan Dösinger <stefandoesinger@gmail.com>
> +
> +description: |
> + The zx297520v3's top clock controller generates clocks for core devices on the
> + board like the main bus, USB and timers. In addition to clocks it has reset
> + controls for peripherals, a global board reset and watchdog reset controls.
> +
> + The controller has two clock inputs: a 26 MHz and a 32 KHz external
> + oscillator. They need to be provided as input clocks. The controller provides
> + clocks to the downstream Matrix clock controller.
> +
> + All available clocks are defined as preprocessor macros in the
> + 'dt-bindings/clock/zte,zx297520v3-clk.h' header.
> +
> +properties:
> + compatible:
> + items:
> + - const: zte,zx297520v3-topclk
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: 26 MHz external oscillator
> + - description: 32 KHz external oscillator
> +
> + clock-names:
> + items:
> + - const: osc26m
> + - const: osc32k
> +
> + "#clock-cells":
> + const: 1
> +
> + "#reset-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - '#clock-cells'
> + - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +
> + clock-controller@13b000 {
> + compatible = "zte,zx297520v3-topclk", "syscon";
> + reg = <0x0013b000 0x400>;
> + clocks = <&osc26m>, <&osc32k>;
> + clock-names = "osc26m", "osc32k";
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8629ed2aa82f..0cc1ede3c80c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3867,8 +3867,10 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Odd fixes
> F: Documentation/arch/arm/zte/
> F: Documentation/devicetree/bindings/arm/zte.yaml
> +F: Documentation/devicetree/zte,zx297520v3-*
Sashiko complaint here looks valid.
FWIW
/scripts/get_maintainer.pl --self-test=patterns
will catch these kinds of things.
pw-bot: changes-requested
Cheers,
Conor.
> F: arch/arm/boot/dts/zte/
> F: arch/arm/mach-zte/
> +F: include/dt-bindings/clock/zte,zx297520v3-clk.h
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
From: Conor Dooley @ 2026-06-17 16:05 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: linux-iio, devicetree, linux-kernel, jic23, nuno.sa,
Michael.Hennerich, dlechner, andy, robh, krzk+dt, conor+dt,
pop.ioan-daniel, marcelo.schmitt1
In-Reply-To: <f9e88abdbd23df8039282497a81d3c8698a10665.1781661028.git.marcelo.schmitt@analog.com>
[-- Attachment #1: Type: text/plain, Size: 6691 bytes --]
On Tue, Jun 16, 2026 at 11:03:11PM -0300, Marcelo Schmitt wrote:
> Document how to describe LTC2378-20 and similar ADCs in device tree.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Change log v2 -> v3:
> - Re-added device tree fallback compatibles for LTC2378 chips, now with options
> to provide a single compatible string or a pair of single compatible string
> plus a fallback string to a slower sample rate spec in case a driver for the
> specific part is not found.
>
> .../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 167 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
> new file mode 100644
> index 000000000000..7d30a2cade8f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
> @@ -0,0 +1,160 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2378 and similar Analog to Digital Converters
> +
> +maintainers:
> + - Marcelo Schmitt <marcelo.schmitt@analog.com>
> +
> +description: |
> + Analog Devices LTC2378 series of ADCs.
> + Specifications can be found at:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + # Single compatible string match.
> + - enum:
> + - adi,ltc2338-18
> + - adi,ltc2364-16
> + - adi,ltc2364-18
> + - adi,ltc2367-16
> + - adi,ltc2367-18
> + - adi,ltc2368-16
> + - adi,ltc2368-18
> + - adi,ltc2369-18
> + - adi,ltc2370-16
> + - adi,ltc2376-16
> + - adi,ltc2376-18
> + - adi,ltc2376-20
> + - adi,ltc2377-16
> + - adi,ltc2377-18
> + - adi,ltc2377-20
> + - adi,ltc2378-16
> + - adi,ltc2378-18
> + - adi,ltc2378-20
> + - adi,ltc2379-18
> + - adi,ltc2380-16
> +
> + # Low sample rate fallback for 16-bit unipolar sensors.
> + - items:
> + - enum:
> + - adi,ltc2370-16 # 2 MSPS
> + - adi,ltc2368-16 # 1 MSPS
> + - adi,ltc2367-16 # 500 kSPS
> + - const: adi,ltc2364-16 # fallback (250 kSPS)
Your driver still matches on ltc2370-16, which makes me question the
value of these fallbacks. That said, the chip info struct contains no
information about sampling rate. What actually is the impact of the
sample rate on the programming model?
Is there actually a benefit to matching on ltc2370-16, or can you just
match on the fallback?
+static const struct ltc2378_chip_info ltc2370_16_chip_info = {
+ .name = "ltc2370-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2368_16_chip_info = {
+ .name = "ltc2368-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2367_16_chip_info = {
+ .name = "ltc2367-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2364_16_chip_info = {
+ .name = "ltc2364-16",
+ .resolution = 16,
+ .bipolar = false,
+};
All the devices have the same match data, other than the name, as the
fallback.
> +
> + # Low sample rate fallback for 18-bit unipolar sensors.
> + - items:
> + - enum:
> + - adi,ltc2369-18 # 1.6 MSPS
> + - adi,ltc2368-18 # 1 MSPS
> + - adi,ltc2367-18 # 500 kSPS
> + - const: adi,ltc2364-18 # fallback (250 kSPS)
> +
> + # Low sample rate fallback for 16-bit bipolar sensors.
> + - items:
> + - enum:
> + - adi,ltc2380-16 # 2 MSPS
> + - adi,ltc2378-16 # 1 MSPS
> + - adi,ltc2377-16 # 500 kSPS
> + - const: adi,ltc2376-16 # fallback (250 kSPS)
> +
> + # Low sample rate fallback for 18-bit bipolar sensors.
> + - items:
> + - enum:
> + - adi,ltc2379-18 # 1.6 MSPS
> + - adi,ltc2338-18 # 1 MSPS
> + - adi,ltc2378-18 # 1 MSPS
> + - adi,ltc2377-18 # 500 kSPS
> + - const: adi,ltc2376-18 # fallback (250 kSPS)
> +
> + # Low sample rate fallback for 20-bit bipolar sensors.
> + - items:
> + - enum:
> + - adi,ltc2378-20 # 1 MSPS
> + - adi,ltc2377-20 # 500 kSPS
> + - const: adi,ltc2376-20 # fallback (250 kSPS)
I didn't check these, but I assume they are the same.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v7 4/4] arm64: tegra: Reorder reg and reg-names to match bindings
From: Thierry Reding @ 2026-06-17 16:01 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter, Karthikeyan Mitran,
Hou Zhiqiang, Thomas Petazzoni, Pali Rohár, Michal Simek,
Kevin Xie, Thierry Reding, Aksh Garg
Cc: linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding
In-Reply-To: <20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com>
From: Thierry Reding <treding@nvidia.com>
The ECAM region cannot be the first entry in the "reg" property, because
in that case the unit-address wouldn't match the first entry. The order
of the nodes can also not be changed to match the ECAM entry because the
ECAM region is global and outside of any of the control busses.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- rebase onto v7.1-rc1
Changes in v4:
- revert ECAM "reg" entry order
Changes in v2:
- order ECAM "reg" entry before others
---
arch/arm64/boot/dts/nvidia/tegra264.dtsi | 48 ++++++++++++++++----------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra264.dtsi b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
index 8f4350c7793b..4c701abd25a8 100644
--- a/arch/arm64/boot/dts/nvidia/tegra264.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
@@ -3513,11 +3513,11 @@ cmdqv4: cmdqv@b200000 {
pci@c000000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xd0 0xb0000000 0x0 0x10000000>,
- <0x00 0x0c000000 0x0 0x00004000>,
+ reg = <0x00 0x0c000000 0x0 0x00004000>,
<0x00 0x0c004000 0x0 0x00001000>,
- <0x00 0x0c005000 0x0 0x00001000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri";
+ <0x00 0x0c005000 0x0 0x00001000>,
+ <0xd0 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -3893,12 +3893,12 @@ gpio_uphy: gpio@8300000 {
pci@8400000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xa8 0xb0000000 0x0 0x10000000>,
- <0x00 0x08400000 0x0 0x00004000>,
+ reg = <0x00 0x08400000 0x0 0x00004000>,
<0x00 0x08404000 0x0 0x00001000>,
<0x00 0x08405000 0x0 0x00001000>,
- <0x00 0x08410000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08410000 0x0 0x00010000>,
+ <0xa8 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -3925,12 +3925,12 @@ pci@8400000 {
pci@8420000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xb0 0xb0000000 0x0 0x10000000>,
- <0x00 0x08420000 0x0 0x00004000>,
+ reg = <0x00 0x08420000 0x0 0x00004000>,
<0x00 0x08424000 0x0 0x00001000>,
<0x00 0x08425000 0x0 0x00001000>,
- <0x00 0x08430000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08430000 0x0 0x00010000>,
+ <0xb0 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -3957,12 +3957,12 @@ pci@8420000 {
pci@8440000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xb8 0xb0000000 0x0 0x10000000>,
- <0x00 0x08440000 0x0 0x00004000>,
+ reg = <0x00 0x08440000 0x0 0x00004000>,
<0x00 0x08444000 0x0 0x00001000>,
<0x00 0x08445000 0x0 0x00001000>,
- <0x00 0x08450000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08450000 0x0 0x00010000>,
+ <0xb8 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -3989,12 +3989,12 @@ pci@8440000 {
pci@8460000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xc0 0xb0000000 0x0 0x10000000>,
- <0x00 0x08460000 0x0 0x00004000>,
+ reg = <0x00 0x08460000 0x0 0x00004000>,
<0x00 0x08464000 0x0 0x00001000>,
<0x00 0x08465000 0x0 0x00001000>,
- <0x00 0x08470000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08470000 0x0 0x00010000>,
+ <0xc0 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -4021,12 +4021,12 @@ pci@8460000 {
pci@8480000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xc8 0xb0000000 0x0 0x10000000>,
- <0x00 0x08480000 0x0 0x00004000>,
+ reg = <0x00 0x08480000 0x0 0x00004000>,
<0x00 0x08484000 0x0 0x00001000>,
<0x00 0x08485000 0x0 0x00001000>,
- <0x00 0x08490000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08490000 0x0 0x00010000>,
+ <0xc8 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
--
2.54.0
^ permalink raw reply related
* [PATCH v7 3/4] PCI: tegra: Add Tegra264 support
From: Thierry Reding @ 2026-06-17 16:01 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter, Karthikeyan Mitran,
Hou Zhiqiang, Thomas Petazzoni, Pali Rohár, Michal Simek,
Kevin Xie, Thierry Reding, Aksh Garg
Cc: linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding, Manikanta Maddireddy
In-Reply-To: <20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com>
From: Thierry Reding <treding@nvidia.com>
Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
driver is very small, with its main purpose being to set up the address
translation registers and then creating a standard PCI host using ECAM.
Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v7:
- select PCI_ECAM to satisfy the build dependency (Jonathan Hunter)
- remove pre-silicon support patch to avoid extra build dependency
Changes in v6:
- remove unneeded pm_runtime_disable() call (Sashiko)
- do not use noirq suspend/resume callbacks (Sashiko)
- wrap PM ops in pm_ptr() macro (Sashiko)
- use standard wait times with msleep() (Lukas Wunner)
- properly check errors for wake IRQs
- fix build failures /o\
Changes in v5:
- make PCIE_TEGRA264 symbol tristate
- drop dependency on PCI_MSI
- reorganize tegra264_pcie struct
- use standard wake-gpios property
- rename tegra264_pcie_bpmp_set_rp_state() to tegra264_pcie_power_off()
- use dev_err() instead of dev_info() for some error messages
- add clarifying comment as to why bandwidth requests aren't fatal
- address some compiler warnings on 32-bit physical address platforms
- drop needless comments
- explicitly deinitialize controller on suspend
- use devm_pm_runtime_active_enabled()
- rename "free" label to "free_ecam"
- use dev_err_probe() in more places
- reselect default pin state during resume, not probe
- return early on absence of wake GPIO
- simplify BW value calculation
Changes in v2:
- specify generations applicable for PCI_TEGRA driver to avoid confusion
- drop SPDX-FileCopyrightText tag
- rename link_state to link_up to clarify meaning
- replace memset() by an empty initializer
- sanity-check only enable BAR regions
- bring PCI link out of reset in case firmware didn't
- use common wait times instead of defining our own
- use core helpers to parse and print PCI link speed
- fix multi-line comment
- use dev_err_probe() more ubiquitously
- fix probe sequence and error cleanup
- use DEFINE_NOIRQ_DEV_PM_OPS() to avoid warnings for !PM_SUSPEND
- reuse more standard registers and remove unused register definitions
- use %pe and ERR_PTR() to print symbolic errors
- add signed-off-by from Manikanta as the original author
- add myself as author after significantly modifying the driver
pcie: remove pre-silicon conditionals
---
drivers/pci/controller/Kconfig | 10 +-
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-tegra264.c | 538 +++++++++++++++++++++++++++++++++
3 files changed, 548 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 2247709ef6d6..3045c8aecc7e 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -255,7 +255,15 @@ config PCI_TEGRA
select IRQ_MSI_LIB
help
Say Y here if you want support for the PCIe host controller found
- on NVIDIA Tegra SoCs.
+ on NVIDIA Tegra SoCs (Tegra20 through Tegra186).
+
+config PCIE_TEGRA264
+ tristate "NVIDIA Tegra264 PCIe controller"
+ depends on ARCH_TEGRA || COMPILE_TEST
+ select PCI_ECAM
+ help
+ Say Y here if you want support for the PCIe host controller found
+ on NVIDIA Tegra264 SoCs.
config PCIE_RCAR_HOST
bool "Renesas R-Car PCIe controller (host mode)"
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index ac8db283f0fe..d478743b5142 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PCI_HYPERV_INTERFACE) += pci-hyperv-intf.o
obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
+obj-$(CONFIG_PCIE_TEGRA264) += pcie-tegra264.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar.o pcie-rcar-host.o
obj-$(CONFIG_PCIE_RCAR_EP) += pcie-rcar.o pcie-rcar-ep.o
diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
new file mode 100644
index 000000000000..e2d295ea4403
--- /dev/null
+++ b/drivers/pci/controller/pcie-tegra264.c
@@ -0,0 +1,538 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Tegra264 SoC
+ *
+ * Copyright (c) 2022-2026, NVIDIA CORPORATION. All rights reserved.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/interconnect.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/pci-ecam.h>
+#include <linux/pci.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_wakeirq.h>
+
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
+#include <soc/tegra/fuse.h>
+
+#include "../pci.h"
+
+/* XAL registers */
+#define XAL_RC_ECAM_BASE_HI 0x00
+#define XAL_RC_ECAM_BASE_LO 0x04
+#define XAL_RC_ECAM_BUSMASK 0x08
+#define XAL_RC_IO_BASE_HI 0x0c
+#define XAL_RC_IO_BASE_LO 0x10
+#define XAL_RC_IO_LIMIT_HI 0x14
+#define XAL_RC_IO_LIMIT_LO 0x18
+#define XAL_RC_MEM_32BIT_BASE_HI 0x1c
+#define XAL_RC_MEM_32BIT_BASE_LO 0x20
+#define XAL_RC_MEM_32BIT_LIMIT_HI 0x24
+#define XAL_RC_MEM_32BIT_LIMIT_LO 0x28
+#define XAL_RC_MEM_64BIT_BASE_HI 0x2c
+#define XAL_RC_MEM_64BIT_BASE_LO 0x30
+#define XAL_RC_MEM_64BIT_LIMIT_HI 0x34
+#define XAL_RC_MEM_64BIT_LIMIT_LO 0x38
+#define XAL_RC_BAR_CNTL_STANDARD 0x40
+#define XAL_RC_BAR_CNTL_STANDARD_IOBAR_EN BIT(0)
+#define XAL_RC_BAR_CNTL_STANDARD_32B_BAR_EN BIT(1)
+#define XAL_RC_BAR_CNTL_STANDARD_64B_BAR_EN BIT(2)
+
+/* XTL registers */
+#define XTL_RC_PCIE_CFG_LINK_STATUS 0x5a
+
+#define XTL_RC_MGMT_PERST_CONTROL 0x218
+#define XTL_RC_MGMT_PERST_CONTROL_PERST_O_N BIT(0)
+
+#define XTL_RC_MGMT_CLOCK_CONTROL 0x47c
+#define XTL_RC_MGMT_CLOCK_CONTROL_PEX_CLKREQ_I_N_PIN_USE_CONV_TO_PRSNT BIT(9)
+
+struct tegra264_pcie {
+ struct device *dev;
+
+ /* I/O memory */
+ void __iomem *xal;
+ void __iomem *xtl;
+ void __iomem *ecam;
+
+ /* bridge configuration */
+ struct pci_config_window *cfg;
+ struct pci_host_bridge *bridge;
+
+ /* wake IRQ */
+ struct gpio_desc *wake_gpio;
+ unsigned int wake_irq;
+
+ /* BPMP and bandwidth management */
+ struct icc_path *icc_path;
+ struct tegra_bpmp *bpmp;
+ u32 ctl_id;
+
+ bool link_up;
+};
+
+static int tegra264_pcie_parse_dt(struct tegra264_pcie *pcie)
+{
+ struct device *dev = pcie->dev;
+ int err;
+
+ pcie->wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
+ if (IS_ERR(pcie->wake_gpio))
+ return PTR_ERR(pcie->wake_gpio);
+
+ if (!pcie->wake_gpio)
+ return 0;
+
+ err = gpiod_to_irq(pcie->wake_gpio);
+ if (err < 0)
+ return dev_err_probe(dev, err, "failed to get wake IRQ\n");
+
+ pcie->wake_irq = (unsigned int)err;
+
+ err = devm_device_init_wakeup(dev);
+ if (err < 0)
+ return dev_err_probe(dev, err, "failed to initialize wakeup\n");
+
+ err = devm_pm_set_wake_irq(dev, pcie->wake_irq);
+ if (err < 0)
+ return dev_err_probe(dev, err, "failed to set wakeup IRQ\n");
+
+ return 0;
+}
+
+static void tegra264_pcie_power_off(struct tegra264_pcie *pcie)
+{
+ struct tegra_bpmp_message msg = {};
+ struct mrq_pcie_request req = {};
+ int err;
+
+ req.cmd = CMD_PCIE_RP_CONTROLLER_OFF;
+ req.rp_ctrlr_off.rp_controller = pcie->ctl_id;
+
+ msg.mrq = MRQ_PCIE;
+ msg.tx.data = &req;
+ msg.tx.size = sizeof(req);
+
+ err = tegra_bpmp_transfer(pcie->bpmp, &msg);
+ if (err)
+ dev_err(pcie->dev, "failed to turn off PCIe #%u: %pe\n",
+ pcie->ctl_id, ERR_PTR(err));
+
+ if (msg.rx.ret)
+ dev_err(pcie->dev, "failed to turn off PCIe #%u: %d\n",
+ pcie->ctl_id, msg.rx.ret);
+}
+
+static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
+{
+ u32 value, speed, width;
+ int err;
+
+ value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
+ speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
+
+ value = Mbps_to_icc(width * PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]));
+
+ /*
+ * We don't want to error out here because a boot-critical device
+ * could be connected to this root port. Failure to set the bandwidth
+ * request may have an adverse impact on performance, but it is not
+ * generally fatal, so we opt to continue regardless so that users
+ * get a chance to fix things.
+ */
+ err = icc_set_bw(pcie->icc_path, value, value);
+ if (err < 0)
+ dev_err(pcie->dev,
+ "failed to request bandwidth (%u MBps): %pe\n",
+ value, ERR_PTR(err));
+}
+
+/*
+ * The various memory regions used by the controller (I/O, memory, ECAM) are
+ * set up during early boot and have hardware-level protections in place. If
+ * the DT ranges don't match what's been setup, the controller won't be able
+ * to write the address endpoints properly, so make sure to validate that DT
+ * and firmware programming agree on these ranges.
+ */
+static bool tegra264_pcie_check_ranges(struct platform_device *pdev)
+{
+ struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
+ struct device_node *np = pcie->dev->of_node;
+ struct of_pci_range_parser parser;
+ phys_addr_t phys, limit, hi, lo;
+ struct of_pci_range range;
+ struct resource *res;
+ bool status = true;
+ u32 value;
+ int err;
+
+ err = of_pci_range_parser_init(&parser, np);
+ if (err < 0)
+ return false;
+
+ for_each_of_pci_range(&parser, &range) {
+ unsigned int addr_hi, addr_lo, limit_hi, limit_lo, enable;
+ unsigned long type = range.flags & IORESOURCE_TYPE_BITS;
+ phys_addr_t start, end, mask;
+ const char *region = NULL;
+
+ end = range.cpu_addr + range.size - 1;
+ start = range.cpu_addr;
+
+ switch (type) {
+ case IORESOURCE_IO:
+ addr_hi = XAL_RC_IO_BASE_HI;
+ addr_lo = XAL_RC_IO_BASE_LO;
+ limit_hi = XAL_RC_IO_LIMIT_HI;
+ limit_lo = XAL_RC_IO_LIMIT_LO;
+ enable = XAL_RC_BAR_CNTL_STANDARD_IOBAR_EN;
+ mask = SZ_64K - 1;
+ region = "I/O";
+ break;
+
+ case IORESOURCE_MEM:
+ if (range.flags & IORESOURCE_PREFETCH) {
+ addr_hi = XAL_RC_MEM_64BIT_BASE_HI;
+ addr_lo = XAL_RC_MEM_64BIT_BASE_LO;
+ limit_hi = XAL_RC_MEM_64BIT_LIMIT_HI;
+ limit_lo = XAL_RC_MEM_64BIT_LIMIT_LO;
+ enable = XAL_RC_BAR_CNTL_STANDARD_64B_BAR_EN;
+ region = "prefetchable memory";
+ } else {
+ addr_hi = XAL_RC_MEM_32BIT_BASE_HI;
+ addr_lo = XAL_RC_MEM_32BIT_BASE_LO;
+ limit_hi = XAL_RC_MEM_32BIT_LIMIT_HI;
+ limit_lo = XAL_RC_MEM_32BIT_LIMIT_LO;
+ enable = XAL_RC_BAR_CNTL_STANDARD_32B_BAR_EN;
+ region = "memory";
+ }
+
+ mask = SZ_1M - 1;
+ break;
+ }
+
+ /* not interested in anything that's not I/O or memory */
+ if (!region)
+ continue;
+
+ /* don't check regions that haven't been enabled */
+ value = readl(pcie->xal + XAL_RC_BAR_CNTL_STANDARD);
+ if ((value & enable) == 0)
+ continue;
+
+ hi = readl(pcie->xal + addr_hi);
+ lo = readl(pcie->xal + addr_lo);
+ phys = ((hi << 16) << 16) | lo;
+
+ hi = readl(pcie->xal + limit_hi);
+ lo = readl(pcie->xal + limit_lo);
+ limit = ((hi << 16) << 16) | lo | mask;
+
+ if (phys != start || limit != end) {
+ dev_err(pcie->dev,
+ "%s region mismatch: %pap-%pap -> %pap-%pap\n",
+ region, &phys, &limit, &start, &end);
+ status = false;
+ }
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam");
+ if (!res)
+ return false;
+
+ hi = readl(pcie->xal + XAL_RC_ECAM_BASE_HI);
+ lo = readl(pcie->xal + XAL_RC_ECAM_BASE_LO);
+ phys = ((hi << 16) << 16) | lo;
+
+ value = readl(pcie->xal + XAL_RC_ECAM_BUSMASK);
+ limit = phys + ((value + 1) << 20) - 1;
+
+ if (phys != res->start || limit != res->end) {
+ dev_err(pcie->dev,
+ "ECAM region mismatch: %pap-%pap -> %pap-%pap\n",
+ &phys, &limit, &res->start, &res->end);
+ status = false;
+ }
+
+ return status;
+}
+
+static bool tegra264_pcie_link_up(struct tegra264_pcie *pcie,
+ enum pci_bus_speed *speed)
+{
+ u16 value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
+
+ if (value & PCI_EXP_LNKSTA_DLLLA) {
+ if (speed)
+ *speed = pcie_link_speed[FIELD_GET(PCI_EXP_LNKSTA_CLS,
+ value)];
+
+ return true;
+ }
+
+ return false;
+}
+
+static void tegra264_pcie_init(struct tegra264_pcie *pcie)
+{
+ enum pci_bus_speed speed;
+ unsigned int i;
+ u32 value;
+
+ /* bring the endpoint out of reset */
+ value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+ value |= XTL_RC_MGMT_PERST_CONTROL_PERST_O_N;
+ writel(value, pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+
+ for (i = 0; i < PCIE_LINK_WAIT_MAX_RETRIES; i++) {
+ if (tegra264_pcie_link_up(pcie, NULL))
+ break;
+
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
+ }
+
+ if (tegra264_pcie_link_up(pcie, &speed)) {
+ msleep(PCIE_RESET_CONFIG_WAIT_MS);
+ dev_info(pcie->dev, "PCIe #%u link is up (speed: %s)\n",
+ pcie->ctl_id, pci_speed_string(speed));
+ tegra264_pcie_icc_set(pcie);
+ pcie->link_up = true;
+ } else {
+ dev_info(pcie->dev, "PCIe #%u link is down\n", pcie->ctl_id);
+
+ value = readl(pcie->xtl + XTL_RC_MGMT_CLOCK_CONTROL);
+
+ /*
+ * Set link state only when link fails and no hot-plug feature
+ * is present.
+ */
+ if ((value & XTL_RC_MGMT_CLOCK_CONTROL_PEX_CLKREQ_I_N_PIN_USE_CONV_TO_PRSNT) == 0) {
+ dev_info(pcie->dev,
+ "PCIe #%u link is down and not hotplug-capable, turning off\n",
+ pcie->ctl_id);
+ tegra264_pcie_power_off(pcie);
+ pcie->link_up = false;
+ } else {
+ pcie->link_up = true;
+ }
+ }
+}
+
+static void tegra264_pcie_deinit(struct tegra264_pcie *pcie)
+{
+ u32 value;
+
+ /* take the endpoint into reset */
+ value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+ value &= ~XTL_RC_MGMT_PERST_CONTROL_PERST_O_N;
+ writel(value, pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+}
+
+static int tegra264_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pci_host_bridge *bridge;
+ struct tegra264_pcie *pcie;
+ struct resource_entry *bus;
+ struct resource *res;
+ int err;
+
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct tegra264_pcie));
+ if (!bridge)
+ return dev_err_probe(dev, -ENOMEM,
+ "failed to allocate host bridge\n");
+
+ pcie = pci_host_bridge_priv(bridge);
+ platform_set_drvdata(pdev, pcie);
+ pcie->bridge = bridge;
+ pcie->dev = dev;
+
+ err = tegra264_pcie_parse_dt(pcie);
+ if (err < 0)
+ return dev_err_probe(dev, err, "failed to parse device tree\n");
+
+ pcie->xal = devm_platform_ioremap_resource_byname(pdev, "xal");
+ if (IS_ERR(pcie->xal))
+ return dev_err_probe(dev, PTR_ERR(pcie->xal),
+ "failed to map XAL memory\n");
+
+ pcie->xtl = devm_platform_ioremap_resource_byname(pdev, "xtl-pri");
+ if (IS_ERR(pcie->xtl))
+ return dev_err_probe(dev, PTR_ERR(pcie->xtl),
+ "failed to map XTL-PRI memory\n");
+
+ bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
+ if (!bus)
+ return dev_err_probe(dev, -ENODEV,
+ "failed to get bus resources\n");
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam");
+ if (!res)
+ return dev_err_probe(dev, -ENXIO,
+ "failed to get ECAM resource\n");
+
+ pcie->icc_path = devm_of_icc_get(dev, "write");
+ if (IS_ERR(pcie->icc_path))
+ return dev_err_probe(dev, PTR_ERR(pcie->icc_path),
+ "failed to get ICC\n");
+
+ pcie->bpmp = tegra_bpmp_get_with_id(dev, &pcie->ctl_id);
+ if (IS_ERR(pcie->bpmp))
+ return dev_err_probe(dev, PTR_ERR(pcie->bpmp),
+ "failed to get BPMP\n");
+
+ err = devm_pm_runtime_set_active_enabled(dev);
+ if (err < 0) {
+ dev_err_probe(dev, err, "failed to enable runtime PM\n");
+ goto put_bpmp;
+ }
+
+ err = pm_runtime_get_sync(dev);
+ if (err < 0) {
+ dev_err_probe(dev, err, "failed to power on device\n");
+ goto put_bpmp;
+ }
+
+ /* sanity check that programmed ranges match what's in DT */
+ if (!tegra264_pcie_check_ranges(pdev)) {
+ err = -EINVAL;
+ goto put_pm;
+ }
+
+ pcie->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
+ if (IS_ERR(pcie->cfg)) {
+ err = dev_err_probe(dev, PTR_ERR(pcie->cfg),
+ "failed to create ECAM\n");
+ goto put_pm;
+ }
+
+ bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
+ bridge->sysdata = pcie->cfg;
+ pcie->ecam = pcie->cfg->win;
+
+ tegra264_pcie_init(pcie);
+
+ if (!pcie->link_up)
+ return 0;
+
+ err = pci_host_probe(bridge);
+ if (err < 0) {
+ dev_err_probe(dev, err, "failed to register host\n");
+ goto free_ecam;
+ }
+
+ return 0;
+
+free_ecam:
+ pci_ecam_free(pcie->cfg);
+put_pm:
+ pm_runtime_put_sync(dev);
+put_bpmp:
+ tegra_bpmp_put(pcie->bpmp);
+
+ return err;
+}
+
+static void tegra264_pcie_remove(struct platform_device *pdev)
+{
+ struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
+
+ /*
+ * If we undo tegra264_pcie_init() then link goes down and need
+ * controller reset to bring up the link again. Remove intention is
+ * to clean up the root bridge and re-enumerate during bind.
+ */
+ pci_lock_rescan_remove();
+ pci_stop_root_bus(pcie->bridge->bus);
+ pci_remove_root_bus(pcie->bridge->bus);
+ pci_unlock_rescan_remove();
+
+ pm_runtime_put_sync(&pdev->dev);
+ tegra_bpmp_put(pcie->bpmp);
+ pci_ecam_free(pcie->cfg);
+}
+
+static int tegra264_pcie_suspend(struct device *dev)
+{
+ struct tegra264_pcie *pcie = dev_get_drvdata(dev);
+ int err;
+
+ tegra264_pcie_deinit(pcie);
+
+ if (pcie->wake_gpio && device_may_wakeup(dev)) {
+ err = enable_irq_wake(pcie->wake_irq);
+ if (err < 0)
+ dev_err(dev, "failed to enable wake IRQ: %pe\n",
+ ERR_PTR(err));
+ }
+
+ return 0;
+}
+
+static int tegra264_pcie_resume(struct device *dev)
+{
+ struct tegra264_pcie *pcie = dev_get_drvdata(dev);
+ int err;
+
+ err = pinctrl_pm_select_default_state(dev);
+ if (err < 0)
+ dev_err(dev, "failed to configure sideband pins: %pe\n",
+ ERR_PTR(err));
+
+ if (pcie->wake_gpio && device_may_wakeup(dev)) {
+ err = disable_irq_wake(pcie->wake_irq);
+ if (err < 0)
+ dev_err(dev, "failed to disable wake IRQ: %pe\n",
+ ERR_PTR(err));
+ }
+
+ if (pcie->link_up == false)
+ return 0;
+
+ tegra264_pcie_init(pcie);
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(tegra264_pcie_pm_ops,
+ tegra264_pcie_suspend,
+ tegra264_pcie_resume);
+
+static const struct of_device_id tegra264_pcie_of_match[] = {
+ {
+ .compatible = "nvidia,tegra264-pcie",
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tegra264_pcie_of_match);
+
+static struct platform_driver tegra264_pcie_driver = {
+ .probe = tegra264_pcie_probe,
+ .remove = tegra264_pcie_remove,
+ .driver = {
+ .name = "tegra264-pcie",
+ .pm = pm_ptr(&tegra264_pcie_pm_ops),
+ .of_match_table = tegra264_pcie_of_match,
+ },
+};
+module_platform_driver(tegra264_pcie_driver);
+
+MODULE_AUTHOR("Manikanta Maddireddy <mmaddireddy@nvidia.com>");
+MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
+MODULE_DESCRIPTION("NVIDIA Tegra264 PCIe host controller driver");
+MODULE_LICENSE("GPL");
--
2.54.0
^ permalink raw reply related
* [PATCH v7 2/4] PCI: Use standard wait times for PCIe link monitoring
From: Thierry Reding @ 2026-06-17 16:01 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter, Karthikeyan Mitran,
Hou Zhiqiang, Thomas Petazzoni, Pali Rohár, Michal Simek,
Kevin Xie, Thierry Reding, Aksh Garg
Cc: linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding
In-Reply-To: <20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com>
From: Thierry Reding <treding@nvidia.com>
Instead of defining the wait values for each driver, use common values
defined in the core pci.h header file. Note that while most drivers use
the usleep_range(), it looks like these were mostly cargo culted and
msleep() is a better choice given the fixed delay that the specification
calls for. Convert all drivers to msleep() and use the existing
definition.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v7:
- rebase on top of next-20260615 (resolve pci-aardvark.c conflict)
Changes in v6:
- convert all drivers to use msleep() (Lukas Wunner)
Changes in v2:
- fix build for Cadence
---
drivers/pci/controller/cadence/pcie-cadence-host-common.c | 6 ++++--
drivers/pci/controller/cadence/pcie-cadence-lga-regs.h | 5 -----
drivers/pci/controller/mobiveil/pcie-mobiveil.c | 4 ++--
drivers/pci/controller/mobiveil/pcie-mobiveil.h | 5 -----
drivers/pci/controller/pci-aardvark.c | 7 ++-----
drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++-------
drivers/pci/controller/plda/pcie-starfive.c | 9 ++-------
7 files changed, 12 insertions(+), 33 deletions(-)
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
index 18e4b6c760b5..0ef4396151b4 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
@@ -16,6 +16,8 @@
#include "pcie-cadence-host-common.h"
#include "../pci-host-common.h"
+#include "../../pci.h"
+
#define LINK_RETRAIN_TIMEOUT HZ
u64 bar_max_size[] = {
@@ -54,12 +56,12 @@ int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie,
int retries;
/* Check if the link is up or not */
- for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+ for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
if (pcie_link_up(pcie)) {
dev_info(dev, "Link up\n");
return 0;
}
- usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
}
return -ETIMEDOUT;
diff --git a/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h b/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
index 857b2140c5d2..15dc4fcaf45d 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
+++ b/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
@@ -10,11 +10,6 @@
#include <linux/bitfield.h>
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_USLEEP_MIN 90000
-#define LINK_WAIT_USLEEP_MAX 100000
-
/* Local Management Registers */
#define CDNS_PCIE_LM_BASE 0x00100000
diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.c b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
index 62ecbaeb0a60..e8346851c49b 100644
--- a/drivers/pci/controller/mobiveil/pcie-mobiveil.c
+++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
@@ -218,11 +218,11 @@ int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
int retries;
/* check if the link is up or not */
- for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+ for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
if (mobiveil_pcie_link_up(pcie))
return 0;
- usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX);
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
}
dev_err(&pcie->pdev->dev, "link never came up\n");
diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
index 7246de6a7176..11010a99e27c 100644
--- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h
+++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
@@ -122,11 +122,6 @@
#define IB_WIN_SIZE ((u64)256 * 1024 * 1024 * 1024)
#define MAX_PIO_WINDOWS 8
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_MIN 90000
-#define LINK_WAIT_MAX 100000
-
#define PAGED_ADDR_BNDRY 0xc00
#define OFFSET_TO_PAGE_ADDR(off) \
((off & PAGE_LO_MASK) | PAGED_ADDR_BNDRY)
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index fd9c7d53e8a7..272c5c8fc1e5 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -256,9 +256,6 @@ enum {
#define PIO_RETRY_CNT 750000 /* 1.5 s */
#define PIO_RETRY_DELAY 2 /* 2 us*/
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_USLEEP_MIN 90000
-#define LINK_WAIT_USLEEP_MAX 100000
#define RETRAIN_WAIT_MAX_RETRIES 10
#define RETRAIN_WAIT_USLEEP_US 2000
@@ -350,13 +347,13 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
int retries;
/* check if the link is up or not */
- for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+ for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
if (advk_pcie_link_up(pcie)) {
pci_host_common_link_train_delay(pcie->link_gen);
return 0;
}
- usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
}
return -ETIMEDOUT;
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 7db2c96c6cec..0dee19fa24ca 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -140,11 +140,6 @@
#define PCIE_PHY_LINKUP_BIT BIT(0)
#define PHY_RDY_LINKUP_BIT BIT(1)
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_USLEEP_MIN 90000
-#define LINK_WAIT_USLEEP_MAX 100000
-
struct nwl_msi { /* MSI information */
DECLARE_BITMAP(bitmap, INT_PCI_MSI_NR);
struct irq_domain *dev_domain;
@@ -203,10 +198,10 @@ static int nwl_wait_for_link(struct nwl_pcie *pcie)
int retries;
/* check if the link is up or not */
- for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+ for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
if (nwl_phy_link_up(pcie))
return 0;
- usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
}
dev_err(dev, "PHY link never came up\n");
diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
index 298036c3e7f9..2835c7af965e 100644
--- a/drivers/pci/controller/plda/pcie-starfive.c
+++ b/drivers/pci/controller/plda/pcie-starfive.c
@@ -45,11 +45,6 @@
#define STG_SYSCON_LNKSTA_OFFSET 0x170
#define DATA_LINK_ACTIVE BIT(5)
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_USLEEP_MIN 90000
-#define LINK_WAIT_USLEEP_MAX 100000
-
struct starfive_jh7110_pcie {
struct plda_pcie_rp plda;
struct reset_control *resets;
@@ -217,12 +212,12 @@ static int starfive_pcie_host_wait_for_link(struct starfive_jh7110_pcie *pcie)
int retries;
/* Check if the link is up or not */
- for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+ for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
if (starfive_pcie_link_up(&pcie->plda)) {
dev_info(pcie->plda.dev, "port link up\n");
return 0;
}
- usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+ msleep(PCIE_LINK_WAIT_SLEEP_MS);
}
return -ETIMEDOUT;
--
2.54.0
^ permalink raw reply related
* [PATCH v7 1/4] dt-bindings: pci: Strictly distinguish C0 from C1-C5
From: Thierry Reding @ 2026-06-17 16:01 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter, Karthikeyan Mitran,
Hou Zhiqiang, Thomas Petazzoni, Pali Rohár, Michal Simek,
Kevin Xie, Thierry Reding, Aksh Garg
Cc: linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding
In-Reply-To: <20260617-tegra264-pcie-v7-0-eae7ae964629@nvidia.com>
From: Thierry Reding <treding@nvidia.com>
Instead of using the ECAM registers as the first entry, strictly make a
distinction between C0 and C1-C5. This is needed because otherwise the
unit address doesn't match the first "reg" entry. We also cannot change
the ordering of these nodes to follow the ECAM addresses because that
would put them outside of their "control bus" hierarchy since the ECAM
address space is a global one outside of any of the control busses.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v7:
- undo changes suggested by Sashiko, should've trust the dedicated tool
rather than the AI
Changes in v6:
- add maxItems as suggested by Sashiko
Changes in v5:
- rebase on top of v7.1-rc1, make it into a fix
Changes in v4:
- ECAM is outside of the controller's region, so it cannot be the first
reg entry, otherwise we get warnings because it doesn't match the
unit-address, so revert back to oneOf construct
Changes in v2:
- move ECAM region first and unify C0 vs. C1-C5
- move unevaluatedProperties to right before the examples
- add description to clarify the two types of controllers
- add examples for C0 and C1-C5
---
.../bindings/pci/nvidia,tegra264-pcie.yaml | 75 ++++++++++++++--------
1 file changed, 50 insertions(+), 25 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
index dc4f8725c9f5..acb677d477fb 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
@@ -10,32 +10,23 @@ maintainers:
- Thierry Reding <thierry.reding@gmail.com>
- Jon Hunter <jonathanh@nvidia.com>
+description: |
+ Of the six PCIe controllers found on Tegra264, one (C0) is used for the
+ internal GPU and the other five (C1-C5) are routed to connectors such as
+ PCI or M.2 slots. Therefore the UPHY registers (XPL) exist only for C1
+ through C5, but not for C0.
+
properties:
compatible:
const: nvidia,tegra264-pcie
reg:
- description: |
- Of the six PCIe controllers found on Tegra264, one (C0) is used for the
- internal GPU and the other five (C1-C5) are routed to connectors such as
- PCI or M.2 slots. Therefore the UPHY registers (XPL) exist only for C1
- through C5, but not for C0.
minItems: 4
- items:
- - description: ECAM-compatible configuration space
- - description: application layer registers
- - description: transaction layer registers
- - description: privileged transaction layer registers
- - description: data link/physical layer registers (not available on C0)
+ maxItems: 5
reg-names:
minItems: 4
- items:
- - const: ecam
- - const: xal
- - const: xtl
- - const: xtl-pri
- - const: xpl
+ maxItems: 5
interrupts:
minItems: 1
@@ -70,6 +61,40 @@ required:
allOf:
- $ref: /schemas/pci/pci-host-bridge.yaml#
+ - oneOf:
+ - description: C0 controller (no UPHY)
+ properties:
+ reg:
+ items:
+ - description: application layer registers
+ - description: transaction layer registers
+ - description: privileged transaction layer registers
+ - description: ECAM compatible configuration space
+
+ reg-names:
+ items:
+ - const: xal
+ - const: xtl
+ - const: xtl-pri
+ - const: ecam
+
+ - description: C1-C5 controllers (with UPHY)
+ properties:
+ reg:
+ items:
+ - description: application layer registers
+ - description: transaction layer registers
+ - description: privileged transaction layer registers
+ - description: data link/physical layer registers
+ - description: ECAM compatible configuration space
+
+ reg-names:
+ items:
+ - const: xal
+ - const: xtl
+ - const: xtl-pri
+ - const: xpl
+ - const: ecam
unevaluatedProperties: false
@@ -81,11 +106,11 @@ examples:
pci@c000000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xd0 0xb0000000 0x0 0x10000000>,
- <0x00 0x0c000000 0x0 0x00004000>,
+ reg = <0x00 0x0c000000 0x0 0x00004000>,
<0x00 0x0c004000 0x0 0x00001000>,
- <0x00 0x0c005000 0x0 0x00001000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri";
+ <0x00 0x0c005000 0x0 0x00001000>,
+ <0xd0 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -118,12 +143,12 @@ examples:
pci@8400000 {
compatible = "nvidia,tegra264-pcie";
- reg = <0xa8 0xb0000000 0x0 0x10000000>,
- <0x00 0x08400000 0x0 0x00004000>,
+ reg = <0x00 0x08400000 0x0 0x00004000>,
<0x00 0x08404000 0x0 0x00001000>,
<0x00 0x08405000 0x0 0x00001000>,
- <0x00 0x08410000 0x0 0x00010000>;
- reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+ <0x00 0x08410000 0x0 0x00010000>,
+ <0xa8 0xb0000000 0x0 0x10000000>;
+ reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
--
2.54.0
^ permalink raw reply related
* [PATCH v7 0/4] PCI: tegra: Add Tegra264 support
From: Thierry Reding @ 2026-06-17 16:01 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter, Karthikeyan Mitran,
Hou Zhiqiang, Thomas Petazzoni, Pali Rohár, Michal Simek,
Kevin Xie, Thierry Reding, Aksh Garg
Cc: linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding, Manikanta Maddireddy
Hi,
this series adds support for the PCIe controllers found on the Tegra264
SoC. There are six instances, one of which is for internal purposes only
and the other five are general purpose.
The first patch tweaks the DT bindings slightly to avoid new DT compiler
warnings that slipped through because they are now disabled by default
(-Wno-unit_address_vs_reg).
Before adding the driver in patch 3, patch 2 introduces some new common
wait times for PCIe and unifies the way that drivers use them. Finally,
patch 4 reorders the reg and reg-names property entries to match the
bindings changes from patch 1.
All of the prerequisites were merged in v7.1-rc1, so this can be applied
to the PCI tree directly. Optionally I can also pick up patch 4 into the
Tegra tree, but there should be no conflicts, so feel free to pick this
up with the rest.
Thanks,
Thierry
Changes in v7:
- fix build dependency on PCI_ECAM
- remove pre-silicon support code
- Link to v6: https://patch.msgid.link/20260602-tegra264-pcie-v6-0-edbcfa7a78fe@nvidia.com
Changes in v6:
- address review comments from Sashiko
- rebase onto v7.1-rc1, adjust DT bindings patch accordingly
- Link to v5: https://patch.msgid.link/20260526-tegra264-pcie-v5-0-84a813b979d7@nvidia.com
Changes in v5:
- address review comments for the PCI driver patch
- Link to v4: https://patch.msgid.link/20260402-tegra264-pcie-v4-0-21e2e19987e8@nvidia.com
Changes in v4:
- strip out dependencies that are going in through the ARM SoC tree
- revert bindings to oneOf construct so that we don't produce new DTC
warnings
- Link to v3: https://patch.msgid.link/20260326135855.2795149-1-thierry.reding@kernel.org
Changes in v3:
- integrate PCI standard wait times patch into the series to maintain
bisectability
- fix review comments from Mikko
- Link to v2: https://patch.msgid.link/20260320225443.2571920-1-thierry.reding@kernel.org
Changes in v2:
- fix an issue with sanity-checking disabled BARs
- address review comments
- Link to v1: https://patch.msgid.link/20260319160110.2131954-1-thierry.reding@kernel.org
Thanks,
Thierry
---
Thierry Reding (4):
dt-bindings: pci: Strictly distinguish C0 from C1-C5
PCI: Use standard wait times for PCIe link monitoring
PCI: tegra: Add Tegra264 support
arm64: tegra: Reorder reg and reg-names to match bindings
.../bindings/pci/nvidia,tegra264-pcie.yaml | 75 ++-
arch/arm64/boot/dts/nvidia/tegra264.dtsi | 48 +-
drivers/pci/controller/Kconfig | 10 +-
drivers/pci/controller/Makefile | 1 +
.../controller/cadence/pcie-cadence-host-common.c | 6 +-
.../pci/controller/cadence/pcie-cadence-lga-regs.h | 5 -
drivers/pci/controller/mobiveil/pcie-mobiveil.c | 4 +-
drivers/pci/controller/mobiveil/pcie-mobiveil.h | 5 -
drivers/pci/controller/pci-aardvark.c | 7 +-
drivers/pci/controller/pcie-tegra264.c | 538 +++++++++++++++++++++
drivers/pci/controller/pcie-xilinx-nwl.c | 9 +-
drivers/pci/controller/plda/pcie-starfive.c | 9 +-
12 files changed, 634 insertions(+), 83 deletions(-)
---
base-commit: 8f5b04d01f6fbbb5537a0979182acf820766660d
change-id: 20260402-tegra264-pcie-e30abe23da07
Best regards,
--
Thierry Reding <treding@nvidia.com>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: clock: ultrarisc: Add DP1000 Clock Controller
From: Conor Dooley @ 2026-06-17 15:56 UTC (permalink / raw)
To: wangjia
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-clk, devicetree,
linux-kernel
In-Reply-To: <20260617-ultrarisc-clock-v2-1-9cb16083e15e@ultrarisc.com>
[-- Attachment #1: Type: text/plain, Size: 4541 bytes --]
On Wed, Jun 17, 2026 at 02:02:54PM +0800, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
>
> Add doc for the clock controller on the UltraRISC DP1000 RISC-V SoC.
>
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
Cheers,
Conor.
> ---
> .../bindings/clock/ultrarisc,dp1000-clk.yaml | 60 ++++++++++++++++++++++
> MAINTAINERS | 7 +++
> include/dt-bindings/clock/ultrarisc,dp1000-clk.h | 27 ++++++++++
> 3 files changed, 94 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml b/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
> new file mode 100644
> index 000000000000..ede565ec440c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ultrarisc,dp1000-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: UltraRISC DP1000 Clock Controller
> +
> +maintainers:
> + - Jia Wang <wangjia@ultrarisc.com>
> +
> +description: |
> + The UltraRISC DP1000 clock controller is driven from a single external
> + oscillator input. It provides a system PLL with fractional multiplier
> + and post-divider stages, several fixed-ratio derived clocks for
> + the on-chip subsystem, Clock Configuration Register (CCR) divider
> + outputs for GMAC and the UART, I2C, and SPI root clocks, and
> + per-instance gate clocks for UART0-3, I2C0-3, and SPI0-1.
> +
> + All available clocks are defined as preprocessor macros in
> + include/dt-bindings/clock/ultrarisc,dp1000-clk.h
> +
> +properties:
> + compatible:
> + const: ultrarisc,dp1000-clk
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description:
> + External oscillator input clock used as the parent of the PLLs.
> +
> + "#clock-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - "#clock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/ultrarisc,dp1000-clk.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + clock-controller@11080000 {
> + compatible = "ultrarisc,dp1000-clk";
> + reg = <0x0 0x11080000 0x0 0x1000>;
> + clocks = <&osc>;
> + #clock-cells = <1>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d6db8cb608f..b7e43313c65f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27439,6 +27439,13 @@ S: Maintained
> F: drivers/usb/common/ulpi.c
> F: include/linux/ulpi/
>
> +ULTRARISC DP1000 CLOCK DRIVER
> +M: Jia Wang <wangjia@ultrarisc.com>
> +L: linux-clk@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
> +F: include/dt-bindings/clock/ultrarisc,dp1000-clk.h
> +
> ULTRATRONIK BOARD SUPPORT
> M: Goran Rađenović <goran.radni@gmail.com>
> M: Börge Strümpfel <boerge.struempfel@gmail.com>
> diff --git a/include/dt-bindings/clock/ultrarisc,dp1000-clk.h b/include/dt-bindings/clock/ultrarisc,dp1000-clk.h
> new file mode 100644
> index 000000000000..751125f99965
> --- /dev/null
> +++ b/include/dt-bindings/clock/ultrarisc,dp1000-clk.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +#ifndef _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H
> +#define _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H
> +
> +#define DP1000_CLK_SYSPLL 0
> +#define DP1000_CLK_SYSPLL_DIV2 1
> +#define DP1000_CLK_SUBSYS 2
> +#define DP1000_CLK_GMAC 3
> +#define DP1000_CLK_UART_ROOT 4
> +#define DP1000_CLK_I2C_ROOT 5
> +#define DP1000_CLK_SPI_ROOT 6
> +#define DP1000_CLK_PCIE_DBI 7
> +#define DP1000_CLK_PCIEX4_CORE 8
> +#define DP1000_CLK_PCIEX16_CORE 9
> +#define DP1000_CLK_PCIE_AUX 10
> +#define DP1000_CLK_UART0 11
> +#define DP1000_CLK_UART1 12
> +#define DP1000_CLK_UART2 13
> +#define DP1000_CLK_UART3 14
> +#define DP1000_CLK_I2C0 15
> +#define DP1000_CLK_I2C1 16
> +#define DP1000_CLK_I2C2 17
> +#define DP1000_CLK_I2C3 18
> +#define DP1000_CLK_SPI0 19
> +#define DP1000_CLK_SPI1 20
> +
> +#endif /* _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H */
>
> --
> 2.34.1
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: Conor Dooley @ 2026-06-17 15:55 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang, cix-kernel-upstream, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260617060437.1474816-4-joakim.zhang@cixtech.com>
[-- Attachment #1: Type: text/plain, Size: 5626 bytes --]
On Wed, Jun 17, 2026 at 02:04:35PM +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> The AUDSS CRU contains an internal clock tree of muxes, dividers and
> gates for DSP, I2S, HDA, DMAC and related blocks. The clock provider is
> a child node of the cix,sky1-audss-system-control syscon and accesses
> registers through the parent MMIO region.
Why can this not just be part of the parent syscon node?
Cheers,
Conor.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> .../bindings/clock/cix,sky1-audss-clock.yaml | 72 +++++++++++++++++++
> .../dt-bindings/clock/cix,sky1-audss-clock.h | 60 ++++++++++++++++
> 2 files changed, 132 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
> create mode 100644 include/dt-bindings/clock/cix,sky1-audss-clock.h
>
> diff --git a/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
> new file mode 100644
> index 000000000000..ea813c5a2307
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/cix,sky1-audss-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cix Sky1 audio subsystem clock controller
> +
> +maintainers:
> + - Joakim Zhang <joakim.zhang@cixtech.com>
> +
> +description: |
> + Clock provider for the Cix Sky1 audio subsystem (AUDSS).
> +
> + This node is a child of a cix,sky1-audss-system-control syscon node
> + (see cix,sky1-system-control.yaml). It does not have a reg property; clock
> + mux, divider and gate fields are accessed through the parent register block.
> +
> + Software reset lines for AUDSS blocks are exposed on the parent syscon via
> + #reset-cells (provider). Reset indices are defined in
> + include/dt-bindings/reset/cix,sky1-audss-system-control.h.
> +
> + Four SoC-level reference clocks listed in clocks/clock-names feed the AUDSS
> + clock tree. The provider exposes the internal AUDSS clocks to other devices
> + via #clock-cells; indices are defined in cix,sky1-audss-clock.h.
> +
> + The parent cix,sky1-audss-system-control node describes the SoC syscon
> + NoC (or bus) reset via resets and the audio subsystem power domain via
> + power-domains.
> +
> +properties:
> + compatible:
> + const: cix,sky1-audss-clock
> +
> + '#clock-cells':
> + const: 1
> + description:
> + Clock indices are defined in include/dt-bindings/clock/cix,sky1-audss-clock.h.
> +
> + clocks:
> + items:
> + - description: I2S parent clock for sampling rates multiple of 8kHz.
> + - description: I2S parent clock for sampling rates multiple of 11.025kHz.
> + - description: clock feeding most devices in audss (NOC, DSP, SRAM, HDA, DMAC, I2S, and Mailbox).
> + - description: clock feeding for HDA, Timer and Watchdog, which is a delicated 48MHz clock.
> +
> + clock-names:
> + items:
> + - const: x8k
> + - const: x11k
> + - const: sys
> + - const: 48m
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/cix,sky1.h>
> +
> + clock-controller {
> + compatible = "cix,sky1-audss-clock";
> + #clock-cells = <1>;
> + clocks = <&scmi_clk CLK_TREE_AUDIO_CLK0>, <&scmi_clk CLK_TREE_AUDIO_CLK2>,
> + <&scmi_clk CLK_TREE_AUDIO_CLK4>, <&scmi_clk CLK_TREE_AUDIO_CLK5>;
> + clock-names = "x8k", "x11k", "sys", "48m";
> + };
> diff --git a/include/dt-bindings/clock/cix,sky1-audss-clock.h b/include/dt-bindings/clock/cix,sky1-audss-clock.h
> new file mode 100644
> index 000000000000..7e9bd3e6c7a1
> --- /dev/null
> +++ b/include/dt-bindings/clock/cix,sky1-audss-clock.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright 2026 Cix Technology Group Co., Ltd.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_CIX_SKY1_AUDSS_CLOCK_H
> +#define _DT_BINDINGS_CLK_CIX_SKY1_AUDSS_CLOCK_H
> +
> +#define CLK_AUD_CLK4_DIV2 0
> +#define CLK_AUD_CLK4_DIV4 1
> +#define CLK_AUD_CLK5_DIV2 2
> +
> +#define CLK_DSP_CLK 3
> +#define CLK_DSP_BCLK 4
> +#define CLK_DSP_PBCLK 5
> +
> +#define CLK_SRAM_AXI 6
> +
> +#define CLK_HDA_SYS 7
> +#define CLK_HDA_HDA 8
> +
> +#define CLK_DMAC_AXI 9
> +
> +#define CLK_WDG_APB 10
> +#define CLK_WDG_WDG 11
> +
> +#define CLK_TIMER_APB 12
> +#define CLK_TIMER_TIMER 13
> +
> +#define CLK_MB_0_APB 14 /* MB0: ap->dsp */
> +#define CLK_MB_1_APB 15 /* MB1: dsp->ap */
> +
> +#define CLK_I2S0_APB 16
> +#define CLK_I2S1_APB 17
> +#define CLK_I2S2_APB 18
> +#define CLK_I2S3_APB 19
> +#define CLK_I2S4_APB 20
> +#define CLK_I2S5_APB 21
> +#define CLK_I2S6_APB 22
> +#define CLK_I2S7_APB 23
> +#define CLK_I2S8_APB 24
> +#define CLK_I2S9_APB 25
> +#define CLK_I2S0 26
> +#define CLK_I2S1 27
> +#define CLK_I2S2 28
> +#define CLK_I2S3 29
> +#define CLK_I2S4 30
> +#define CLK_I2S5 31
> +#define CLK_I2S6 32
> +#define CLK_I2S7 33
> +#define CLK_I2S8 34
> +#define CLK_I2S9 35
> +
> +#define CLK_MCLK0 36
> +#define CLK_MCLK1 37
> +#define CLK_MCLK2 38
> +#define CLK_MCLK3 39
> +#define CLK_MCLK4 40
> +
> +#endif
> --
> 2.50.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: Conor Dooley @ 2026-06-17 15:54 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang, cix-kernel-upstream, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260617060437.1474816-2-joakim.zhang@cixtech.com>
[-- Attachment #1: Type: text/plain, Size: 4379 bytes --]
On Wed, Jun 17, 2026 at 02:04:33PM +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> The Cix Sky1 Audio Subsystem (AUDSS) groups audio-related clock, reset
> and control registers in a dedicated CRU block. Software reset lines are
> exposed on the syscon parent via #reset-cells, following the same model
> as the existing Sky1 FCH and S5 system control bindings.
>
> A clock-controller child node is required under the audss syscon. It has
> no reg property of its own and accesses the parent register block for mux,
> divider and gate fields.
>
> The AUDSS is also controlled by one power domain and reset part.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> .../soc/cix/cix,sky1-system-control.yaml | 48 +++++++++++++++++++
> .../reset/cix,sky1-audss-system-control.h | 25 ++++++++++
> 2 files changed, 73 insertions(+)
> create mode 100644 include/dt-bindings/reset/cix,sky1-audss-system-control.h
>
> diff --git a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> index a01a515222c6..5a1cd5c24ade 100644
> --- a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> @@ -19,6 +19,7 @@ properties:
> - enum:
> - cix,sky1-system-control
> - cix,sky1-s5-system-control
> + - cix,sky1-audss-system-control
> - const: syscon
If the only thing these share are being a reset controller and having a
syscon fallback, I think it should be in a different file.
pw-bot: changes-requested
Cheers,
Conor.
>
> reg:
> @@ -27,6 +28,38 @@ properties:
> '#reset-cells':
> const: 1
>
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + clock-controller:
> + type: object
> + properties:
> + compatible:
> + const: cix,sky1-audss-clock
> + required:
> + - compatible
> + additionalProperties: true
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: cix,sky1-audss-system-control
> + then:
> + required:
> + - clock-controller
> + - power-domains
> + - resets
> + else:
> + properties:
> + clock-controller: false
> + power-domains: false
> + resets: false
> +
> required:
> - compatible
> - reg
> @@ -40,3 +73,18 @@ examples:
> reg = <0x4160000 0x100>;
> #reset-cells = <1>;
> };
> + - |
> + audss_syscon: system-controller@7110000 {
> + compatible = "cix,sky1-audss-system-control", "syscon";
> + reg = <0x7110000 0x10000>;
> + power-domains = <&smc_devpd 0>;
> + resets = <&s5_syscon 31>;
> + #reset-cells = <1>;
> +
> + clock-controller {
> + compatible = "cix,sky1-audss-clock";
> + #clock-cells = <1>;
> + clocks = <&scmi_clk 0>, <&scmi_clk 2>, <&scmi_clk 4>, <&scmi_clk 5>;
> + clock-names = "x8k", "x11k", "sys", "48m";
> + };
> + };
> diff --git a/include/dt-bindings/reset/cix,sky1-audss-system-control.h b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
> new file mode 100644
> index 000000000000..aabdce60b094
> --- /dev/null
> +++ b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright 2026 Cix Technology Group Co., Ltd.
> + */
> +#ifndef DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
> +#define DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
> +
> +#define AUDSS_I2S0_SW_RST 0
> +#define AUDSS_I2S1_SW_RST 1
> +#define AUDSS_I2S2_SW_RST 2
> +#define AUDSS_I2S3_SW_RST 3
> +#define AUDSS_I2S4_SW_RST 4
> +#define AUDSS_I2S5_SW_RST 5
> +#define AUDSS_I2S6_SW_RST 6
> +#define AUDSS_I2S7_SW_RST 7
> +#define AUDSS_I2S8_SW_RST 8
> +#define AUDSS_I2S9_SW_RST 9
> +#define AUDSS_WDT_SW_RST 10
> +#define AUDSS_TIMER_SW_RST 11
> +#define AUDSS_MB0_SW_RST 12
> +#define AUDSS_MB1_SW_RST 13
> +#define AUDSS_HDA_SW_RST 14
> +#define AUDSS_DMAC_SW_RST 15
> +
> +#endif
> --
> 2.50.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Conor Dooley @ 2026-06-17 15:51 UTC (permalink / raw)
To: Gerald Loacker
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, devicetree
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-2-4611ff00b0ff@wolfvision.net>
[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]
On Wed, Jun 17, 2026 at 02:23:14PM +0200, Gerald Loacker wrote:
> Add support for the optional rockchip,clk-lane-phase device tree property
> to allow board-specific tuning of the clock lane sampling phase for
> improved signal integrity across supported data rates.
>
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
> Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 03950b3cad08c..0d824d1511bc0 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -56,6 +56,13 @@ properties:
> description:
> Some additional phy settings are access through GRF regs.
>
> + rockchip,clk-lane-phase:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + description:
> + Clock lane sampling phase in 40 ps steps. The hardware default is 3.
Can this instead become rockchip,clk-lane-phase-ps and be listed in the
actual unit?
With the -ps suffix, you can then drop the $ref.
The default should be listed as "default: 3" (or default: 120)
pw-bot: changes-requested
> +
> required:
> - compatible
> - reg
>
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: (subset) [PATCH v2 3/4] mfd: mt6397-core: add mt6323 EFUSE support
From: Lee Jones @ 2026-06-17 15:46 UTC (permalink / raw)
To: Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Srinivas Kandagatla, Roman Vivchar
Cc: Andy Shevchenko, linux-pm, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, Ben Grisdale
In-Reply-To: <20260617-mt6323-nvmem-v2-3-4f30e36aa0f4@protonmail.com>
On Wed, 17 Jun 2026 12:48:46 +0300, Roman Vivchar wrote:
> The mt6323 PMIC includes an EFUSE. Register the EFUSE in the mt6323
> devices array to allow the corresponding driver to probe using compatible
> string.
Applied, thanks!
[3/4] mfd: mt6397-core: add mt6323 EFUSE support
commit: f5ce60535d04ca21799d558cfa13ba91bbe714b5
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v8 5/5] iio: adc: versal-sysmon: add oversampling support
From: Andy Shevchenko @ 2026-06-17 15:42 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-6-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:59PM +0100, Salih Erim wrote:
> Add support for reading and writing the oversampling ratio through
> the IIO oversampling_ratio attribute. The hardware supports averaging
> 2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
>
> Temperature and supply channels share oversampling configuration at
> the type level (all temperature channels share one ratio, all supply
> channels share another), exposed through info_mask_shared_by_type.
>
> The hardware encoding uses sample_count / 2 in a 4-bit field within
> the CONFIG register. Per-channel averaging enable registers must also
> be updated to activate or deactivate averaging.
This one LGTM now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: (subset) [PATCH v3 0/3] dt-bindings: mfd: syscon: Tighten checks
From: Lee Jones @ 2026-06-17 15:41 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Jacky Huang,
Shan-Chun Hung, Geert Uytterhoeven, Magnus Damm, Heiko Stuebner,
Aaro Koskinen, Andreas Kemnade, Kevin Hilman, Roger Quadros,
Tony Lindgren, Krzysztof Kozlowski
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-renesas-soc, linux-rockchip, linux-omap
In-Reply-To: <20260608-n-dt-bindings-simple-bus-syscon-v3-0-4eba9ec1212a@oss.qualcomm.com>
On Mon, 08 Jun 2026 22:44:23 +0200, Krzysztof Kozlowski wrote:
> Changes in v3:
> - Drop patch #2:
> dt-bindings: mfd: syscon: Drop unneeded case for syscon + simple-mfd
> - Bump dtschema requirement
> - Link to v2: https://patch.msgid.link/20260608-n-dt-bindings-simple-bus-syscon-v2-0-0203e6c249dc@oss.qualcomm.com
>
> Changes in v2:
> 1. New patches #2 and #3
> 1. Add missing part of patch #1, thus not adding Rob's Ack.
> https://lore.kernel.org/all/20260531110404.12768-3-krzysztof.kozlowski@oss.qualcomm.com/
>
> [...]
Applied, thanks!
[1/3] dt-bindings: mfd: syscon: Disallow simple-bus with syscon
commit: c11c918b40295dcb0ad2460d9534454072386f4c
[2/3] dt-bindings: mfd: syscon: Drop custom select for older dtschema
commit: f78049ca80ba2e68f7f46870b0d68eb54a6ce378
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v8 4/5] iio: adc: versal-sysmon: add threshold event support
From: Andy Shevchenko @ 2026-06-17 15:38 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-5-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:58PM +0100, Salih Erim wrote:
> Add threshold event support for temperature and supply voltage
> channels.
>
> Temperature events:
> - Rising threshold with configurable value on the device
> temperature channel (current max across all satellites)
> - Per-channel hysteresis as a millicelsius value
> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
>
> Supply voltage events:
> - Rising/falling threshold per supply channel
> - Per-channel alarm enable via alarm configuration registers
>
> The hardware supports both window and hysteresis alarm modes for
> temperature. This driver uses hysteresis mode, where the upper
> threshold triggers the alarm and the lower threshold clears it
> (re-arm point). The hardware has a single ISR bit per temperature
> channel with no indication of which threshold was crossed, so
> hysteresis mode is the natural fit. The lower threshold register
> is computed internally as (upper - hysteresis).
>
> Hysteresis is stored in the driver as a millicelsius value,
> initialized from the hardware registers at probe. Writing the
> rising threshold or hysteresis recomputes the lower register.
> ALARM_CONFIG is hard-coded to hysteresis mode during init.
>
> The hardware also provides a separate over-temperature (OT)
> threshold, but it is not exposed through IIO as it serves as a
> hardware safety mechanism for platform shutdown. OT will be
> exposed through the thermal framework in a follow-up series.
>
> The interrupt handler masks active threshold interrupts (which are
> level-sensitive) and schedules a delayed worker to poll for condition
> clear before unmasking. When no hardware IRQ is available, event
> specs are not attached and interrupt init is skipped, since the
> I2C regmap backend cannot be called from atomic context.
>
> When disabling a supply channel alarm, the group interrupt remains
> active if any other channel in the same alarm group still has an
> alarm enabled.
>
> A devm cleanup action masks all interrupts on driver unbind to
> prevent unhandled interrupt storms after the IRQ handler is freed.
...
> +static void sysmon_supply_processedtoraw(int val, u32 reg_val, u32 *raw_data)
> +{
> + int exponent = FIELD_GET(SYSMON_MODE_MASK, reg_val);
> + int format = FIELD_GET(SYSMON_FMT_MASK, reg_val);
> + int scale, tmp;
> +
> + scale = BIT(SYSMON_SUPPLY_MANTISSA_BITS - exponent);
> + tmp = (val * scale) / (int)MILLI;
> +
> + if (format)
> + tmp = clamp(tmp, S16_MIN, S16_MAX);
> + else
> + tmp = clamp(tmp, 0, U16_MAX);
Double check that minmax.h is included.
> + *raw_data = (u16)tmp;
> +}
...
> +static int sysmon_supply_thresh_offset(int address,
> + enum iio_event_direction dir)
Make it a single line. OTOH why is 'address' signed? Perhaps u32?
Or for some reason unsigned long as per _alarm_config()?
> +{
> + if (dir == IIO_EV_DIR_RISING)
> + return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_UP;
> + if (dir == IIO_EV_DIR_FALLING)
> + return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_LOW;
> +
> + return -EINVAL;
> +}
...
> +static int sysmon_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + unsigned int imr;
> + int config_value;
> + u32 mask;
> + int ret;
> +
> + mask = sysmon_get_event_mask(chan);
Just make it together, as we don't validate the value of 'mask'.
struct sysmon *sysmon = iio_priv(indio_dev);
u32 mask = sysmon_get_event_mask(chan);
...
int ret;
> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
> + if (ret)
> + return ret;
> +
> + /* IMR bits are 1=masked, invert to get 1=enabled */
> + imr = ~imr;
> +
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + config_value = sysmon_read_alarm_config(sysmon, chan->address);
> + if (config_value < 0)
> + return config_value;
> + return config_value && (imr & mask);
> +
> + case IIO_TEMP:
> + /*
> + * Return the administrative state, not the hardware IMR.
> + * The IRQ handler temporarily masks the interrupt during
> + * the polling window; reading IMR would show it as disabled.
> + * temp_mask bit is set when administratively disabled.
> + */
> + return !(sysmon->temp_mask & mask);
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static int sysmon_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> +{
> + u32 offset = SYSMON_ALARM_OFFSET(chan->address);
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + u32 ier = sysmon_get_event_mask(chan);
Here you call the variable 'ier'. Please, make this consistent in the related
APIs (see above).
> + unsigned int alarm_config;
> + int ret;
> +
> + guard(mutex)(&sysmon->lock);
> +
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + ret = sysmon_write_alarm_config(sysmon, chan->address, state);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(sysmon->regmap, offset, &alarm_config);
> + if (ret)
> + return ret;
> +
> + if (alarm_config)
> + return regmap_write(sysmon->regmap, SYSMON_IER, ier);
> +
> + return regmap_write(sysmon->regmap, SYSMON_IDR, ier);
> +
> + case IIO_TEMP:
> + if (state) {
> + ret = regmap_write(sysmon->regmap, SYSMON_IER, ier);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask &= ~ier;
> + } else {
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, ier);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask |= ier;
> + }
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static int sysmon_update_temp_lower(struct sysmon *sysmon)
> +{
> + unsigned int upper_reg;
> + int upper_mc, lower_mc;
> + u32 raw_val;
> + int ret;
> +
> + ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, &upper_reg);
> + if (ret)
> + return ret;
> +
> + sysmon_q8p7_to_millicelsius(upper_reg, &upper_mc);
> +
^^^
> + lower_mc = upper_mc - sysmon->temp_hysteresis;
Either add a blank line here, or remove the one above as these three is kinda
semantically coupled.
> + sysmon_millicelsius_to_q8p7(&raw_val, lower_mc);
> +
> + return regmap_write(sysmon->regmap, SYSMON_TEMP_TH_LOW, raw_val);
> +}
...
> +static void sysmon_unmask_temp(struct sysmon *sysmon, unsigned int isr)
> +{
> + unsigned int unmask, status;
As per above perhaps name 'unmask' as 'u32 ier'? Or did I miss the use case?
> + status = isr & SYSMON_TEMP_INTR_MASK;
> +
> + unmask = ~status & sysmon->masked_temp;
> + sysmon->masked_temp &= status;
> +
> + /* Only unmask if not administratively disabled by userspace */
> + unmask &= ~sysmon->temp_mask;
> +
> + regmap_write(sysmon->regmap, SYSMON_IER, unmask);
> +}
Also looking at all this, please double check variable names in all functions
and make types and names consistent across the whole driver code.
> + }
...
> - num_chan = size_add(num_temp, size_add(ARRAY_SIZE(temp_channels), num_supply));
> + num_static = ARRAY_SIZE(temp_channels);
> + num_chan = size_add(num_temp, size_add(num_static, num_supply));
At glance I don't see any additional arguments, can we introduce num_static in
the previous patch to reduce a churn here?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Conor Dooley @ 2026-06-17 15:36 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Amit Barzilai, robh, krzk+dt, conor+dt, devicetree, dri-devel,
linux-kernel, airlied, maarten.lankhorst, mripard, simona,
tzimmermann
In-Reply-To: <87a4suzc0c.fsf@ocarina.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]
On Tue, Jun 16, 2026 at 06:27:31PM +0200, Javier Martinez Canillas wrote:
> Conor Dooley <conor@kernel.org> writes:
>
> Hello Conor,
>
> > On Mon, Jun 15, 2026 at 08:56:20PM +0300, Amit Barzilai wrote:
> >> Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
> >> RGB OLED display controller driven over a 4-wire SPI bus. The binding
> >> builds on the shared solomon,ssd-common.yaml properties already used by
> >> the other Solomon display controllers.
> >>
> >> Assisted-by: Claude:claude-opus-4-8
> >> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
> >> ---
> >> Changes since v1:
> >> - Drop solomon,width / solomon,height: both are deducible from the
> >> compatible and are already declared (as optional) by the referenced
> >> solomon,ssd-common.yaml, so a local override is unnecessary.
> >> - Drop the rotation property: it has no consumer (rotation is being removed from the driver).
> >> - Use dt-bindings/gpio/gpio.h flag defines in the example
> >> (reset-gpios active-low, dc-gpios active-high).
> >
> > The user for this appears to be in staging. As far as I understand, the
> > policy is that we only add bindings for staging things when they move
> > out of staging.
> > Sure, this is straightforward but why should an exception be made here?
> > Are you working on moving this out of staging?
> >
>
>
> This DT binding was part of a series to add support for "solomon,ssd1351"
> to drivers/gpu/drm/solomon/ DRM driver. Amit only sent a v2 of the binding
> schema because he had some questions about the driver:
>
> https://lore.kernel.org/dri-devel/87cxxqzwxn.fsf@ocarina.mail-host-address-is-not-set/
>
> But yes, I agree that it would had been better for him to post this as a
> part of v2 (and I still expect him to do it), otherwise it is confusing.
>
> Specially since as you pointed out, there is an existing fbdev driver for
> the same device in staging.
Right, I'll expect this to reappear in a larger patchset then that
deals with the fbdev driver and adds the drm driver.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
From: Conor Dooley @ 2026-06-17 15:34 UTC (permalink / raw)
To: David Lechner
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
In-Reply-To: <c24982d7-40d6-4fd6-a083-90a8d3ce7f63@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 3818 bytes --]
On Tue, Jun 16, 2026 at 04:04:43PM -0500, David Lechner wrote:
> On 6/16/26 3:50 PM, Conor Dooley wrote:
> > On Tue, Jun 16, 2026 at 02:54:55PM -0500, David Lechner wrote:
> >> On 6/16/26 11:07 AM, Conor Dooley wrote:
> >>> On Mon, Jun 15, 2026 at 04:59:59PM -0500, David Lechner (TI) wrote:
> >>>> Add new bindings for ti,ads122c14 and similar devices.
> >>>>
> >>>> This is an ADC that is primarily intended for use with temperature
> >>>> sensors. There are a few unusual properties because of this. In
> >>>> particular, the reference voltage source and current output requirements
> >>>> can be different for each measurement, so these are included in the
> >>>> channel bindings.
> >>>>
> >>>> The REFP/REFN reference voltage is usually just connected to a resistor
> >>>> that is being driven by the ADC's current outputs, so there is special
> >>>> property for this case rather than requiring a regulator to be defined
> >>>> to represent that.
> >>>>
> >>>> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> >>>> have preferred an enum of strings).
> >>>>
> >>>> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> >>>> ---
> >>>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> >>>> MAINTAINERS | 7 +
> >>>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> >>>> 3 files changed, 242 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..dc7f37cad772
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> >>>> @@ -0,0 +1,224 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: Texas Instruments' ADS112C14 and similar ADC chips
> >>>> +
> >>>> +description: |
> >>>> + Supports the following Texas Instruments' ADC chips:
> >>>> + - ADS112C14 (16-bit)
> >>>> + - ADS122C14 (24-bit)
> >>>> +
> >>>> + https://www.ti.com/lit/ds/symlink/ads122c14.pdf
> >>>> +
> >>>> + These chips are primarily designed for use with temperature sensors such as
> >>>> + RTDs and thermocouples. The channel bindings reflect this in that each channel
> >>>> + represents the conditions required to make a measurement rather than strictly
> >>>> + just the physical input channels.
> >>>> +
> >>>> +maintainers:
> >>>> + - David Lechner <dlechner@baylibre.com>
> >>>> +
> >>>> +unevaluatedProperties: false
> >>>
> >>> Weird positioning of this.
> >>
> >> IIRC, Rob asked that I do it in this order on another binding a while
> >> ago (the reasoning being that it was too far away from properties:
> >> otherwise), so I've done it like this on a few bindings now. It doesn't
> >> make much difference to me though.
> >
> > Too far away because it refers to properties in the "main" node, but
> > appears conventionally after a rake of properties belonging to the
> > children?
> >
> I found the original request:
>
> https://lore.kernel.org/all/20241022204312.GA1524310-robh@kernel.org/
>
> "Easier to read the indented cases that way."
>
> Reading it again, it sounds like the request was just for the indented
> additionalProperties to be moved.
I think so, yeah. It's definitely common to see
patternProperties:
"^dma-channel@[0-9a-f]+$":
type: object
unevaluatedProperties: false
description:
DMA channel properties based on HDL compile-time configuration.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/2] drm/tiny: add support for PIXPAPER 4.26 monochrome e-ink panel
From: Thomas Zimmermann @ 2026-06-17 15:31 UTC (permalink / raw)
To: LiangCheng Wang, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Wig Cheng
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260529-bar-v3-2-5c2ac1c751ee@gmail.com>
Hi,
apologies for being late with the review.
Am 29.05.26 um 12:31 schrieb LiangCheng Wang:
> Introduce a DRM driver for the Mayqueen Pixpaper 4.26
> monochrome e-ink display panel, which is controlled via SPI.
> The driver supports an 800x480 display with XRGB8888
> framebuffer input.
>
> Also, add Kconfig and Makefile entries for the driver and
> update MAINTAINERS for the Pixpaper DRM drivers and binding.
>
> Signed-off-by: LiangCheng Wang <zaq14760@gmail.com>
> ---
> MAINTAINERS | 3 +-
> drivers/gpu/drm/tiny/Kconfig | 16 +
> drivers/gpu/drm/tiny/Makefile | 1 +
> drivers/gpu/drm/tiny/pixpaper-426m.c | 817 +++++++++++++++++++++++++++++++++++
> 4 files changed, 836 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 882214b0e7db53bb8cc8e75b5d2269ee0591ea20..eebd73ee1f531d3785ec963da03fbab265c2d188 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8234,11 +8234,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
> F: Documentation/devicetree/bindings/display/repaper.txt
> F: drivers/gpu/drm/tiny/repaper.c
>
> -DRM DRIVER FOR PIXPAPER E-INK PANEL
> +DRM DRIVER FOR PIXPAPER E-INK PANELS
> M: LiangCheng Wang <zaq14760@gmail.com>
> L: dri-devel@lists.freedesktop.org
> S: Maintained
> F: Documentation/devicetree/bindings/display/mayqueen,pixpaper.yaml
> +F: drivers/gpu/drm/tiny/pixpaper-426m.c
> F: drivers/gpu/drm/tiny/pixpaper.c
>
> DRM DRIVER FOR QEMU'S CIRRUS DEVICE
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f0e72d4b6a4709564e63c758e857bdb4a320dbe7..028c4314106ac31dfa717f6433c28e58b34c21e8 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -98,6 +98,22 @@ config DRM_PIXPAPER
>
> If M is selected, the module will be built as pixpaper.ko.
>
> +config DRM_PIXPAPER_426M
> + tristate "DRM support for PIXPAPER 4.26 monochrome display panel"
> + depends on DRM && SPI
> + depends on MMU
> + select DRM_CLIENT_SELECTION
> + select DRM_GEM_SHMEM_HELPER
> + select DRM_KMS_HELPER
> + help
> + DRM driver for the Mayqueen Pixpaper 4.26 monochrome e-ink
> + display panel.
> +
> + This driver supports SPI-connected 800x480 monochrome panels
> + with an XRGB8888 framebuffer input format.
> +
> + If M is selected, the module will be built as pixpaper-426m.ko.
> +
> config TINYDRM_HX8357D
> tristate "DRM support for HX8357D display panels"
> depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 48d30bf6152f979404ac1004174587823a30109e..037b751a1a851cc2f86f701ff71008bcb9c59f29 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus-qemu.o
> obj-$(CONFIG_DRM_GM12U320) += gm12u320.o
> obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o
> obj-$(CONFIG_DRM_PIXPAPER) += pixpaper.o
> +obj-$(CONFIG_DRM_PIXPAPER_426M) += pixpaper-426m.o
> obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
> obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o
> obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
> diff --git a/drivers/gpu/drm/tiny/pixpaper-426m.c b/drivers/gpu/drm/tiny/pixpaper-426m.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..99464d564f315543037a3621ff85f98f1bd8f34c
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/pixpaper-426m.c
> @@ -0,0 +1,817 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DRM driver for PIXPAPER 4.26 monochrome e-ink panel
> + *
> + * Author: LiangCheng Wang <zaq14760@gmail.com>,
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/string.h>
> +
> +#include <drm/clients/drm_client_setup.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_shmem.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> +
> +MODULE_IMPORT_NS("DMA_BUF");
> +
> +/* Panel visible resolution */
> +#define PIXPAPER_WIDTH 800
> +#define PIXPAPER_HEIGHT 480
We already use the prefixes PIXPAPER_, and pixpaper, and variants
thereof in the current pixpaper driver. You have to choose a different
prefix. If I may suggest one, pix426m_ would be fine.
> +
> +/*
> + * The panel datasheet specifies an active area of 92.8 mm x 55.68 mm.
> + * Round to whole millimeters for drm_display_info.
> + */
> +#define PIXPAPER_WIDTH_MM 93
> +#define PIXPAPER_HEIGHT_MM 56
> +
> +/*
> + * According to the panel datasheet, no RGB-style timing parameters
> + * (porches, sync widths, or a dot clock) are provided. Define a minimal
> + * fixed mode only to satisfy the DRM mode API for this SPI-driven
> + * e-paper panel.
> + */
> +#define PIXPAPER_HSYNC_LEN 1
> +#define PIXPAPER_HFRONT_PORCH 1
> +#define PIXPAPER_HBACK_PORCH 1
> +#define PIXPAPER_VSYNC_LEN 1
> +#define PIXPAPER_VFRONT_PORCH 1
> +#define PIXPAPER_VBACK_PORCH 1
> +#define PIXPAPER_MODE_REFRESH_HZ 1
> +#define PIXPAPER_MODE_CLOCK_KHZ \
> + (((PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN + \
> + PIXPAPER_HBACK_PORCH) * \
> + (PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN + \
> + PIXPAPER_VBACK_PORCH) * \
> + PIXPAPER_MODE_REFRESH_HZ) / 1000)
You can, I think, drop all these constants. Further below where you
define a display mode, just use DRM_SIMPLE_MODE() with the pixel
resolution and physical dimensions. This is how other drivers make up
display modes.
> +
> +#define PIXPAPER_SPI_BITS_PER_WORD 8
> +#define PIXPAPER_SPI_SPEED_DEFAULT 1000000
> +
> +#define PIXPAPER_TX_BUF_SIZE 8
> +
> +#define PIXPAPER_PIXEL_THRESHOLD 128
> +
> +#define PIXPAPER_BUSY_TIMEOUT_MS 10000
> +#define PIXPAPER_BUSY_POLL_INITIAL_US_MIN 1000
> +#define PIXPAPER_BUSY_POLL_INITIAL_US_MAX 1500
> +#define PIXPAPER_BUSY_POLL_US_MIN 100
> +#define PIXPAPER_BUSY_POLL_US_MAX 200
> +
> +#define PIXPAPER_RAM_START_ADDR 0x00
> +
> +#define PIXPAPER_LUMA_R_WEIGHT 299
> +#define PIXPAPER_LUMA_G_WEIGHT 587
> +#define PIXPAPER_LUMA_B_WEIGHT 114
> +#define PIXPAPER_LUMA_DIVISOR 1000
> +#define PIXPAPER_LUMA_ROUNDING_BIAS 500
> +
> +#define PIXPAPER_CMD_DRIVER_OUTPUT_CTRL 0x01
> +#define PIXPAPER_CMD_BOOSTER_SOFT_START_CTRL 0x0C
> +#define PIXPAPER_CMD_TEMP_SENSOR_CONTROL 0x18
> +#define PIXPAPER_CMD_MASTER_ACTIVATION 0x20
> +#define PIXPAPER_CMD_DISPLAY_UPDATE_CTRL2 0x22
> +#define PIXPAPER_CMD_WRITE_RAM_BW 0x24
> +#define PIXPAPER_CMD_BORDER_WAVEFORM_CONTROL 0x3C
> +#define PIXPAPER_CMD_SET_RAM_X_START_END 0x44
> +#define PIXPAPER_CMD_SET_RAM_Y_START_END 0x45
> +#define PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER 0x4E
> +#define PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER 0x4F
> +
> +#define PIXPAPER_DRIVER_OUTPUT_SM BIT(1)
> +
> +#define PIXPAPER_BORDER_WAVEFORM_GS_TRANSITION (0x0 << 6)
> +#define PIXPAPER_BORDER_WAVEFORM_LUT1_SEL 0x1
> +
> +#define PIXPAPER_UPDATE_CTRL2_ENABLE_CLK BIT(7)
> +#define PIXPAPER_UPDATE_CTRL2_ENABLE_ANALOG BIT(6)
> +#define PIXPAPER_UPDATE_CTRL2_LOAD_TEMP BIT(5)
> +#define PIXPAPER_UPDATE_CTRL2_LOAD_LUT BIT(4)
> +#define PIXPAPER_UPDATE_CTRL2_PATTERN_DISPLAY BIT(2)
> +
> +#define PIXPAPER_TEMP_SENSOR_INTERNAL 0x80
> +#define PIXPAPER_SOFTSTART_A 0xAE
> +#define PIXPAPER_SOFTSTART_B 0xC7
> +#define PIXPAPER_SOFTSTART_C 0xC3
> +#define PIXPAPER_SOFTSTART_D 0xC0
> +#define PIXPAPER_SOFTSTART_E 0x80
> +#define PIXPAPER_DRIVER_OUTPUT_GD_SM_TB PIXPAPER_DRIVER_OUTPUT_SM
> +#define PIXPAPER_BORDER_LUT1 \
> + (PIXPAPER_BORDER_WAVEFORM_GS_TRANSITION | \
> + PIXPAPER_BORDER_WAVEFORM_LUT1_SEL)
> +#define PIXPAPER_UPDATE_INITIAL \
> + (PIXPAPER_UPDATE_CTRL2_ENABLE_CLK | \
> + PIXPAPER_UPDATE_CTRL2_ENABLE_ANALOG | \
> + PIXPAPER_UPDATE_CTRL2_LOAD_TEMP | \
> + PIXPAPER_UPDATE_CTRL2_LOAD_LUT | \
> + PIXPAPER_UPDATE_CTRL2_PATTERN_DISPLAY)
Empty line here?
> +struct pixpaper_error_ctx {
> + int errno_code;
> +};
> +
> +struct pixpaper_init_seq {
> + u8 cmd;
> + const u8 *data;
> + u8 len;
> +};
> +
> +struct pixpaper_panel {
> + struct drm_device drm;
> + struct drm_plane plane;
> + struct drm_crtc crtc;
> + struct drm_encoder encoder;
> + struct drm_connector connector;
> +
> + struct spi_device *spi;
> + struct gpio_desc *reset;
> + struct gpio_desc *busy;
> + struct gpio_desc *dc;
> +
> + u8 *tx_buf;
> +};
> +
> +static const uint32_t pixpaper_formats[] = {
> + DRM_FORMAT_XRGB8888,
> +};
> +
> +static const u8 pixpaper_init_temp_sensor[] = {
> + PIXPAPER_TEMP_SENSOR_INTERNAL,
> +};
> +
> +static const u8 pixpaper_init_softstart[] = {
> + PIXPAPER_SOFTSTART_A,
> + PIXPAPER_SOFTSTART_B,
> + PIXPAPER_SOFTSTART_C,
> + PIXPAPER_SOFTSTART_D,
> + PIXPAPER_SOFTSTART_E,
> +};
> +
> +static const u8 pixpaper_init_driver_output[] = {
> + (PIXPAPER_HEIGHT - 1) & 0xff,
> + (PIXPAPER_HEIGHT - 1) >> 8,
> + PIXPAPER_DRIVER_OUTPUT_GD_SM_TB,
> +};
> +
> +static const u8 pixpaper_init_border[] = {
> + PIXPAPER_BORDER_LUT1,
> +};
> +
> +static const u8 pixpaper_init_ram_x_window[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> + (PIXPAPER_WIDTH - 1) & 0xff,
> + (PIXPAPER_WIDTH - 1) >> 8,
> +};
> +
> +static const u8 pixpaper_init_ram_y_window[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> + (PIXPAPER_HEIGHT - 1) & 0xff,
> + (PIXPAPER_HEIGHT - 1) >> 8,
> +};
> +
> +static const u8 pixpaper_init_ram_x_counter[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> +};
> +
> +static const u8 pixpaper_init_ram_y_counter[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> +};
> +
> +static const struct pixpaper_init_seq pixpaper_init_seqs[] = {
> + {
> + .cmd = PIXPAPER_CMD_TEMP_SENSOR_CONTROL,
> + .data = pixpaper_init_temp_sensor,
> + .len = ARRAY_SIZE(pixpaper_init_temp_sensor),
> + },
> + {
> + .cmd = PIXPAPER_CMD_BOOSTER_SOFT_START_CTRL,
> + .data = pixpaper_init_softstart,
> + .len = ARRAY_SIZE(pixpaper_init_softstart),
> + },
> + {
> + .cmd = PIXPAPER_CMD_DRIVER_OUTPUT_CTRL,
> + .data = pixpaper_init_driver_output,
> + .len = ARRAY_SIZE(pixpaper_init_driver_output),
> + },
> + {
> + .cmd = PIXPAPER_CMD_BORDER_WAVEFORM_CONTROL,
> + .data = pixpaper_init_border,
> + .len = ARRAY_SIZE(pixpaper_init_border),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_X_START_END,
> + .data = pixpaper_init_ram_x_window,
> + .len = ARRAY_SIZE(pixpaper_init_ram_x_window),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_Y_START_END,
> + .data = pixpaper_init_ram_y_window,
> + .len = ARRAY_SIZE(pixpaper_init_ram_y_window),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER,
> + .data = pixpaper_init_ram_x_counter,
> + .len = ARRAY_SIZE(pixpaper_init_ram_x_counter),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER,
> + .data = pixpaper_init_ram_y_counter,
> + .len = ARRAY_SIZE(pixpaper_init_ram_y_counter),
> + },
> +};
> +
> +static inline struct pixpaper_panel *to_pixpaper_panel(struct drm_device *drm)
> +{
> + return container_of(drm, struct pixpaper_panel, drm);
> +}
> +
> +static void pixpaper_wait_for_panel(struct pixpaper_panel *panel)
> +{
> + unsigned int timeout_ms = PIXPAPER_BUSY_TIMEOUT_MS;
> + unsigned long timeout_jiffies = jiffies + msecs_to_jiffies(timeout_ms);
> +
> + usleep_range(PIXPAPER_BUSY_POLL_INITIAL_US_MIN,
> + PIXPAPER_BUSY_POLL_INITIAL_US_MAX);
> + while (gpiod_get_value_cansleep(panel->busy) != 0) {
> + if (time_after(jiffies, timeout_jiffies)) {
> + /*
> + * Treat a busy timeout as warning-only. Some panels may
> + * keep BUSY asserted longer than expected during
> + * initialization or refresh.
> + */
> + drm_warn(&panel->drm, "Busy wait timed out\n");
> + return;
> + }
> + usleep_range(PIXPAPER_BUSY_POLL_US_MIN,
> + PIXPAPER_BUSY_POLL_US_MAX);
> + }
> +}
> +
> +static void pixpaper_spi_write(struct pixpaper_panel *panel, int dc,
> + const void *buf, size_t len,
> + struct pixpaper_error_ctx *err)
> +{
> + int ret;
> +
> + if (err->errno_code || !len)
> + return;
> +
> + gpiod_set_value_cansleep(panel->dc, dc);
> + usleep_range(1, 5);
> +
> + ret = spi_write(panel->spi, buf, len);
> + if (ret < 0)
> + err->errno_code = ret;
> +}
> +
> +static void pixpaper_send_cmd(struct pixpaper_panel *panel, u8 cmd,
> + struct pixpaper_error_ctx *err)
> +{
> + panel->tx_buf[0] = cmd;
> + pixpaper_spi_write(panel, 0, panel->tx_buf, sizeof(cmd), err);
> +}
> +
> +static void pixpaper_send_data(struct pixpaper_panel *panel, u8 data,
> + struct pixpaper_error_ctx *err)
> +{
> + panel->tx_buf[0] = data;
> + pixpaper_spi_write(panel, 1, panel->tx_buf, sizeof(data), err);
> +}
> +
> +static void pixpaper_reset_ram_counters(struct pixpaper_panel *panel,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> +}
> +
> +static void pixpaper_write_ram(struct pixpaper_panel *panel, u8 cmd,
> + const u8 *buf, u32 len,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code || !buf || !len)
> + return;
> +
> + pixpaper_reset_ram_counters(panel, err);
> +
> + pixpaper_send_cmd(panel, cmd, err);
> + pixpaper_spi_write(panel, 1, buf, len, err);
> +}
> +
> +static void pixpaper_send_init_seq(struct pixpaper_panel *panel,
> + const struct pixpaper_init_seq *seq,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code || !seq->data || !seq->len)
> + return;
> +
> + if (seq->len > PIXPAPER_TX_BUF_SIZE) {
> + err->errno_code = -EINVAL;
> + return;
> + }
> +
> + pixpaper_send_cmd(panel, seq->cmd, err);
> + memcpy(panel->tx_buf, seq->data, seq->len);
> + pixpaper_spi_write(panel, 1, panel->tx_buf, seq->len, err);
> +}
> +
> +static void pixpaper_trigger_update(struct pixpaper_panel *panel,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_DISPLAY_UPDATE_CTRL2, err);
> + pixpaper_send_data(panel, PIXPAPER_UPDATE_INITIAL, err);
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_MASTER_ACTIVATION, err);
> + pixpaper_wait_for_panel(panel);
> +}
> +
> +static void pixpaper_xrgb8888_to_bw(const void *src, void *dst, u32 height,
> + u32 width, u32 src_pitch, u32 dst_pitch)
The code in this function is very similar to drm_fb_xrgb8888_to_mono();
just reversed. Unfortunately, there's no simple way of reusing anything.
https://elixir.bootlin.com/linux/v7.1/source/drivers/gpu/drm/drm_format_helper.c#L1204
> +{
> + const uint8_t *src_base = src;
> + uint8_t *dst_pixels = dst;
> +
> + if (dst == NULL || src == NULL)
> + return;
> +
> + for (u32 y = 0; y < height; y++) {
> + uint8_t *dst_row = dst_pixels + y * dst_pitch;
> + const __le32 *src_pixels =
> + (const __le32 *)(src_base + y * src_pitch);
> +
> + for (u32 x = 0; x < width; x++) {
> + /*
> + * The panel RAM X direction is reversed relative to DRM
> + * coordinates. Read pixels from right to left so the
> + * displayed image matches the expected orientation on
> + * the panel.
> + */
> + u32 src_x = width - 1 - x;
> + uint8_t r, g, b;
> + u8 bit;
> + u32 bit_pos = x % 8;
> + u32 byte_pos = x / 8;
> + uint32_t gray_val;
> + uint32_t pixel;
> +
> + pixel = le32_to_cpu(src_pixels[src_x]);
> + r = (pixel >> 16) & 0xFF;
> + g = (pixel >> 8) & 0xFF;
> + b = pixel & 0xFF;
> +
> + gray_val = (r * PIXPAPER_LUMA_R_WEIGHT +
> + g * PIXPAPER_LUMA_G_WEIGHT +
> + b * PIXPAPER_LUMA_B_WEIGHT +
> + PIXPAPER_LUMA_ROUNDING_BIAS) /
> + PIXPAPER_LUMA_DIVISOR;
> + bit = gray_val >= PIXPAPER_PIXEL_THRESHOLD;
> +
> + if (bit)
> + dst_row[byte_pos] |= BIT(7 - bit_pos);
> + else
> + dst_row[byte_pos] &= ~BIT(7 - bit_pos);
> + }
> + }
> +}
> +
> +static void *pixpaper_prepare_buffer(const void *vaddr,
> + const struct drm_framebuffer *fb,
> + u32 *dst_pitch,
> + struct pixpaper_error_ctx *err)
> +{
> + void *dst;
> +
> + if (err->errno_code)
> + return NULL;
> +
> + *dst_pitch = DIV_ROUND_UP(fb->width, 8);
> + dst = kzalloc(*dst_pitch * fb->height, GFP_KERNEL);
> + if (!dst) {
> + err->errno_code = -ENOMEM;
> + return NULL;
> + }
> +
> + pixpaper_xrgb8888_to_bw(vaddr, dst, fb->height, fb->width,
> + fb->pitches[0], *dst_pitch);
> +
> + return dst;
> +}
> +
> +static void pixpaper_write_image(struct pixpaper_panel *panel,
> + const u8 *buf, u32 len,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_write_ram(panel, PIXPAPER_CMD_WRITE_RAM_BW, buf, len, err);
> +}
> +
> +static int pixpaper_panel_hw_init(struct pixpaper_panel *panel)
> +{
> + struct pixpaper_error_ctx err = { .errno_code = 0 };
> + u8 i;
> +
> + gpiod_set_value_cansleep(panel->reset, 0);
> + msleep(50);
> + gpiod_set_value_cansleep(panel->reset, 1);
> + msleep(50);
> +
> + pixpaper_wait_for_panel(panel);
> +
> + for (i = 0; i < ARRAY_SIZE(pixpaper_init_seqs); i++) {
> + pixpaper_send_init_seq(panel, &pixpaper_init_seqs[i], &err);
> + if (err.errno_code)
> + goto init_fail;
> + }
> +
> + return 0;
> +
> +init_fail:
> + drm_err(&panel->drm, "Hardware initialization failed (err=%d)\n",
> + err.errno_code);
> + return err.errno_code;
> +}
> +
> +static int pixpaper_plane_helper_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *new_plane_state =
> + drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc *new_crtc = new_plane_state->crtc;
> + struct drm_crtc_state *new_crtc_state = NULL;
> + int ret;
> +
> + if (new_crtc)
> + new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> + new_crtc_state, DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING, false, false);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int pixpaper_crtc_helper_atomic_check(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state =
> + drm_atomic_get_new_crtc_state(state, crtc);
> +
> + if (!crtc_state->enable)
> + return 0;
> +
> + return drm_atomic_helper_check_crtc_primary_plane(crtc_state);
> +}
> +
> +static void pixpaper_crtc_atomic_enable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> + struct drm_device *drm = &panel->drm;
> + int idx;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + drm_dev_exit(idx);
Simply keep this helper empty.
> +}
> +
> +static void pixpaper_crtc_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> + struct drm_device *drm = &panel->drm;
> + int idx;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + drm_dev_exit(idx);
Simply keep this helper empty.
> +}
> +
> +static void pixpaper_plane_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *plane_state =
> + drm_atomic_get_new_plane_state(state, plane);
> + struct drm_shadow_plane_state *shadow_plane_state =
> + to_drm_shadow_plane_state(plane_state);
> + struct pixpaper_panel *panel = to_pixpaper_panel(plane->dev);
> +
> + if (!plane_state->crtc || !plane_state->fb || !plane_state->visible)
> + return;
> +
> + {
No such blocks, please.
> + struct drm_device *drm = &panel->drm;
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct iosys_map map = shadow_plane_state->data[0];
> + const void *vaddr = map.vaddr;
> + int idx;
> + struct pixpaper_error_ctx err = { .errno_code = 0 };
> + uint32_t dst_pitch;
> + void *dst = NULL;
> + u32 dst_len;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + if (fb->format->format != DRM_FORMAT_XRGB8888) {
No need to do a format check. The DRM framework checks this for you.
> + err.errno_code = -EINVAL;
> + drm_err_once(drm, "Unsupported framebuffer format: 0x%08x\n",
> + fb->format->format);
> + goto update_cleanup;
> + }
> +
> + dst = pixpaper_prepare_buffer(vaddr, fb, &dst_pitch, &err);
> + if (err.errno_code) {
> + drm_err_once(drm, "Failed to allocate temporary buffer\n");
> + goto update_cleanup;
> + }
This call allocates memory. atomic_update is not a good place to do that.
While you operate on the framebuffer memory, you need to use
drm_gem_fb_begin_cpu_access() and drm_gem_fb_end_cpu_access().
Otherwise, another DMA and other drivers can mess around with your
buffer content. See the other drivers for examples.
> +
> + dst_len = dst_pitch * fb->height;
> + pixpaper_write_image(panel, dst, dst_len, &err);
> + if (err.errno_code)
> + goto update_cleanup;
And this call always writes out the full image, I think. Is it possible
to use damage clipping like the other drivers do?
> +
> + pixpaper_trigger_update(panel, &err);
> + if (err.errno_code)
> + goto update_cleanup;
> +update_cleanup:
> + if (err.errno_code && err.errno_code != -ETIMEDOUT)
> + drm_err_once(drm, "Frame update failed: %d\n",
> + err.errno_code);
> +
> + kfree(dst);
And here you free the allocated memory. There should never be a reason
to allocate more than 800x400 bits. I think you can pre-allocate the
buffer in the main device structure and re-use it on each transfer.
Best regards
Thomas
> + drm_dev_exit(idx);
> + }
> +}
> +
> +static const struct drm_display_mode pixpaper_mode = {
> + .clock = PIXPAPER_MODE_CLOCK_KHZ,
> + .hdisplay = PIXPAPER_WIDTH,
> + .hsync_start = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH,
> + .hsync_end = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN,
> + .htotal = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN +
> + PIXPAPER_HBACK_PORCH,
> + .vdisplay = PIXPAPER_HEIGHT,
> + .vsync_start = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH,
> + .vsync_end = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN,
> + .vtotal = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN +
> + PIXPAPER_VBACK_PORCH,
> + .width_mm = PIXPAPER_WIDTH_MM,
> + .height_mm = PIXPAPER_HEIGHT_MM,
> + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static int pixpaper_connector_get_modes(struct drm_connector *connector)
> +{
> + return drm_connector_helper_get_modes_fixed(connector, &pixpaper_mode);
> +}
> +
> +static enum drm_mode_status
> +pixpaper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode)
> +{
> + if (mode->hdisplay == PIXPAPER_WIDTH &&
> + mode->vdisplay == PIXPAPER_HEIGHT)
> + return MODE_OK;
> +
> + return MODE_BAD;
> +}
> +
> +static const struct drm_plane_funcs pixpaper_plane_funcs = {
> + .update_plane = drm_atomic_helper_update_plane,
> + .disable_plane = drm_atomic_helper_disable_plane,
> + .destroy = drm_plane_cleanup,
> + DRM_GEM_SHADOW_PLANE_FUNCS,
> +};
> +
> +static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = {
> + DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> + .atomic_check = pixpaper_plane_helper_atomic_check,
> + .atomic_update = pixpaper_plane_atomic_update,
> +};
> +
> +static const struct drm_crtc_funcs pixpaper_crtc_funcs = {
> + .set_config = drm_atomic_helper_set_config,
> + .page_flip = drm_atomic_helper_page_flip,
> + .reset = drm_atomic_helper_crtc_reset,
> + .destroy = drm_crtc_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static const struct drm_crtc_helper_funcs pixpaper_crtc_helper_funcs = {
> + .mode_valid = pixpaper_mode_valid,
> + .atomic_check = pixpaper_crtc_helper_atomic_check,
> + .atomic_enable = pixpaper_crtc_atomic_enable,
> + .atomic_disable = pixpaper_crtc_atomic_disable,
> +};
> +
> +static const struct drm_encoder_funcs pixpaper_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +static const struct drm_connector_funcs pixpaper_connector_funcs = {
> + .reset = drm_atomic_helper_connector_reset,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_connector_helper_funcs pixpaper_connector_helper_funcs = {
> + .get_modes = pixpaper_connector_get_modes,
> +};
> +
> +DEFINE_DRM_GEM_FOPS(pixpaper_fops);
> +
> +static struct drm_driver pixpaper_drm_driver = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> + .fops = &pixpaper_fops,
> + .name = "pixpaper-426m",
> + .desc = "DRM driver for PIXPAPER 4.26 monochrome e-ink panel",
> + .major = 1,
> + .minor = 0,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> + DRM_FBDEV_SHMEM_DRIVER_OPS,
> +};
> +
> +static const struct drm_mode_config_funcs pixpaper_mode_config_funcs = {
> + .fb_create = drm_gem_fb_create_with_dirty,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int pixpaper_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct pixpaper_panel *panel;
> + struct drm_device *drm;
> + int ret;
> +
> + panel = devm_drm_dev_alloc(dev, &pixpaper_drm_driver,
> + struct pixpaper_panel, drm);
> + if (IS_ERR(panel))
> + return PTR_ERR(panel);
> +
> + drm = &panel->drm;
> + panel->spi = spi;
> + spi_set_drvdata(spi, panel);
> +
> + panel->tx_buf = devm_kzalloc(dev, PIXPAPER_TX_BUF_SIZE, GFP_KERNEL);
> + if (!panel->tx_buf)
> + return -ENOMEM;
> +
> + ret = drmm_mode_config_init(drm);
> + if (ret)
> + return ret;
> +
> + spi->mode = SPI_MODE_0;
> + spi->bits_per_word = PIXPAPER_SPI_BITS_PER_WORD;
> +
> + if (!spi->max_speed_hz) {
> + drm_warn(drm,
> + "spi-max-frequency not specified in DT, using default %u Hz\n",
> + PIXPAPER_SPI_SPEED_DEFAULT);
> + spi->max_speed_hz = PIXPAPER_SPI_SPEED_DEFAULT;
> + }
> +
> + ret = spi_setup(spi);
> + if (ret < 0) {
> + drm_err(drm, "SPI setup failed: %d\n", ret);
> + return ret;
> + }
> +
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret) {
> + drm_err(drm, "Failed to set DMA mask: %d\n", ret);
> + return ret;
> + }
> +
> + panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(panel->reset))
> + return PTR_ERR(panel->reset);
> +
> + panel->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
> + if (IS_ERR(panel->busy))
> + return PTR_ERR(panel->busy);
> +
> + panel->dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_HIGH);
> + if (IS_ERR(panel->dc))
> + return PTR_ERR(panel->dc);
> +
> + ret = pixpaper_panel_hw_init(panel);
> + if (ret) {
> + drm_err(drm, "Panel hardware initialization failed: %d\n", ret);
> + return ret;
> + }
> +
> + drm->mode_config.funcs = &pixpaper_mode_config_funcs;
> + drm->mode_config.min_width = PIXPAPER_WIDTH;
> + drm->mode_config.max_width = PIXPAPER_WIDTH;
> + drm->mode_config.min_height = PIXPAPER_HEIGHT;
> + drm->mode_config.max_height = PIXPAPER_HEIGHT;
> +
> + ret = drm_universal_plane_init(drm, &panel->plane, 1, &pixpaper_plane_funcs,
> + pixpaper_formats, ARRAY_SIZE(pixpaper_formats), NULL,
> + DRM_PLANE_TYPE_PRIMARY, NULL);
> + if (ret)
> + return ret;
> + drm_plane_helper_add(&panel->plane, &pixpaper_plane_helper_funcs);
> +
> + ret = drm_crtc_init_with_planes(drm, &panel->crtc, &panel->plane, NULL,
> + &pixpaper_crtc_funcs, NULL);
> + if (ret)
> + return ret;
> + drm_crtc_helper_add(&panel->crtc, &pixpaper_crtc_helper_funcs);
> +
> + ret = drm_encoder_init(drm, &panel->encoder, &pixpaper_encoder_funcs,
> + DRM_MODE_ENCODER_NONE, NULL);
> + if (ret)
> + return ret;
> +
> + ret = drm_connector_init(drm, &panel->connector,
> + &pixpaper_connector_funcs,
> + DRM_MODE_CONNECTOR_SPI);
> + if (ret)
> + return ret;
> +
> + drm_connector_helper_add(&panel->connector,
> + &pixpaper_connector_helper_funcs);
> + drm_connector_attach_encoder(&panel->connector, &panel->encoder);
> + panel->encoder.possible_crtcs = drm_crtc_mask(&panel->crtc);
> +
> + drm_mode_config_reset(drm);
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret)
> + return ret;
> +
> + drm_client_setup(drm, NULL);
> +
> + return 0;
> +}
> +
> +static void pixpaper_remove(struct spi_device *spi)
> +{
> + struct pixpaper_panel *panel = spi_get_drvdata(spi);
> +
> + if (!panel)
> + return;
> +
> + drm_dev_unplug(&panel->drm);
> + drm_atomic_helper_shutdown(&panel->drm);
> +}
> +
> +static const struct spi_device_id pixpaper_ids[] = { { "pixpaper-426m", 0 }, {} };
> +MODULE_DEVICE_TABLE(spi, pixpaper_ids);
> +
> +static const struct of_device_id pixpaper_dt_ids[] = {
> + { .compatible = "mayqueen,pixpaper-426m" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pixpaper_dt_ids);
> +
> +static struct spi_driver pixpaper_spi_driver = {
> + .driver = {
> + .name = "pixpaper-426m",
> + .of_match_table = pixpaper_dt_ids,
> + },
> + .id_table = pixpaper_ids,
> + .probe = pixpaper_probe,
> + .remove = pixpaper_remove,
> +};
> +
> +module_spi_driver(pixpaper_spi_driver);
> +
> +MODULE_AUTHOR("LiangCheng Wang");
> +MODULE_DESCRIPTION("DRM SPI driver for PIXPAPER 4.26 monochrome e-ink panel");
> +MODULE_LICENSE("GPL");
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* [PATCH v3] dt-bindings: arm: xen: Convert to DT schema
From: Tejas Mutalikdesai @ 2026-06-17 15:28 UTC (permalink / raw)
To: devicetree; +Cc: robh, krzk+dt, conor+dt, sstabellini, Tejas
From: Tejas <tejasmutalikdesai@gmail.com>
Convert the Xen ARM device tree binding documentation from the legacy
plain-text format (Documentation/devicetree/bindings/arm/xen.txt) to
the DT schema format, as required by the modern DT binding process.
Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
---
Changes since v2:
- Replace 'YAML schema' with 'DT schema' in the commit description
- Drop unnecessary '|' block scalars from description fields that do
not need to preserve literal formatting
- Fix unit_address_vs_reg warning: replace $nodename const with a
pattern requiring a unit address; update example to hypervisor@b0000000
and drop the root node, GIC, and interrupt-parent
- Update Documentation reference in arch/arm/xen/enlighten.c from
xen.txt to xen.yaml
Documentation/devicetree/bindings/arm/xen.txt | 62 ----------
.../devicetree/bindings/arm/xen.yaml | 109 ++++++++++++++++++
arch/arm/xen/enlighten.c | 2 +-
3 files changed, 110 insertions(+), 63 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..a22e950566c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/xen.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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:
+ pattern: "^hypervisor@[0-9a-f]+$"
+
+ 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 a 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
+
+ required:
+ - xen,uefi-system-table
+ - xen,uefi-mmap-start
+ - xen,uefi-mmap-size
+ - xen,uefi-mmap-desc-size
+ - xen,uefi-mmap-desc-ver
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ hypervisor@b0000000 {
+ compatible = "xen,xen-4.3", "xen,xen";
+ reg = <0xb0000000 0x20000>;
+ 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>;
+ };
+ };
+...
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 25a0ce3b4584..0b7b7e3417e3 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -251,7 +251,7 @@ static int __init fdt_find_hyper_node(unsigned long node, const char *uname,
}
/*
- * see Documentation/devicetree/bindings/arm/xen.txt for the
+ * see Documentation/devicetree/bindings/arm/xen.yaml for the
* documentation of the Xen Device Tree format.
*/
void __init xen_early_init(void)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: gpu: img,powervr-rogue: Document GE8300 GPU in Renesas R-Car D3
From: Conor Dooley @ 2026-06-17 15:28 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Marek Vasut, Geert Uytterhoeven, Conor Dooley, David Airlie,
Frank Binns, Krzysztof Kozlowski, Maarten Lankhorst, Magnus Damm,
Matt Coster, Maxime Ripard, Rob Herring, Simona Vetter,
Thomas Zimmermann, devicetree, dri-devel, linux-renesas-soc
In-Reply-To: <20260616182802.GB1662668@fsdn.se>
[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]
On Tue, Jun 16, 2026 at 08:28:02PM +0200, Niklas Söderlund wrote:
> On 2026-06-16 19:58:34 +0200, Niklas Söderlund wrote:
> > Document Imagination Technologies PowerVR Rogue GE8300 BNVC 22.67.54.30
> > present in Renesas R-Car R8A77995 D3 SoCs.
> >
> > Compared to other R-Car Gen3 SoCs the D3 only have one power domain and
> > it is always on. Extend the list of special cases for this to also cover
> > R8A77995 and update the description of it.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > * Changes since v1
> > - Sort img,img-ge8300 after img,img-ge7800.
> > - Fold special case for power domain into an existing one and update the
> > description.
> > ---
> > .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > index a1f54dbae3f3..b93f49f1fa0a 100644
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -25,6 +25,11 @@ properties:
> > - renesas,r8a779a0-gpu
> > - const: img,img-ge7800
> > - const: img,img-rogue
> > + - items:
> > + - enum:
> > + - renesas,r8a77995-gpu
> > + - const: img,img-ge8300
> > + - const: img,img-rogue
> > - items:
> > - enum:
> > - ti,am62-gpu
> > @@ -114,6 +119,7 @@ allOf:
> > contains:
> > enum:
> > - img,img-ge7800
> > + - img,img-ge8300
> > - img,img-gx6250
> > - thead,th1520-gpu
> > then:
> > @@ -159,14 +165,14 @@ allOf:
> > - if:
> > properties:
> > compatible:
> > - contains:
>
> The 'contains' node should have been kept, my bad. I wonder why 'make
> dt_binding_check' or `make dtbs_check' did not catch it. Sorry for the
> noise.
Because it's valid syntax, the contains syntax means that it'll match
against things that use the listed compatibles as fallbacks. What you
have done works for exact matches only.
I think recently Rob said the contains syntax is preferred, with some
rationale that escapes me.
>
> > - const: thead,th1520-gpu
> > + enum:
> > + - renesas,r8a77995-gpu
> > + - thead,th1520-gpu
> > then:
> > properties:
> > power-domains:
> > items:
> > - - description: The single, unified power domain for the GPU on the
> > - TH1520 SoC, integrating all internal IP power domains.
> > + - description: The single, unified power domain for the GPU.
> > power-domain-names: false
> > required:
> > - power-domains
> > --
> > 2.54.0
> >
>
> --
> Kind Regards,
> Niklas Söderlund
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
From: Frank Li @ 2026-06-17 15:28 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, devicetree, robh, linux-i3c, Frank.Li, conor+dt
In-Reply-To: <20260617111846.EAB851F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:18:46AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] The SMBus transaction wait logic uses a fixed sleep instead of polling the hardware completion flag, leading to premature reads of incomplete transactions and data corruption.
> - [High] Overwriting the parent device's `of_node` pointer iteratively causes a memory leak of the original parent node, a double-free of the target port nodes, and state corruption.
> - [Medium] Potential `of_node` reference leak in `p3h2x4x_get_target_port_dt_conf()` if multiple child nodes specify the same target port index.
> - [Medium] The hub's protected registers remain permanently unlocked if the hardware configuration or bus registration fails during probe.
> - [Low] Comment describes a read followed by a write, but the code implements a write followed by a read.
> --
>
> commit 1c982ecf71c6fb3b765caf255191a821a5e37cb5
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
>
> Add I3C hub functionality for the NXP P3H2x4x family of multiport hubs.
> These devices support downstream target ports that can be configured
> as I3C, I2C, or SMBus.
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> > new file mode 100644
> > index 0000000000000..05e19bff20d5c
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> [ ... ]
> > +static void p3h2x4x_get_target_port_dt_conf(struct device *dev,
> > + const struct device_node *node)
> > +{
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
> > + u64 tp_port;
> > +
> > + for_each_available_child_of_node_scoped(node, dev_node) {
> > + if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
> > + continue;
> > +
> > + if (tp_port < P3H2X4X_TP_MAX_COUNT) {
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].of_node = of_node_get(dev_node);
>
> [Severity: Medium]
> Could this leak the of_node reference if a malformed device tree specifies
> multiple child nodes with the same target port index?
>
> If multiple child nodes specify the same reg value, the pointer is
> overwritten with a new of_node_get() result, which abandons the previous
> reference.
>
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_mask = P3H2X4X_SET_BIT(tp_port);
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].p3h2x4x_i3c_hub = p3h2x4x_i3c_hub;
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_port = tp_port;
> > + }
> > + }
> > +}
> [ ... ]
> > +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> > +{
> [ ... ]
> > + /* Unlock access to protected registers */
> > + ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + P3H2X4X_REGISTERS_UNLOCK_CODE);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to unlock HUB's protected registers\n");
> > +
> > + ret = p3h2x4x_configure_hw(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to configure the HUB\n");
>
> [Severity: Medium]
> Are the hub's protected registers left permanently unlocked if probe fails
> during p3h2x4x_configure_hw() or bus registration?
>
> The driver unlocks the hardware registers at the start of probe and locks
> them at the very end. However, if any initialization step fails and returns
> an error, the function exits immediately without locking the registers again.
>
> Could this be mitigated by using a devm_add_action_or_reset() callback or
> explicit error handling labels to ensure the registers are safely relocked?
It is not big problem if device can support P3H2X4X_REGISTERS_UNLOCK_CODE
twice without LOCK. The driver should recover from failure state.
>
> > +
> > + /* Register virtual I3C master controllers for I3C target ports */
> > + if (p3h2x4x->i3cdev) {
> [ ... ]
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> > new file mode 100644
> > index 0000000000000..8adb235b992c0
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> [ ... ]
> > +int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
> > +{
> [ ... ]
> > + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
> > + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> > + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> > + continue;
> > +
> > + /* Assign DT node for this TP */
> > + p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
>
> [Severity: High]
> Does overwriting the parent device's of_node pointer iteratively cause a
> memory leak of the original parent node and a double-free of the target port
> nodes?
Look like AI is correct.
>
> The loop over target ports directly mutates the shared pdev->dev.of_node
> pointer which was populated in probe using device_set_of_node_from_dev().
> This leaks the reference to the original parent node, and leaves
> dev->of_node pointing to the last target port node.
>
> When the driver is unbound, the driver core will drop a reference on
> dev->of_node, but p3h2x4x_put_target_port_of_nodes() also drops a reference
> on the same node, which can cause a use-after-free or double-free refcount
> underflow. Mutating dev->of_node also creates a race with sysfs/uevent.
>
> Should the driver assign the node to controller->dev.of_node directly
> instead of mutating the parent's pointer?
>
> > +
> > + struct i3c_hub_controller *hub_controller =
> > + &p3h2x4x_hub->tp_bus[tp].hub_controller;
> [ ... ]
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > new file mode 100644
> > index 0000000000000..cf664425af071
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> [ ... ]
> > +static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
> > + u8 target_port_status,
> > + u8 data_length)
> > +{
> > + u32 status_read;
> > + u8 status;
> > + int ret;
> > +
> > + fsleep(P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
> > +
> > + ret = regmap_read(hub->regmap, target_port_status, &status_read);
>
> [Severity: High]
> Does using a hardcoded sleep based on bytes transferred instead of polling a
> hardware completion status bit lead to premature reads of incomplete
> transactions?
>
> If the downstream device performs clock stretching or is slow to respond,
> the transfer will take longer than the hardcoded timeout. The driver then
> reads target_port_status and shifts to check the status bits.
>
> Since an ongoing transaction leaves the status bits at 0
> (P3H2X4X_SMBUS_CNTRL_STATUS_TXN_OK), the driver will incorrectly assume the
> transaction has finished successfully. It will then proceed to read stale or
> uninitialized memory buffers and return garbage data.
>
> Could this be updated to use a polling loop checking the finish flag?
Yes, it should use read_poll_timeout here.
>
> > + if (ret)
> > + return ret;
> > +
> > + status = (u8)status_read;
> [ ... ]
> > +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > +{
> [ ... ]
> > + rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
> > + if (!rw) {
> > + /* If a read message is immediately followed by a write message to
> > + * the same address, consider combining them into a single transaction.
> > + */
>
> [Severity: Low]
> This isn't a bug, but the comment describes a read followed by a write while
> the code implements a write followed by a read.
>
> The actual code block checks !rw for the first message and flags & I2C_M_RD
> for the second message, which accurately handles a write message immediately
> followed by a read message.
small issue, easy to fix it.
Frank
>
> > + if (msg_count + 1 < num &&
> > + msgs[msg_count].addr == msgs[msg_count + 1].addr &&
> > + (msgs[msg_count + 1].flags & I2C_M_RD)) {
> > + if (msgs[msg_count].len + msgs[msg_count + 1].len >
> > + P3H2X4X_SMBUS_PAYLOAD_SIZE)
> > + return -EINVAL;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
From: Frank Li @ 2026-06-17 15:19 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, robh, conor+dt, Frank.Li, devicetree, linux-i3c
In-Reply-To: <20260617111804.ABFF71F00A3D@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:18:04AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
> - [Critical] Deterministic NULL pointer dereference in `i3c_hub_free_ibi()` due to recursive callback chain.
> - [Critical] Memory leak and double-free memory corruption in `i3c_hub_request_ibi()` error paths.
> - [Critical] Critical race condition and parent bus state corruption in `i3c_hub_master_priv_xfers()` due to unlocked address mutation.
> - [Critical] Unimplemented `i3c_hub_recycle_ibi_slot()` permanently leaks hardware IBI slots.
> - [Critical] Architectural bypass in `i3c_hub_do_daa()` attaches dynamically discovered devices to the parent bus instead of the virtual hub bus.
> - [High] Calling `i3c_hub_enable_port()` outside of any locks allows concurrent operations to physically enable multiple downstream ports simultaneously, breaking electrical isolation.
> - [High] Silently dropping broadcast RSTDAA commands prevents resetting of downstream devices.
> - [Medium] Accepting I2C devices while unconditionally rejecting all I2C transfers violates the API contract.
> --
>
> commit 2a201a9e0e9a55b31467d874340085240b34afc3
> Author: Lakshay Piplani <lakshay.piplani@nxp.com>
>
> i3c: hub: Add support for the I3C interface in the I3C hub
>
> This patch adds virtual I3C bus support for the hub and provides an
> interface to enable or disable downstream ports.
>
> > diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> > new file mode 100644
> > index 0000000000000..e8744b5172340
> > --- /dev/null
> > +++ b/drivers/i3c/hub.c
> > @@ -0,0 +1,483 @@
>
> [ ... ]
>
> > +static int i3c_hub_do_daa(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_do_daa(parent);
>
> [Severity: Critical]
> Since `i3c_master_do_daa()` executes the DAA sequence natively on the parent
> controller, will the newly discovered devices be allocated and attached
> directly to the parent bus instead of this virtual hub's bus?
>
> If they are attached to the parent bus, won't subsequent transfers completely
> bypass `i3c_hub_master_priv_xfers()`, causing communication with these
> dynamically enumerated devices to fail?
I forget some background, does all devices under hub port shared the same
I3C address space? that's means dynmatic address is difference for all
difference ports?
I2C switcher can support the same address at difference port. look like
diffenent port devices should attach to hub's bus. Only temparily attach
to parent during transfer?
Frank
>
> [Severity: High]
> Is there a risk of breaking electrical isolation here? `i3c_hub_enable_port()`
> is called before acquiring any parent bus locks.
>
> If multiple threads operate on different virtual controllers concurrently,
> could they physically enable multiple downstream ports at the same time
> before the bus lock is acquired?
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_send_ccc_cmd(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_send_ccc_cmd(parent, cmd);
>
> [Severity: High]
> Similar to `i3c_hub_do_daa()`, does calling `i3c_hub_enable_port()` without a
> lock risk enabling multiple ports concurrently if multiple CCC commands are
> issued on different ports?
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_master_priv_xfers(struct i3c_dev_desc *dev,
> > + struct i3c_xfer *xfers,
> > + int nxfers,
> > + enum i3c_xfer_mode mode)
> > +{
>
> [ ... ]
>
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = target_addr;
> > +
> > + ret = i3c_master_reattach_i3c_dev_locked(hub_dev, hub_addr);
> > + up_write(&parent->bus.lock);
> > + if (ret) {
> > + hub_dev->info.dyn_addr = hub_addr;
> > + goto disable;
> > + }
> > + }
> > +
> > + ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
>
> [Severity: Critical]
> Does dropping `parent->bus.lock` before calling `i3c_device_do_xfers()` expose
> a race condition?
>
> If multiple downstream ports initiate transfers concurrently, could they
> overwrite the shared `hub_dev->info.dyn_addr` before the transfer completes,
> causing packets to be routed to the wrong physical device?
>
> > +
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = hub_addr;
> > +
> > + xfer_ret = i3c_master_reattach_i3c_dev_locked(hub_dev,
> > + target_addr);
>
> [Severity: Critical]
> Could passing `target_addr` as the old address corrupt the parent bus state?
>
> `i3c_master_reattach_i3c_dev_locked()` frees the old address slot in the bus
> slot map. Will this erroneously mark `target_addr` as `I3C_ADDR_SLOT_FREE`
> on the parent bus?
>
> > +
> > + up_write(&parent->bus.lock);
> > + if (!ret)
> > + ret = xfer_ret;
> > + }
> > +disable:
> > + i3c_hub_disable_port(controller);
> > + return ret;
> > +}
> > +
> > +static int i3c_hub_attach_i2c_dev(struct i2c_dev_desc *dev)
> > +{
> > + return 0;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_i2c_xfers(struct i2c_dev_desc *dev,
> > + struct i2c_msg *xfers, int nxfers)
> > +{
> > + return -EOPNOTSUPP;
> > +}
>
> [Severity: Medium]
> Is it intentional to return `0` from `i3c_hub_attach_i2c_dev()` while rejecting
> all I2C transfers with `-EOPNOTSUPP`?
>
> Would it be safer to return an error from the attach callback to cleanly
> prevent I2C driver binding if the hub does not support I2C forwarding?
>
> [ ... ]
>
> > +static int i3c_hub_master_send_ccc_cmd(struct i3c_master_controller *controller,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + struct i3c_hub_controller *hub_controller;
> > + struct i3c_hub *hub;
> > +
> > + hub_controller = dev_get_drvdata(&controller->dev);
> > + if (!hub_controller || !hub_controller->hub)
> > + return -ENODEV;
> > +
> > + hub = hub_controller->hub;
> > +
> > + if (!hub->hub_dev)
> > + return -ENODEV;
> > +
> > + if (cmd->id == I3C_CCC_RSTDAA(true))
> > + return 0;
>
> [Severity: High]
> Does silently dropping the broadcast RSTDAA command prevent downstream devices
> from being correctly reset by the hub?
>
> If the command is dropped instead of being forwarded to the downstream ports,
> will devices retain their old dynamic addresses and fail to re-initialize
> during hot-plug recovery?
>
> > +
> > + return i3c_hub_send_ccc_cmd(hub, controller, cmd);
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
> > + const struct i3c_ibi_setup *req)
> > +{
>
> [ ... ]
>
> > + ret = i3c_master_direct_attach_i3c_dev_locked(parent, desc);
> > + if (ret) {
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > + return ret;
> > + }
> > +
> > + kfree(desc->ibi);
>
> [Severity: Critical]
> Does freeing `desc->ibi` here leak `desc->ibi->wq`?
>
> When the core function `i3c_dev_request_ibi_locked()` allocates `ibi`, it also
> allocates `ibi->wq`.
>
> Also, if the inner call to `i3c_dev_request_ibi_locked()` fails, this function
> returns an error to the original core caller. Will the outer caller's error
> path execute `kfree(ibi)` on its local variable, resulting in a double-free
> since it was already freed here?
>
> > + desc->ibi = NULL;
> > + ret = i3c_dev_request_ibi_locked(desc, req);
> > + if (ret)
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
> > +
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > +
> > + up_write(&parent->bus.lock);
> > +
> > + return ret;
> > +}
> > +
> > +static void i3c_hub_free_ibi(struct i3c_dev_desc *desc)
> > +{
>
> [ ... ]
>
> > + i3c_hub_enable_port(controller);
> > +
> > + down_write(&parent->bus.lock);
> > + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
> > + i3c_dev_free_ibi_locked(desc);
>
> [Severity: Critical]
> Can this recursive call to `i3c_dev_free_ibi_locked(desc)` cause a null
> pointer dereference?
>
> When the core initially calls `i3c_dev_free_ibi_locked(desc)`, it invokes
> this hub callback. The inner call here successfully frees `desc->ibi` and sets
> it to null.
>
> When control returns to the outer core function, it unconditionally evaluates:
>
> if (dev->ibi->wq) {
>
> Will this immediately dereference the null pointer?
>
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > +
> > + i3c_hub_disable_port(controller);
> > +}
>
> [ ... ]
>
> > +static void i3c_hub_recycle_ibi_slot(struct i3c_dev_desc *desc,
> > + struct i3c_ibi_slot *slot)
> > +{
> > +}
>
> [Severity: Critical]
> Does leaving this function empty permanently leak hardware IBI slots?
>
> When the core finishes processing an IBI, it returns the slot to the virtual
> controller. Because this doesn't forward the recycle request to the parent
> hardware controller, will the parent controller eventually exhaust its hardware
> slots and permanently stop processing IBIs?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply
* Re: [PATCH v8 6/6] leds: is31fl32xx: Move pwm frequency setting to init_regs()
From: Lee Jones @ 2026-06-17 15:17 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-7-jerrysteve1101@gmail.com>
/* Sashiko Automation: Reviewed (2 Findings) */
And this one please.
On Sun, 07 Jun 2026, Jun Yan wrote:
> commit a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> swapped the order of is31fl32xx_parse_dt() and is31fl32xx_init_regs().
[Severity: High]
This isn't a bug introduced by this patch, but registering LED class devices
before hardware initialization completes exposes the LEDs prematurely.
In is31fl32xx_probe(), is31fl32xx_parse_dt() is called, which internally
executes devm_led_classdev_register_ext() for each LED:
drivers/leds/leds-is31fl32xx.c:is31fl32xx_probe() {
...
ret = is31fl32xx_parse_dt(dev, priv);
if (ret)
return ret;
ret = is31fl32xx_init_regs(priv);
...
}
This registers the LEDs to the subsystem, potentially immediately triggering
led_set_brightness_nopm() via default state settings (e.g., "default-on").
This schedules an asynchronous work item to configure the LED over I2C.
However, is31fl32xx_probe() then immediately calls is31fl32xx_init_regs(),
which performs a full hardware reset (is31fl32xx_reset_regs()).
If the hardware reset races with or occurs after the default state triggers,
will the initial LED states be wiped out? Must the hardware be fully
initialized and reset before registering the LED devices?
>
> This causes the PWM frequency configuration programmed in
> is31fl32xx_parse_dt() to be overwritten by the register reset
> operation performed in is31fl32xx_init_regs().
>
> Move the PWM frequency setting logic from is31fl32xx_parse_dt() to
> is31fl32xx_init_regs() and separates device tree parsing from hardware
> initialization.
>
> Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index cbda7edbfb48..b43879bffb06 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
[Severity: High]
This is a pre-existing issue, but there is an incorrect use of max()
instead of min() in is31fl3293_reset():
drivers/leds/leds-is31fl32xx.c:is31fl3293_reset() {
...
for (i = 0; i < priv->num_leds; i++) {
struct is31fl32xx_led_data *led_data = &priv->leds[i];
int current_level_reg = IS31FL3293_CL_REG + led_data->channel - 1;
int microamp = max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP);
int current_level = (microamp * 0xff) / IS31FL3293_MAX_MICROAMP;
ret = is31fl32xx_write(priv, current_level_reg, current_level);
...
}
This bounds the user-specified current limit against the hardware maximum of
20,000 µA (IS31FL3293_MAX_MICROAMP) using max(), forcing the current to
always be at least the max chip rating.
If an LED is only rated for 5mA, does this force it to 20mA, which could
physically damage it?
Furthermore, if a user requests a value larger than 20mA (e.g., 30,000 µA),
max() allows it. The subsequent calculation exceeds 255. Because
is31fl32xx_write() takes a u8 argument, this value is truncated, resulting in
unexpectedly lower current.
Should the code use min() to properly enforce the hardware's maximum rating?
> @@ -63,6 +63,7 @@ struct is31fl32xx_priv {
> struct i2c_client *client;
> struct gpio_desc *powerdown_gpio;
> unsigned int num_leds;
> + bool pwm_22khz;
> struct is31fl32xx_led_data leds[];
> };
>
> @@ -346,6 +347,13 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
> if (ret)
> return ret;
>
> + if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) && priv->pwm_22khz) {
> + ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
> + IS31FL32XX_PWM_FREQUENCY_22KHZ);
> + if (ret)
> + return ret;
> + }
> +
> /*
> * Set enable bit for all channels.
> * We will control state with PWM registers alone.
> @@ -420,7 +428,6 @@ static struct is31fl32xx_led_data *is31fl32xx_find_led_data(
> static int is31fl32xx_parse_dt(struct device *dev,
> struct is31fl32xx_priv *priv)
> {
> - const struct is31fl32xx_chipdef *cdef = priv->cdef;
> int ret = 0;
>
> /*
> @@ -432,17 +439,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
> return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
> "Failed to get 'powerdown' GPIO\n");
>
> - if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
> - of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
> -
> - ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
> - IS31FL32XX_PWM_FREQUENCY_22KHZ);
> -
> - if (ret) {
> - dev_err(dev, "Failed to write output PWM frequency register\n");
> - return ret;
> - }
> - }
> + priv->pwm_22khz = of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm");
>
> for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
> struct led_init_data init_data = {};
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* [PATCH v2] dt-bindings: arm: qcom: Document Hawi SoC and its reference boards
From: Mukesh Ojha @ 2026-06-17 15:16 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Mukesh Ojha
Add Hawi SoC and its reference boards to the Qualcomm binding.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
Changes in v2: https://lore.kernel.org/lkml/20260617080147.1657632-1-mukesh.ojha@oss.qualcomm.com/
- Fixed the position of the Documentation.
- Corrected the commit text.
Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 50cc18a6ec5e..e229c0097e6f 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -303,6 +303,11 @@ properties:
- xiaomi,sagit
- const: qcom,msm8998
+ - items:
+ - enum:
+ - qcom,hawi-mtp
+ - const: qcom,hawi
+
- items:
- enum:
- 8dev,jalapeno
--
2.53.0
^ permalink raw reply related
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