Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 net-next 1/3] dt-bindings: vendor-prefixes: add SiTime Corporation
From: Krzysztof Kozlowski @ 2026-05-21 21:12 UTC (permalink / raw)
  To: Ali Rouhi
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <CALFSGuoWkHYmtrouvLk2M7bWS1=qTSPUn5GuabFvK92cPBfuZA@mail.gmail.com>

On 21/05/2026 22:40, Ali Rouhi wrote:
>> Mismatch in From/DCO.
> 
> Acknowledged — Gmail SMTP relay issue, will fix for v3.

Do not top post.

> 
>> It looks like you received a tag and forgot to add it.
> 
> Thank you for the reminder.  Will carry Conor's tag in v3:
> 
>   Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Best regards,
> Ali
> 
> On Thu, May 21, 2026 at 3:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Wed, May 20, 2026 at 12:19:41PM -0700, Ali Rouhi wrote:
>>> Add vendor prefix for SiTime Corporation, manufacturer of
>>> programmable clock generators and MEMS oscillators.
>>>
>>> Signed-off-by: Ali Rouhi <arouhi@sitime.com>
>>
>> Mismatch in From/DCO.
>>
>> Best regards,
>> Krzysztof
>>


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v2 net-next 2/3] dt-bindings: dpll: add SiTime SiT9531x clock generator
From: Krzysztof Kozlowski @ 2026-05-21 21:12 UTC (permalink / raw)
  To: Ali Rouhi
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <CALFSGupB_eRHQ=h2PzwiLE1gTvd7kZbBP+eWOVWVxapVjzgeXw@mail.gmail.com>

On 21/05/2026 22:38, Ali Rouhi wrote:
>> Mismatched DCO. Use consistent identity or fix your commits.
> 
> Apologies — Gmail's SMTP relay rewrites the From header to my
> personal address (rouhi.ali@gmail.com).  I'm working with IT to

I don't think so. Gmail does not do it. Properly configured Gmail SMTP
should work fine with two From: headers.

> get corporate SMTP credentials so the From matches the Signed-off-by
> (arouhi@sitime.com) in v3.
> 
>> Same as last time. Why are you describing drivers?
> 
> Will drop the clocks description entirely — maxItems and
> clock-names are sufficient.

You completely cut the context. No clue what was there.

> 
>> Drop node. Wasn't here before, so why did you add it?
>> Drop.
> 
> Will drop both xo and xo2 fixed-clock nodes from the examples.
> 
> Thanks for the review.
> 
> Best regards,
> Ali
> 
> On Thu, May 21, 2026 at 12:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 20/05/2026 21:19, Ali Rouhi wrote:

Heh, you just top posted.

Do not top post ever.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 6/9] pinctrl: ultrarisc: Add UltraRISC DP1000 pinctrl driver
From: Krzysztof Kozlowski @ 2026-05-21 21:09 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-6-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:18, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
> 
> Add pinctrl driver for UltraRISC DP1000 pinctrl controller.
> 
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
>  MAINTAINERS                                   |   1 +
>  drivers/pinctrl/Kconfig                       |   1 +
>  drivers/pinctrl/Makefile                      |   1 +
>  drivers/pinctrl/ultrarisc/Kconfig             |  23 +
>  drivers/pinctrl/ultrarisc/Makefile            |   4 +
>  drivers/pinctrl/ultrarisc/pinctrl-dp1000.c    | 112 ++++
>  drivers/pinctrl/ultrarisc/pinctrl-ultrarisc.c | 746 ++++++++++++++++++++++++++
>  drivers/pinctrl/ultrarisc/pinctrl-ultrarisc.h |  71 +++
>  8 files changed, 959 insertions(+)

Organize your patchset correctly. Such style:
DTS
pinctrl
DTS

is very confusing and discouraged. See submitting patches in DT directory.


> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 832e01898ae5..ecd87d58f28c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27364,6 +27364,7 @@ M:	Jia Wang <wangjia@ultrarisc.com>
>  L:	linux-gpio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml
> +F:	drivers/pinctrl/ultrarisc/*
>  F:	include/dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h
>  
>  ULTRATRONIK BOARD SUPPORT
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 03f2e3ee065f..76105be8b395 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -711,5 +711,6 @@ source "drivers/pinctrl/ti/Kconfig"
>  source "drivers/pinctrl/uniphier/Kconfig"
>  source "drivers/pinctrl/visconti/Kconfig"
>  source "drivers/pinctrl/vt8500/Kconfig"
> +source "drivers/pinctrl/ultrarisc/Kconfig"

u < v

Do not add to the end of files. You are messing how this file is sorted.

>  
>  endif
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f7d5d5f76d0c..4df3e52518ea 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -98,3 +98,4 @@ obj-y				+= ti/
>  obj-$(CONFIG_PINCTRL_UNIPHIER)	+= uniphier/
>  obj-$(CONFIG_PINCTRL_VISCONTI)	+= visconti/
>  obj-$(CONFIG_ARCH_VT8500)	+= vt8500/
> +obj-$(CONFIG_ARCH_ULTRARISC)	+= ultrarisc/

Missing compile test. Don't use that style. See other decent SoC vendors
how they solved that problem.

Also, do not add to the end of the files. Keep things sorted, more or less.


> diff --git a/drivers/pinctrl/ultrarisc/Kconfig b/drivers/pinctrl/ultrarisc/Kconfig
> new file mode 100644
> index 000000000000..ba8747b90127
> --- /dev/null
> +++ b/drivers/pinctrl/ultrarisc/Kconfig
> @@ -0,0 +1,23 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config PINCTRL_ULTRARISC
> +	tristate
> +	depends on OF
> +	select PINMUX
> +	select GENERIC_PINCTRL_GROUPS
> +	select GENERIC_PINCONF
> +	select GENERIC_PINMUX_FUNCTIONS
> +	select GPIOLIB
> +	select IRQ_DOMAIN_HIERARCHY
> +	select MFD_SYSCON
> +


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 5/9] riscv: dts: ultrarisc: Add initial device tree for UltraRISC DP1000
From: Krzysztof Kozlowski @ 2026-05-21 21:05 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-5-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:18, Jia Wang via B4 Relay wrote:
> +
> +		cluster0_l3: l3-cache0 {
> +			/* L3 cache:

Use Linux coding style. In other places as well. Even netdev does not
use that style anymore!

> +			 * cache-unified, block-size 64B
> +			 * 16-way set associative, size 4MB
> +			 * per-cluster.
> +			 */
> +			compatible = "cache";
> +			cache-block-size = <64>;
> +			cache-level = <3>;
> +			cache-size = <0x400000>;
> +			cache-sets = <0x1000>;
> +			cache-unified;
> +			next-level-cache = <&l4_cache>;
> +		};
> +
> +		cluster1_l3: l3-cache1 {
> +			/* L3 cache:
> +			 * cache-unified, block-size 64B
> +			 * 16-way set associative, size 4MB
> +			 * per-cluster.
> +			 */
> +			compatible = "cache";
> +			cache-block-size = <64>;
> +			cache-level = <3>;
> +			cache-size = <0x400000>;
> +			cache-sets = <0x1000>;
> +			cache-unified;
> +			next-level-cache = <&l4_cache>;
> +		};
> +	};
> +
> +	clocks {
> +		device_clk: device_clk {

You need to follow DTS coding style.

Anyway, something like "device clock" is completely uninformative or
even incorrect. I really doubt such thing as "device clock" exists...

Please use name for all fixed clocks which matches current format
recommendation: 'clock-<freq>' (see also the pattern in the binding for
any other options).

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml

> +			compatible = "fixed-clock";
> +			clock-frequency = <62500000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		timer_clk: timer_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <50000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		csr_clk: csr_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +	};
> +
> +	l4_cache: l4-cache {
> +		/* L4 cache:
> +		 * cache-unified, block-size 64B
> +		 * 16-way set associative, size 16MB
> +		 * shared by the SoC.
> +		 */
> +		compatible = "cache";
> +		cache-block-size = <64>;
> +		cache-level = <4>;
> +		cache-size = <0x1000000>;
> +		cache-sets = <0x4000>;
> +		cache-unified;
> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x00 0x80000000 0x4 0x00000000>;
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		ranges;
> +		#address-cells = <0x02>;
> +		#size-cells = <0x02>;

<2> This is not hex and definitely does not need padding with 0.

> +
> +		clint: clint@8000000 {
> +			compatible = "sifive,clint0", "riscv,clint0";
> +			reg = <0x00 0x8000000 0x00 0x100000>;
> +			interrupts-extended = <&cpu0_intc 0x03>, <&cpu0_intc 0x07>,
> +					      <&cpu1_intc 0x03>, <&cpu1_intc 0x07>,
> +					      <&cpu2_intc 0x03>, <&cpu2_intc 0x07>,
> +					      <&cpu3_intc 0x03>, <&cpu3_intc 0x07>,
> +					      <&cpu4_intc 0x03>, <&cpu4_intc 0x07>,
> +					      <&cpu5_intc 0x03>, <&cpu5_intc 0x07>,
> +					      <&cpu6_intc 0x03>, <&cpu6_intc 0x07>,
> +					      <&cpu7_intc 0x03>, <&cpu7_intc 0x07>;
> +		};
> +
> +		plic: plic@9000000 {
> +			compatible = "ultrarisc,dp1000-plic", "ultrarisc,cp100-plic";
> +			reg = <0x00 0x9000000 0x00 0x4000000>;
> +			#interrupt-cells = <1>;
> +			#address-cells = <0>;

So hex or not hex? Please fix your DTS so it is consistent.

> +			interrupt-controller;
> +			interrupts-extended = <&cpu0_intc 0xb>, <&cpu0_intc 0x9>, <&cpu0_intc 0xa>,
> +					      <&cpu1_intc 0xb>, <&cpu1_intc 0x9>, <&cpu1_intc 0xa>,
> +					      <&cpu2_intc 0xb>, <&cpu2_intc 0x9>, <&cpu2_intc 0xa>,
> +					      <&cpu3_intc 0xb>, <&cpu3_intc 0x9>, <&cpu3_intc 0xa>,
> +					      <&cpu4_intc 0xb>, <&cpu4_intc 0x9>, <&cpu4_intc 0xa>,
> +					      <&cpu5_intc 0xb>, <&cpu5_intc 0x9>, <&cpu5_intc 0xa>,
> +					      <&cpu6_intc 0xb>, <&cpu6_intc 0x9>, <&cpu6_intc 0xa>,
> +					      <&cpu7_intc 0xb>, <&cpu7_intc 0x9>, <&cpu7_intc 0xa>;
> +			riscv,ndev = <160>;
> +		};
> +
> +		pmx0: pinmux@11081000 {
> +			compatible = "ultrarisc,dp1000-pinctrl";
> +			reg = <0x0 0x11081000  0x0 0x1000>;
> +			#pinctrl-cells = <2>;
> +		};
> +
> +		gpio: gpio@20200000 {
> +			compatible = "snps,dw-apb-gpio";
> +			reg = <0x0 0x20200000 0x0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-names = "bus", "db";
> +			clocks = <&csr_clk>, <&device_clk>;
> +
> +			gpio_a: gpio-port@0 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <0>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <16>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupt-parent = <&plic>;
> +				interrupts = <34>;
> +				gpio-ranges = <&pmx0 0 0 16>;
> +			};
> +
> +			gpio_b: gpio-port@1 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <1>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <8>;
> +				gpio-ranges = <&pmx0 16 0 8>;
> +			};
> +
> +			gpio_c: gpio-port@2 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <2>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <8>;
> +				gpio-ranges = <&pmx0 24 0 8>;
> +			};
> +
> +			gpio_d: gpio-port@3 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <3>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <8>;
> +				gpio-ranges = <&pmx0 32 0 8>;
> +			};
> +		};
> +
> +		uart0: serial@20300000 {
> +			compatible = "ultrarisc,dp1000-uart", "snps,dw-apb-uart";
> +			reg = <0x00 0x20300000 0x00 0x10000>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <17>;
> +			clock-frequency = <62500000>;
> +			reg-io-width = <0x04>;
> +			reg-shift = <0x02>;
> +		};
> +
> +		uart1: serial@20310000 {
> +			compatible = "ultrarisc,dp1000-uart", "snps,dw-apb-uart";
> +			reg = <0x00 0x20310000 0x00 0x10000>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <18>;
> +			clock-frequency = <62500000>;
> +			reg-io-width = <0x04>;
> +			reg-shift = <0x02>;
> +		};
> +
> +		uart2: serial@20400000 {
> +			compatible = "ultrarisc,dp1000-uart", "snps,dw-apb-uart";
> +			reg = <0x00 0x20400000 0x00 0x10000>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <25>;
> +			clock-frequency = <62500000>;
> +			reg-io-width = <0x04>;
> +			reg-shift = <0x02>;
> +		};
> +
> +		uart3: serial@20410000 {
> +			compatible = "ultrarisc,dp1000-uart", "snps,dw-apb-uart";
> +			reg = <0x00 0x20410000 0x00 0x10000>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <26>;
> +			clock-frequency = <62500000>;
> +			reg-io-width = <0x04>;
> +			reg-shift = <0x02>;
> +		};
> +
> +		spi0: spi@20320000 {
> +			compatible = "snps,dw-apb-ssi";
> +			reg = <0x0 0x20320000 0x0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <19>;
> +			num-cs = <3>;
> +		};
> +
> +		spi1: spi@20420000 {
> +			compatible = "snps,dw-apb-ssi";
> +			reg = <0x0 0x20420000 0x0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <27>;
> +			num-cs = <3>;
> +		};
> +
> +		i2c0: i2c@20330000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0x20330000 0x0 0x100>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <20>;
> +		};
> +
> +		i2c1: i2c@20340000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0x20340000 0x0 0x100>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <21>;
> +		};
> +
> +		i2c2: i2c@20430000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0x20430000 0x0 0x100>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <28>;
> +		};
> +
> +		i2c3: i2c@20440000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0x20440000 0x0 0x100>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <29>;
> +		};
> +
> +		pcie_x16: pcie@21000000 {
> +			compatible = "ultrarisc,dp1000-pcie";
> +			reg = <0x0 0x21000000 0x0 0x01000000>,
> +			      <0x0 0x4fff0000 0x0 0x00010000>;
> +			reg-names = "dbi", "config";
> +			ranges = <0x81000000  0x0 0x4fbf0000  0x0 0x4fbf0000  0x0 0x00400000>,
> +				 <0x82000000  0x0 0x40000000  0x0 0x40000000  0x0 0x0fbf0000>,
> +				 <0xc3000000 0x40 0x00000000 0x40 0x00000000  0xd 0x00000000>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			#interrupt-cells = <1>;
> +			device_type = "pci";
> +			dma-coherent;
> +			bus-range = <0x0 0xff>;
> +			num-lanes = <16>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <43>, <44>, <45>, <46>, <47>;
> +			interrupt-names = "msi", "inta", "intb", "intc", "intd";
> +			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +			interrupt-map = <0x0 0x0 0x0 0x1 &plic 44>,
> +					<0x0 0x0 0x0 0x2 &plic 45>,
> +					<0x0 0x0 0x0 0x3 &plic 46>,
> +					<0x0 0x0 0x0 0x4 &plic 47>;

Why PCIe without any devices is enabled? That's a bus.

Please look how other DTS are written because you created something
pretty different than entire kernel style.

> +		};
> +
> +		pcie_x4a: pcie@23000000 {
> +			compatible = "ultrarisc,dp1000-pcie";
> +			reg = <0x0 0x23000000 0x0 0x01000000>,
> +			      <0x0 0x6fff0000 0x0 0x00010000>;
> +			reg-names = "dbi", "config";
> +			ranges = <0x81000000  0x0 0x6fbf0000  0x0 0x6fbf0000  0x0 0x00400000>,
> +				 <0x82000000  0x0 0x60000000  0x0 0x60000000  0x0 0x0fbf0000>,
> +				 <0xc3000000 0x80 0x00000000 0x80 0x00000000  0xd 0x00000000>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			#interrupt-cells = <1>;
> +			device_type = "pci";
> +			dma-coherent;
> +			bus-range = <0x0 0xff>;
> +			num-lanes = <4>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <63>, <64>, <65>, <66>, <67>;
> +			interrupt-names = "msi", "inta", "intb", "intc", "intd";
> +			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +			interrupt-map = <0x0 0x0 0x0 0x1 &plic 64>,
> +					<0x0 0x0 0x0 0x2 &plic 65>,
> +					<0x0 0x0 0x0 0x3 &plic 66>,
> +					<0x0 0x0 0x0 0x4 &plic 67>;
> +		};
> +
> +		pcie_x4b: pcie@24000000 {
> +			compatible = "ultrarisc,dp1000-pcie";
> +			reg = <0x0 0x24000000 0x0 0x01000000>,
> +			      <0x0 0x7fff0000 0x0 0x00010000>;
> +			reg-names = "dbi", "config";
> +			ranges = <0x81000000  0x0 0x7fbf0000  0x0 0x7fbf0000 0x0 0x00400000>,
> +				 <0x82000000  0x0 0x70000000  0x0 0x70000000 0x0 0x0fbf0000>,
> +				 <0xc3000000 0xc0 0x00000000 0xc0 0x00000000 0xd 0x00000000>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			#interrupt-cells = <1>;
> +			device_type = "pci";
> +			dma-coherent;
> +			bus-range = <0x0 0xff>;
> +			num-lanes = <4>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <73>, <74>, <75>, <76>, <77>;
> +			interrupt-names = "msi", "inta", "intb", "intc", "intd";
> +			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +			interrupt-map = <0x0 0x0 0x0 0x1 &plic 74>,
> +					<0x0 0x0 0x0 0x2 &plic 75>,
> +					<0x0 0x0 0x0 0x3 &plic 76>,
> +					<0x0 0x0 0x0 0x4 &plic 77>;
> +		};
> +
> +		ethernet: ethernet@38000000 {
> +			compatible = "snps,dwmac", "snps,dwmac-5.10a";
> +			reg = <0x00 0x38000000 0x00 0x1000000>;
> +			clocks = <&csr_clk>;
> +			clock-names = "stmmaceth";
> +			interrupt-parent = <&plic>;
> +			interrupts = <84>;
> +			interrupt-names = "macirq";
> +			local-mac-address = [ff ff ff ff ff ff];

Drop. Not a property of the SoC.

> +			max-speed = <1000>;
> +			phy-mode = "rgmii-id";
> +			snps,txpbl = <8>;
> +			snps,rxpbl = <8>;

I doubt that Ethernet is complete on the SoC - without MAC and all other
resources. IOW, it is very weird that this is enabled here. Please explain.

> +		};
> +
> +		dmac: dma-controller@39000000 {
> +			compatible = "snps,axi-dma-1.01a";
> +			reg = <0x0 0x39000000 0x0 0x400>;

<0x0 here but why in other places is <0x00?

Write consistent code.



Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 7/9] riscv: dts: ultrarisc: add Rongda M0 board device tree
From: Krzysztof Kozlowski @ 2026-05-21 20:59 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-7-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:18, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
> 
> Rongda M0 is an mATX motherboard based on the UltraRISC DP1000 SoC.
> 
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
>  arch/riscv/boot/dts/Makefile                       |   1 +
>  arch/riscv/boot/dts/ultrarisc/Makefile             |   2 +
>  .../dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi    |  85 ++++++++++++++++
>  arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts | 111 +++++++++++++++++++++
>  4 files changed, 199 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> index 69d8751fb17c..702882974251 100644
> --- a/arch/riscv/boot/dts/Makefile
> +++ b/arch/riscv/boot/dts/Makefile
> @@ -12,3 +12,4 @@ subdir-y += spacemit
>  subdir-y += starfive
>  subdir-y += tenstorrent
>  subdir-y += thead
> +subdir-y += ultrarisc
> diff --git a/arch/riscv/boot/dts/ultrarisc/Makefile b/arch/riscv/boot/dts/ultrarisc/Makefile
> new file mode 100644
> index 000000000000..d01a770d3cba
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_ULTRARISC) += dp1000-rongda-m0.dtb
> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi
> new file mode 100644
> index 000000000000..101b416b1079
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
> + */
> +
> +#include "dp1000.dtsi"
> +
> +&pmx0 {
> +	i2c0_pins: i2c0-pins {
> +		pins = "PA12", "PA13";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	i2c1_pins: i2c1-pins {
> +		pins = "PB6", "PB7";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	i2c2_pins: i2c2-pins {
> +		pins = "PC0", "PC1";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	i2c3_pins: i2c3-pins {
> +		pins = "PC2", "PC3";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	pciex4a_link_pins: pciex4a-link-pins {
> +		pins = "PC0";
> +		function = "func1";
> +		bias-pull-down;
> +		drive-strength = <33>;
> +	};
> +
> +	pciex4b_link_pins: pciex4b-link-pins {
> +		pins = "PC1";
> +		function = "func1";
> +		bias-pull-down;
> +		drive-strength = <33>;
> +	};
> +
> +	spi0_pins: spi0-pins {
> +		pins = "PD0", "PD1", "PD2", "PD3", "PD4", "PD5", "PD6", "PD7";
> +		function = "func1";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	spi1_pins: spi1-pins {
> +		pins = "PA0", "PA1", "PA2", "PA3";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	uart0_pins: uart0-pins {
> +		pins = "PA8", "PA9";
> +		function = "func1";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	uart1_pins: uart1-pins {
> +		pins = "PB4", "PB5";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +
> +	uart2_pins: uart2-pins {
> +		pins = "PC4", "PC5";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
> +};
> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts
> new file mode 100644
> index 000000000000..6f72d60ad55e
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
> + */
> +
> +#include "dp1000-rongda-m0-pinctrl.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "Rongda M0 Board";
> +	compatible = "rongda,m0", "ultrarisc,dp1000";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	gpio-poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio_b 0 GPIO_ACTIVE_HIGH>;
> +		active-delay-ms = <100>;
> +
> +		status = "disabled";

Why is this disabled? You just added final board, so it cannot have any
nodes disabled. Disabled at this point means you add dead code without
explanation.


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 9/9] riscv: defconfig: enable ARCH_ULTRARISC
From: Krzysztof Kozlowski @ 2026-05-21 20:57 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-9-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:18, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
> 
> Enable `ARCH_ULTRARISC` in the default RISC-V defconfig.
> 
> Link: https://lore.kernel.org/lkml/20260427-ultrarisc-pcie-v4-1-98935f6cdfb5@ultrarisc.com/

Drop link, not relevant here.

> 
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
>  arch/riscv/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index c2c37327b987..9fdc4d1831ed 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -32,6 +32,7 @@ CONFIG_SOC_STARFIVE=y
>  CONFIG_ARCH_SUNXI=y
>  CONFIG_ARCH_TENSTORRENT=y
>  CONFIG_ARCH_THEAD=y
> +CONFIG_ARCH_ULTRARISC=y

This patch should be sent with with the patch adding that config option.



Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 4/9] dt-bindings: pinctrl: Add UltraRISC DP1000 pinctrl bindings
From: Krzysztof Kozlowski @ 2026-05-21 20:56 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-4-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:18, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
> 
> Add bindings for the pin controllers on the UltraRISC DP1000 RISC-V SoC.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
>  .../bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml | 168 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  .../dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h |  65 ++++++++
>  3 files changed, 240 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml
> new file mode 100644
> index 000000000000..c7ed1f96382a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/ultrarisc,dp1000-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: UltraRISC DP1000 Pin Controller

Missing blank line.

Please organize your bindings to match expected style. See example-schema.

> +maintainers:
> +  - Jia Wang <wangjia@ultrarisc.com>
> +
> +description: |
> +  UltraRISC RISC-V SoC DP1000 pin controller.
> +
> +  The binding supports two child node styles under the same controller
> +  compatible:
> +
> +  - legacy DP1000-specific nodes using phandle-array properties

New bindings CANNOT introduce legacy. It's contradictory.

> +    `pinctrl-pins` and `pinconf-pins`
> +  - generic pinctrl nodes using `pins`, `function` and generic pin
> +    configuration properties
> +
> +properties:
> +  compatible:
> +    const: ultrarisc,dp1000-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#pinctrl-cells":

Use consistent quotes, either ' or "

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +patternProperties:
> +  '.*-pins$':
> +    type: object
> +    allOf:
> +      - $ref: /schemas/pinctrl/pincfg-node.yaml#
> +      - $ref: /schemas/pinctrl/pinmux-node.yaml#
> +    additionalProperties: false
> +    properties:
> +      pinctrl-pins:
> +        description: |
> +          The list of pins and their mux settings that properties in the node
> +          apply to. The format: `PORT  PIN  FUNCTION`.
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +      pinconf-pins:
> +        description: |
> +          The list of pins and their pad configuration that properties in the
> +          node apply to. The format: `PORT  PIN  CONF`.
> +          CONF is a DP1000-specific encoding of pull and drive strength as
> +          defined in dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h.
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 1
> +        maxItems: 32
> +      pins:
> +        description: List of pins affected by this state node.
> +        minItems: 1
> +        uniqueItems: true
> +        items:
> +          type: string
> +          pattern: '^(PA([0-9]|1[0-5])|P[BCD][0-7]|LPC([0-9]|1[0-2]))$'
> +
> +      function:
> +        description: |
> +          Mux function to select for the listed pins.
> +          gpio maps to the hardware default mode. The default mode is
> +          GPIO for PA/PB/PC/PD pins and LPC for LPC pins.
> +          func1 is not supported on LPC pins.
> +        enum:
> +          - gpio
> +          - func0
> +          - func1
> +
> +      bias-disable: true
> +      bias-high-impedance: true
> +      bias-pull-up: true
> +      bias-pull-down: true
> +
> +      drive-strength:
> +        description: Output drive strength in mA.
> +        enum: [20, 27, 33, 40]
> +
> +    oneOf:
> +      - allOf:
> +          - anyOf:
> +              - required: [pinctrl-pins]
> +              - required: [pinconf-pins]
> +          - not:
> +              required: [pins]
> +      - allOf:
> +          - required: [pins]
> +          - not:
> +              anyOf:
> +                - required: [pinctrl-pins]
> +                - required: [pinconf-pins]
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pinmux@11081000 {
> +        compatible = "ultrarisc,dp1000-pinctrl";
> +        reg = <0x0 0x11081000  0x0 0x1000>;
> +        #pinctrl-cells = <2>;
> +
> +        i2c0-pins {
> +          pins = "PA12", "PA13";
> +          function = "func0";
> +          bias-pull-up;
> +          drive-strength = <33>;
> +        };
> +
> +        uart0-pins {
> +          pins = "PA8", "PA9";
> +          function = "func1";
> +          bias-pull-up;
> +          drive-strength = <33>;
> +        };
> +      };
> +    };
> +
> +  - |
> +    /* Legacy example */

We do not take new code which is already legacy. Why would we want a legacy?

> +    #include <dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pinmux@11081000 {
> +        compatible = "ultrarisc,dp1000-pinctrl";
> +        reg = <0x0 0x11081000  0x0 0x1000>;
> +        #pinctrl-cells = <2>;
> +
> +        i2c0-pins {
> +          pinctrl-pins = <
> +            UR_DP1000_IOMUX_A  12  UR_DP1000_FUNC0
> +            UR_DP1000_IOMUX_A  13  UR_DP1000_FUNC0
> +          >;
> +
> +          pinconf-pins = <
> +            UR_DP1000_IOMUX_A  12  UR_DP1000_BIAS(UR_DP1000_PULL_UP,
> +                                                  UR_DP1000_DRIVE_DEF)
> +            UR_DP1000_IOMUX_A  13  UR_DP1000_BIAS(UR_DP1000_PULL_UP,
> +                                                  UR_DP1000_DRIVE_DEF)
> +          >;
> +        };
> +
> +        uart0-pins {
> +          pinctrl-pins = <
> +            UR_DP1000_IOMUX_A  8  UR_DP1000_FUNC1
> +            UR_DP1000_IOMUX_A  9  UR_DP1000_FUNC1
> +          >;
> +
> +          pinconf-pins = <
> +            UR_DP1000_IOMUX_A  8   UR_DP1000_BIAS(UR_DP1000_PULL_UP,
> +                                                  UR_DP1000_DRIVE_DEF)
> +            UR_DP1000_IOMUX_A  9   UR_DP1000_BIAS(UR_DP1000_PULL_UP,
> +                                                  UR_DP1000_DRIVE_DEF)
> +          >;
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5bf971ff48b2..baaaa46b1a56 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27358,6 +27358,13 @@ S:	Maintained
>  F:	drivers/usb/common/ulpi.c
>  F:	include/linux/ulpi/
>  
> +ULTRARISC DP1000 PINCTRL DRIVER
> +M:	Jia Wang <wangjia@ultrarisc.com>
> +L:	linux-gpio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/pinctrl/ultrarisc,dp1000-pinctrl.yaml
> +F:	include/dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.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/pinctrl/ultrarisc,dp1000-pinctrl.h b/include/dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h
> new file mode 100644
> index 000000000000..bef28115898d
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/ultrarisc,dp1000-pinctrl.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * UltraRISC DP1000 pinctrl header.
> + *
> + * Copyright (C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_ULTRARISC_DP1000_PINCTRL_H
> +#define _DT_BINDINGS_PINCTRL_ULTRARISC_DP1000_PINCTRL_H
> +
> +/**
> + * UltraRISC DP1000 IO pad configuration
> + * port: A, B, C, D, LPC
> + *     Pin in the port
> + * pin:
> + *     PA: 0 - 15
> + *     PB-PD: 0 - 7
> + *     LPC: 0 - 12
> + * func:
> + *     UR_DP1000_FUNC_DEF: default
> + *     UR_DP1000_FUNC0: func0
> + *     UR_DP1000_FUNC1: func1
> + */
> +#define UR_DP1000_IOMUX_A		0x0
> +#define UR_DP1000_IOMUX_B		0x1
> +#define UR_DP1000_IOMUX_C		0x2
> +#define UR_DP1000_IOMUX_D		0x3
> +#define UR_DP1000_IOMUX_LPC		0x4

Not bindings, drop.

> +
> +#define UR_DP1000_FUNC_DEF		0
> +#define UR_DP1000_FUNC0			1
> +#define UR_DP1000_FUNC1			0x10000

Not a binding.

> +
> +/**
> + * Configure pull up/down resistor of the IO pin
> + * UR_DP1000_PULL_DIS: disable pull-up and pull-down
> + * UR_DP1000_PULL_UP: enable pull-up
> + * UR_DP1000_PULL_DOWN: enable pull-down
> + */
> +#define UR_DP1000_PULL_DIS	0
> +#define UR_DP1000_PULL_UP	1
> +#define UR_DP1000_PULL_DOWN	2

I don't see usage in the driver, so not a binding.

> +/**
> + * Configure drive strength of the IO pin
> + * UR_DP1000_DRIVE_DEF: default value, reset value is 2
> + * UR_DP1000_DRIVE_0: 20mA
> + * UR_DP1000_DRIVE_1: 27mA
> + * UR_DP1000_DRIVE_2: 33mA
> + * UR_DP1000_DRIVE_3: 40mA
> + */
> +#define UR_DP1000_DRIVE_DEF	2
> +#define UR_DP1000_DRIVE_0	0
> +#define UR_DP1000_DRIVE_1	1
> +#define UR_DP1000_DRIVE_2	2
> +#define UR_DP1000_DRIVE_3	3

Not a binding, use actual mA values in the DTS.

> +
> +/**
> + * Combine the pull-up/down resistor and drive strength
> + * pull: UR_DP1000_PULL_DIS, UR_DP1000_PULL_UP, UR_DP1000_PULL_DOWN
> + * drive: UR_DP1000_DRIVE_DEF, UR_DP1000_DRIVE_0, UR_DP1000_DRIVE_1,
> + *        UR_DP1000_DRIVE_2, UR_DP1000_DRIVE_3
> + */
> +#define UR_DP1000_BIAS(pull, drive)		(((pull) << 2) | (drive))
> +
> +#endif /* _DT_BINDINGS_PINCTRL_ULTRARISC_DP1000_PINCTRL_H */
> 

Also not used in the driver. Drop entire header or move out of bindings.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 1/9] dt-bindings: vendor-prefixes: add Rongda
From: Krzysztof Kozlowski @ 2026-05-21 20:51 UTC (permalink / raw)
  To: wangjia, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Linus Walleij, Bartosz Golaszewski, Samuel Holland
  Cc: Paul Walmsley, Palmer Dabbelt, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260515-ultrarisc-pinctrl-v1-1-bf559589ea8a@ultrarisc.com>

On 15/05/2026 03:17, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@ultrarisc.com>
> 
> Add Shenzhen Rongda Computer Co., Ltd. to the vendor prefixes.
> 
> Link: http://www.shenrongda.com/

So your prefix is shenrongda.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v4 4/5] dt-bindings: display: Add Synaptics R63455 panel support
From: Rob Herring (Arm) @ 2026-05-21 20:46 UTC (permalink / raw)
  To: Jun Nie
  Cc: Simona Vetter, Thomas Zimmermann, Krzysztof Kozlowski,
	Marijn Suijten, Neil Armstrong, linux-kernel, devicetree,
	linux-arm-msm, Dmitry Baryshkov, Rob Clark, Maxime Ripard,
	dri-devel, Maarten Lankhorst, Jessica Zhang, Conor Dooley,
	David Airlie, freedreno, Dmitry Baryshkov, Abhinav Kumar,
	Sean Paul
In-Reply-To: <20260521-sm8650-7-1-bonded-dsi-v4-4-a4dd5e0850f1@linaro.org>


On Thu, 21 May 2026 22:46:06 +0800, Jun Nie wrote:
> Add support for the dual-panel system found in the virtual reality device.
> This system consists of two physical 2160x2160 panels, each connected via
> a MIPI DSI interface. The backlight is managed through DSI link.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../bindings/display/panel/synaptics,r63455.yaml   | 125 +++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml:26:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml:55:14: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml: ignoring, error parsing file
./Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml:55:14: mapping values are not allowed here
make[2]: *** Deleting file 'Documentation/devicetree/bindings/display/panel/synaptics,r63455.example.dts'
Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml:55:14: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/panel/synaptics,r63455.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1659: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260521-sm8650-7-1-bonded-dsi-v4-4-a4dd5e0850f1@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [PATCH net-next 3/5] net: phy: Add support for the ADIN1140 PHY
From: Andrew Lunn @ 2026-05-21 20:44 UTC (permalink / raw)
  To: Regus, Ciprian
  Cc: Parthiban Veerasooran, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
	Shuah Khan, Heiner Kallweit, Russell King, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <1d7f5247b07f44f2a30cfed8f7d3cd6d@analog.com>

On Thu, May 21, 2026 at 08:24:41PM +0000, Regus, Ciprian wrote:
> 
> > > +static int adin1140_phy_read_mmd(struct phy_device *phydev, int
> > devnum,
> > > +				 u16 regnum)
> > > +{
> > > +	struct mii_bus *bus = phydev->mdio.bus;
> > > +	int addr = phydev->mdio.addr;
> > > +
> > > +	return __mdiobus_c45_read(bus, addr, devnum, regnum);
> > > +}
> > > +
> > > +static int adin1140_phy_write_mmd(struct phy_device *phydev, int
> > devnum,
> > > +				  u16 regnum, u16 val)
> > > +{
> > > +	struct mii_bus *bus = phydev->mdio.bus;
> > > +	int addr = phydev->mdio.addr;
> > > +
> > > +	return __mdiobus_c45_write(bus, addr, devnum, regnum, val);
> > > +}
> > 
> > Why do these exist?
> >
> 
> The PHY is always probed over C22. Unless read_mmd()/write_mmd() are defined,
> phylib will default to indirect accesses to MMD devices. The 0xD and 0xE PHY registers
> are not implemented, so those transfers won't have any effect.

In oa_tc6_mdiobus_register() there is the comment:

       /* OPEN Alliance 10BASE-T1x compliance MAC-PHYs will have both C22 and
         * C45 registers space. If the PHY is discovered via C22 bus protocol it
         * assumes it uses C22 protocol and always uses C22 registers indirect
         * access to access C45 registers. This is because, we don't have a
         * clean separation between C22/C45 register space and C22/C45 MDIO bus
         * protocols. Resulting, PHY C45 registers direct access can't be used
         * which can save multiple SPI bus access. To support this feature, PHY
         * drivers can set .read_mmd/.write_mmd in the PHY driver to call
         * .read_c45/.write_c45. Ex: drivers/net/phy/microchip_t1s.c
         */

which is what you are doing.

If this was a DT driven device, you would add:

compatible = "ethernet-phy-ieee802.3-c45";

which would result in the device being probed via C45, and is_c45
would be set true.

Maybe we need to improve the situation here. We know C45 is
implemented, it is part of the standard. So maybe we need to set
is_c45?

In oa_tc6_phy_init() we already have:

tc6->phydev->is_internal = true;

what happens if we add

tc6->phydev->is_c45 = true; ?

Maybe this was discussed already once, when oa_tc6.c? I don't
remember.

	Andrew

^ permalink raw reply

* Re: [PATCH V8 05/10] iio: imu: inv_icm42607: Add PM support for icm42607
From: Chris Morgan @ 2026-05-21 20:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner,
	jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
	conor+dt, krzk+dt, robh, andriy.shevchenko
In-Reply-To: <20260520181353.0a0371cb@jic23-huawei>

On Wed, May 20, 2026 at 06:13:53PM +0100, Jonathan Cameron wrote:
> On Mon, 18 May 2026 15:05:20 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
> 
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add power management support for the ICM42607 device driver.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> 
> Hi Chris,  runtime PM is my current place to look closest
> for bugs because we've had a lot of them recently.
> 
> Anyhow, took another look and here I think we can optimize
> things a little more.
> 
> > ---
> >  drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
> >  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 147 ++++++++++++++++++
> >  .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
> >  .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
> >  4 files changed, 167 insertions(+)
> > 
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > index 74d8d3d7c890..b05828415053 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/bitops.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/mutex.h>
> > +#include <linux/pm.h>
> >  #include <linux/regmap.h>
> >  #include <linux/regulator/consumer.h>
> >  
> > @@ -96,24 +97,34 @@ struct inv_icm42607_hw {
> >  	const struct inv_icm42607_conf *conf;
> >  };
> >  
> > +struct inv_icm42607_suspended {
> > +	enum inv_icm42607_sensor_mode gyro;
> > +	enum inv_icm42607_sensor_mode accel;
> > +	bool temp;
> > +};
> > +
> >  /**
> >   *  struct inv_icm42607_state - driver state variables
> >   *  @lock:		lock for serializing multiple registers access.
> >   *  @hw:		Hardware specific data.
> >   *  @map:		regmap pointer.
> >   *  @vddio_supply:	I/O voltage regulator for the chip.
> > + *  @vddio_en:		I/O voltage status for runtime PM.
> >   *  @irq:		chip irq, required to enable/disable and set wakeup
> >   *  @orientation:	sensor chip orientation relative to main hardware.
> >   *  @conf:		chip sensors configurations.
> > + *  @suspended:		suspended sensors configuration.
> >   */
> >  struct inv_icm42607_state {
> >  	struct mutex lock;
> >  	const struct inv_icm42607_hw *hw;
> >  	struct regmap *map;
> >  	struct regulator *vddio_supply;
> > +	bool vddio_en;
> >  	int irq;
> >  	struct iio_mount_matrix orientation;
> >  	struct inv_icm42607_conf conf;
> > +	struct inv_icm42607_suspended suspended;
> 
> Is this used?  If not bring it in only when needed.
> 
> >  };
> 
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > index e9c81b52f9ef..bc0cefa2fb77 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/irq.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/property.h>
> >  #include <linux/regmap.h>
> >  #include <linux/regulator/consumer.h>
> > @@ -72,6 +73,62 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
> >  };
> >  EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
> >  
> > +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> > +				      enum inv_icm42607_sensor_mode gyro,
> > +				      enum inv_icm42607_sensor_mode accel,
> > +				      bool temp, unsigned int *sleep_ms)
> > +{
> > +	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> > +	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> > +	bool oldtemp = st->conf.temp_en;
> > +	unsigned int sleepval;
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> > +		return 0;
> > +
> > +	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
> > +	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
> > +	if (!temp)
> > +		val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
> > +	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	st->conf.gyro.mode = gyro;
> > +	st->conf.accel.mode = accel;
> > +	st->conf.temp_en = temp;
> > +
> > +	sleepval = 0;
> > +	if (temp && !oldtemp) {
> > +		if (sleepval < INV_ICM42607_TEMP_STARTUP_TIME_MS)
> > +			sleepval = INV_ICM42607_TEMP_STARTUP_TIME_MS;
> 		sleepval = max(sleepval,)
> or just assign it here if not later patches add stuff in between
> the assignment to 0 and here.

I'm going to assign it to 0 here (unless you think I should define it
at the beginning as 0) and then tweak as needed. I think this code
here can be further optimized, especially if we make the assumption
that START and STOP time for each sensor is comparable (the datasheet
doesn't say, so I'm going to go with yes since that greatly simplifies
things).

> > +	}
> > +	if (accel != oldaccel && oldaccel == INV_ICM42607_SENSOR_MODE_OFF) {
> > +		usleep_range(200, 300);
> 
> fsleep() + add a comment on why we are sleeping in here.
> 

Just going to delete these usleep_ranges since 1) I can't find them
documented in the datasheet and 2) we sleep later on anyway if this
condition is met.

> > +		if (sleepval < INV_ICM42607_ACCEL_STARTUP_TIME_MS)
> > +			sleepval = INV_ICM42607_ACCEL_STARTUP_TIME_MS;
> 		sleepval = max(sleepval, INV...);
> 
> > +	}
> > +	if (gyro != oldgyro) {
> > +		if (oldgyro == INV_ICM42607_SENSOR_MODE_OFF) {
> > +			usleep_range(200, 300);
> 
> fsleep()
> 
> > +			if (sleepval < INV_ICM42607_GYRO_STARTUP_TIME_MS)
> > +				sleepval = INV_ICM42607_GYRO_STARTUP_TIME_MS;
> I'd use max for these as well.
> 
> > +		} else if (gyro == INV_ICM42607_SENSOR_MODE_OFF) {
> > +			if (sleepval < INV_ICM42607_GYRO_STARTUP_TIME_MS)
> > +				sleepval = INV_ICM42607_GYRO_STARTUP_TIME_MS;
> > +		}
> > +	}
> > +
> > +	if (sleep_ms)
> > +		*sleep_ms = sleepval;
> > +	else if (sleepval)
> > +		msleep(sleepval);
> > +
> > +	return 0;
> > +}
> 
> >  
> > +static int inv_icm42607_suspend(struct device *dev)
> > +{
> > +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> Could you use pm_runtime_force_suspend()?
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	ret = inv_icm42607_set_pwr_mgmt0(st, INV_ICM42607_SENSOR_MODE_OFF,
> > +					 INV_ICM42607_SENSOR_MODE_OFF,
> > +					 false, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	inv_icm42607_disable_vddio_reg(st);
> > +
> > +	return 0;
> > +}
> > +
> > +static int inv_icm42607_resume(struct device *dev)
> > +{
> > +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> Given the bunch of stuff we've run into recently around these
> I'm getting more paranoid.
> Similar to above, could you use pm_runtime_force_resume()
> You would need to gate stuff added later to not occur
> though if it wasn't runtime suspended.

This I'm having trouble understanding. If I use
pm_force_runtime_resume() I'm assuming that either I got an error (in
which case I'd return the error) or the device is runtime resumed
after the call completes. If that's the case, wouldn't my suspend and
resume steps just be pm_force_runtime_suspend/resume, and enabling the
regulator (first for resume) or disabling the regulator (last for
suspend) as needed?

> 
> 
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	ret = inv_icm42607_enable_vddio_reg(st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Nothing else to restore at this time. */
> > +
> > +	return 0;
> > +}
> > +
> > +static int inv_icm42607_runtime_suspend(struct device *dev)
> > +{
> > +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	ret = inv_icm42607_set_pwr_mgmt0(st, INV_ICM42607_SENSOR_MODE_OFF,
> > +					 INV_ICM42607_SENSOR_MODE_OFF, false,
> > +					 NULL);
> Different alignment to above - aim for consistent choices on this.
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	inv_icm42607_disable_vddio_reg(st);
> > +
> > +	return 0;
> > +}
> > +
> > +static int inv_icm42607_runtime_resume(struct device *dev)
> > +{
> > +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	return inv_icm42607_enable_vddio_reg(st);
> > +}
> > +
> > +EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {
> > +	SYSTEM_SLEEP_PM_OPS(inv_icm42607_suspend, inv_icm42607_resume)
> > +	RUNTIME_PM_OPS(inv_icm42607_runtime_suspend,
> > +		       inv_icm42607_runtime_resume, NULL)
> > +};
> > +

Thank you,
Chris

^ permalink raw reply

* Re: [PATCH v2 net-next 1/3] dt-bindings: vendor-prefixes: add SiTime Corporation
From: Ali Rouhi @ 2026-05-21 20:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <20260521-happy-celadon-hamster-94802c@quoll>

> Mismatch in From/DCO.

Acknowledged — Gmail SMTP relay issue, will fix for v3.

> It looks like you received a tag and forgot to add it.

Thank you for the reminder.  Will carry Conor's tag in v3:

  Acked-by: Conor Dooley <conor.dooley@microchip.com>

Best regards,
Ali

On Thu, May 21, 2026 at 3:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, May 20, 2026 at 12:19:41PM -0700, Ali Rouhi wrote:
> > Add vendor prefix for SiTime Corporation, manufacturer of
> > programmable clock generators and MEMS oscillators.
> >
> > Signed-off-by: Ali Rouhi <arouhi@sitime.com>
>
> Mismatch in From/DCO.
>
> Best regards,
> Krzysztof
>

^ permalink raw reply

* Re: [PATCH v2 net-next 2/3] dt-bindings: dpll: add SiTime SiT9531x clock generator
From: Ali Rouhi @ 2026-05-21 20:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <48cc27ed-48e8-41db-8351-166774466a69@kernel.org>

> Mismatched DCO. Use consistent identity or fix your commits.

Apologies — Gmail's SMTP relay rewrites the From header to my
personal address (rouhi.ali@gmail.com).  I'm working with IT to
get corporate SMTP credentials so the From matches the Signed-off-by
(arouhi@sitime.com) in v3.

> Same as last time. Why are you describing drivers?

Will drop the clocks description entirely — maxItems and
clock-names are sufficient.

> Drop node. Wasn't here before, so why did you add it?
> Drop.

Will drop both xo and xo2 fixed-clock nodes from the examples.

Thanks for the review.

Best regards,
Ali

On Thu, May 21, 2026 at 12:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 20/05/2026 21:19, Ali Rouhi wrote:
> > Add device tree binding documentation for the SiTime SiT95316
> > and SiT95317 DPLL clock generators.
> >
> > Co-developed-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>
> > Signed-off-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>
> > Signed-off-by: Ali Rouhi <arouhi@sitime.com>
>
> Mismatched DCO. Use consistent identity or fix your commits.
>
> > ---
> >  .../bindings/dpll/sitime,sit9531x.yaml        | 145 ++++++++++++++++++
> >  1 file changed, 145 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml b/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> > new file mode 100644
> > index 000000000000..ac88f2f0b2ae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> > @@ -0,0 +1,145 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dpll/sitime,sit9531x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiTime SiT9531x DPLL Clock Generator
> > +
> > +maintainers:
> > +  - Ali Rouhi <arouhi@sitime.com>
> > +
> > +description: |
> > +  SiTime SiT95316 and SiT95317 are I2C-controlled programmable clock
> > +  generators with integrated DPLL for synchronization applications.  Both
> > +  variants contain four PLLs with automatic/manual reference selection,
> > +  DCO frequency adjustment, and phase offset measurement via an on-chip
> > +  TDC (Time-to-Digital Converter).
> > +
> > +  SiT95317 provides 4 inputs and 8 outputs; SiT95316 provides
> > +  4 inputs and 12 outputs.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - sitime,sit95316
> > +      - sitime,sit95317
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description:
> > +      External crystal/oscillator feeding the chip's XIN/XO_CLK input.
> > +      The chip's PLL Fvco is computed relative to this reference, so the
> > +      driver requires a non-zero rate at probe time.
>
> Same as last time. Why are you describing drivers?
>
> > +
> > +  clock-names:
> > +    items:
> > +      - const: xtal
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO connected to the chip's active-low reset pin (RESETB).
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description:
> > +      Interrupt from the chip's active-low INTRB output.  Asserted when
> > +      the device detects a status change such as lock acquisition or loss.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +
> > +allOf:
> > +  - $ref: /schemas/dpll/dpll-device.yaml#
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    xo: xo {
> > +        compatible = "fixed-clock";
> > +        #clock-cells = <0>;
> > +        clock-frequency = <48000000>;
> > +    };
>
> Drop node. Wasn't here before, so why did you add it?
>
>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        dpll@68 {
> > +            compatible = "sitime,sit95317";
> > +            reg = <0x68>;
> > +            clocks = <&xo>;
> > +            clock-names = "xtal";
> > +        };
> > +    };
> > +
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    xo2: xo2 {
> > +        compatible = "fixed-clock";
> > +        #clock-cells = <0>;
> > +        clock-frequency = <48000000>;
> > +    };
>
> Drop.
>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
>
>
>
> Best regards,
> Krzysztof

^ permalink raw reply

* Re: [PATCH v1 1/3] dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
From: Krzysztof Kozlowski @ 2026-05-21 20:34 UTC (permalink / raw)
  To: lianfeng.ouyang, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Mika Westerberg, Andy Shevchenko, Jan Dabros
  Cc: linux-i2c, devicetree, linux-kernel
In-Reply-To: <20260521034340.27837-2-lianfeng.ouyang@starfivetech.com>

On 21/05/2026 05:43, lianfeng.ouyang wrote:
> +
> +  starfive,mctp-i2c-ms:
> +    description: |
> +      The property should contain reference to the master node associated with the slave.

It's redundant to say that property is "the property ...". Just SAY
describe that.

Also, wrap according to Linux coding style.

> +      This value is only used in slave mode, especially for MCTP application.
> +
> +  dwc-i2c-tx-fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property describes the tx fifo depth.
> +    default: 8
> +
> +  dwc-i2c-rx-fifo-depth:
Wrong name. Generic properties DO NOT use "dwc" name. Please use
existing standard properties from dtschema, other common schemas or just
look around.

Do not invent your own stuff and unfortunately half of this binding is
such invention - done completely different than everything else. That's
a sign of downstream code and we really do not like such code.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Please read carefully writing-bindings document and submitting-patches
in DT dir.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v4 1/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Conor Dooley @ 2026-05-21 20:26 UTC (permalink / raw)
  To: Arnab Layek
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, robh,
	krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
	andersson, mathieu.poirier, linux-remoteproc,
	Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260520112629.3420612-2-arnab.layek@mediatek.com>

[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]

On Wed, May 20, 2026 at 07:26:29PM +0800, Arnab Layek wrote:
> The MT8188 SCP can use either one or two reserved memory regions:
> - Required: Main SCP SRAM memory region (mandatory for SCP operation)
> - Optional: SCP L1TCM memory region (Level 1 Tightly Coupled Memory,
>   used for performance optimization when available, but not required
>   for basic SCP functionality)
> 
> Other MediaTek SoCs (MT8183, MT8186, MT8192, MT8195) use only a single
> memory region and must remain restricted to one region for backward
> compatibility.
> 
> Update the base schema to allow 1-2 memory regions (minItems: 1,
> maxItems: 2), following the pattern used by other MediaTek dt-bindings
> like mediatek,vcodec-encoder.yaml where the base property defines a
> permissive range accommodating all device variants.
> 
> Add two conditionals:
> 1. Explicitly restrict non-MT8188 devices to maxItems: 1
> 2. Document MT8188's two regions with descriptions (minItems: 1 makes
>    the L1TCM region optional, allowing boards to specify 1-2 regions
>    based on hardware configuration)
> 
> This approach maintains backward compatibility while enabling MT8188 to
> specify 1-2 memory regions depending on board design and performance
> requirements.
> 
> Signed-off-by: Arnab Layek <arnab.layek@mediatek.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

Cheers,
Conor.

> ---
>  .../bindings/remoteproc/mtk,scp.yaml          | 45 ++++++++++++++++++-
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index bdbb12118da4..fca9b0675eae 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -55,7 +55,8 @@ properties:
>        initializing SCP.
>  
>    memory-region:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    cros-ec-rpmsg:
>      $ref: /schemas/embedded-controller/google,cros-ec.yaml
> @@ -123,7 +124,8 @@ patternProperties:
>            initializing sub cores of multi-core SCP.
>  
>        memory-region:
> -        maxItems: 1
> +        minItems: 1
> +        maxItems: 2
>  
>        cros-ec-rpmsg:
>          $ref: /schemas/embedded-controller/google,cros-ec.yaml
> @@ -205,6 +207,45 @@ allOf:
>            items:
>              - const: cfg
>              - const: l1tcm
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt8183-scp
> +            - mediatek,mt8186-scp
> +            - mediatek,mt8192-scp
> +            - mediatek,mt8195-scp
> +            - mediatek,mt8195-scp-dual
> +    then:
> +      properties:
> +        memory-region:
> +          maxItems: 1
> +      patternProperties:
> +        "^scp@[a-f0-9]+$":
> +          properties:
> +            memory-region:
> +              maxItems: 1
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt8188-scp
> +            - mediatek,mt8188-scp-dual
> +    then:
> +      properties:
> +        memory-region:
> +          minItems: 1
> +          items:
> +            - description: Main SCP SRAM memory region
> +            - description: Optional SCP L1TCM memory region
> +      patternProperties:
> +        "^scp@[a-f0-9]+$":
> +          properties:
> +            memory-region:
> +              minItems: 1
> +              items:
> +                - description: Main SCP SRAM memory region
> +                - description: Optional SCP L1TCM memory region
>  
>  additionalProperties: false
>  
> -- 
> 2.45.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/1] dt-bindings: trivial-devices: add fsl,mc1323
From: Frank Li @ 2026-05-21 20:25 UTC (permalink / raw)
  To: Frank.Li
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Guenter Roeck,
	Wensheng Wang, Cosmo Chou, Brian Chiang, Eddie James,
	Nuno Sá, Antoni Pokusinski, Dixit Parmar,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx
In-Reply-To: <20260521195233.1532852-1-Frank.Li@oss.nxp.com>

On Thu, May 21, 2026 at 03:52:31PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
>
> Add freescale 2.4 GHz IEEE® 802.15.4/ZigBee mc1323 to fix the below
> CHECK_DTBS warnings.
>   arch/arm/boot/dts/nxp/imx/imx53-smd.dtb: /soc/bus@60000000/tve@63ff0000: failed to match any schema with compatible: ['fsl,imx53-tve']

Sorry, paste wrong error log here, should be

arch/arm/boot/dts/nxp/imx/imx53-smd.dtb: /soc/bus@50000000/spba-bus@50000000/spi@50010000/mc1323@0: failed to match any schema with compatible: ['fsl,mc1323']

Frank

>
> Since the i.MX53 platform is more than 20 years old, it is difficult to
> find detailed information about how the MC1323 was used on the i.MX53 SMD
> board, as the functionality depended on firmware.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v2
> - add descript about reason in commit message
> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 46a4dca50c485..6ff96e10d0785 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -127,6 +127,8 @@ properties:
>            - domintech,dmard09
>              # DMARD10: 3-axis Accelerometer
>            - domintech,dmard10
> +            # Freescale 2.4 GHz IEEE® 802.15.4/ZigBee
> +          - fsl,mc1323
>              # MMA7660FC: 3-Axis Orientation/Motion Detection Sensor
>            - fsl,mma7660
>              # MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
> --
> 2.43.0
>

^ permalink raw reply

* RE: [PATCH net-next 3/5] net: phy: Add support for the ADIN1140 PHY
From: Regus, Ciprian @ 2026-05-21 20:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Parthiban Veerasooran, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
	Shuah Khan, Heiner Kallweit, Russell King, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <2cfa6680-503a-4c4a-91bc-5f9a4331967d@lunn.ch>


> > +static int adin1140_phy_read_mmd(struct phy_device *phydev, int
> devnum,
> > +				 u16 regnum)
> > +{
> > +	struct mii_bus *bus = phydev->mdio.bus;
> > +	int addr = phydev->mdio.addr;
> > +
> > +	return __mdiobus_c45_read(bus, addr, devnum, regnum);
> > +}
> > +
> > +static int adin1140_phy_write_mmd(struct phy_device *phydev, int
> devnum,
> > +				  u16 regnum, u16 val)
> > +{
> > +	struct mii_bus *bus = phydev->mdio.bus;
> > +	int addr = phydev->mdio.addr;
> > +
> > +	return __mdiobus_c45_write(bus, addr, devnum, regnum, val);
> > +}
> 
> Why do these exist?
>

The PHY is always probed over C22. Unless read_mmd()/write_mmd() are defined,
phylib will default to indirect accesses to MMD devices. The 0xD and 0xE PHY registers
are not implemented, so those transfers won't have any effect.

^ permalink raw reply

* Re: [PATCH v4 4/5] dt-bindings: display: Add Synaptics R63455 panel support
From: Dmitry Baryshkov @ 2026-05-21 20:24 UTC (permalink / raw)
  To: Jun Nie
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
In-Reply-To: <20260521-sm8650-7-1-bonded-dsi-v4-4-a4dd5e0850f1@linaro.org>

On Thu, May 21, 2026 at 10:46:06PM +0800, Jun Nie wrote:
> Add support for the dual-panel system found in the virtual reality device.
> This system consists of two physical 2160x2160 panels, each connected via
> a MIPI DSI interface. The backlight is managed through DSI link.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../bindings/display/panel/synaptics,r63455.yaml   | 125 +++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml b/Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml
> new file mode 100644
> index 0000000000000..a94b355ed9557
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/synaptics,r63455.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/synaptics,r63455.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synaptics R63455 based dual 2160x2160 MIPI-DSI Panel
> +
> +maintainers:
> +  - Jun Nie <jun.nie@linaro.org>
> +
> +description:
> +  Synaptics R63455 is a Virtual Reality Display Driver and VR Bridge, used in
> +  pair in Headset devices. The Virtual Reality Display complex is composed of
> +  two strictly identical display panels, each driven by its own DSI interface
> +  but forms a single virtual display for the human eye perception and thus
> +  requires a strict synchronization of the two display panel content update.
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +        - sharp,ls026b3sa06
> +        - boe,vs026c4m-n52-6000
> +      - const: synaptics,r63455
> +
> +  reset-gpios:
> +    maxItems: 2
> +    description: 2 reset pins for 2 physical panels
> +
> +  left-pos-supply:
> +    description: Positive 5.7V supply for left panel

So, is the R63455 driving both panels or are there two panels, each
having R63455 controller? What if somebody gets a single Sharp panel and
wants to use it in their device? How will it match these bindings?

> +
> +  right-pos-supply:
> +    description: Positive 5.7V supply for right panel
> +
> +  left-neg-supply:
> +    description: Negative 5.7V supply for left panel
> +
> +  right-neg-supply:
> +    description: Negative 5.7V supply for right panel
> +
> +  left-backlight-supply:
> +    description: Backlight 21V supply for left panel
> +
> +  right-backlight-supply:
> +    description: Backlight 21V supply for right panel
> +
> +  vdda-supply:
> +    description: core 1.8V supply for panels
> +
> +  ports: $ref: /schemas/graph.yaml#/properties/ports
> +
> +required:
> +  - compatible
> +  - reset-gpios
> +  - left-pos-supply
> +  - left-neg-supply
> +  - right-pos-supply
> +  - right-neg-supply
> +  - left-backlight-supply
> +  - right-backlight-supply
> +  - vdda-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    &mdss_dsi0 {

Please drop the MDSS specifics, there should be one (or two) DSI busses,
driving your panels. The rests are details which are not necessary for
the example.

> +        vdda-supply = <&vreg_l3i_1p2>;
> +        status = "okay";
> +
> +        qcom,dual-dsi-mode;
> +        qcom,master-dsi;
> +
> +        panel: panel@0 {
> +            compatible = "sharp,ls026b3sa06", "synaptics,r63455";
> +            reg = <0>;
> +
> +            reset-gpios = <&pm8550_gpios 3 GPIO_ACTIVE_HIGH>,
> +                          <&pm8550_gpios 11 GPIO_ACTIVE_HIGH>;
> +
> +            left-pos-supply = <&vpos_left>;
> +            left-neg-supply = <&vneg_left>;
> +            right-pos-supply = <&vpos_right>;
> +            right-neg-supply = <&vneg_right>;
> +            left-backlight-supply = <&backlight_left>;
> +            right-backlight-supply = <&backlight_right>;
> +
> +            vdda-supply = <&vreg_l12b_1p8>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                port@0 {
> +                    reg = <0>;
> +                    panel0_in: endpoint {
> +                        remote-endpoint = <&mdss_dsi0_out>;

What is mdss_dsi0_out?

> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    panel1_in: endpoint {
> +                        remote-endpoint = <&mdss_dsi1_out>;
> +                    };
> +                };
> +            };
> +    };
> +
> +    &mdss_dsi0_out {
> +            remote-endpoint = <&panel0_in>;
> +            data-lanes = <0 1 2>;
> +    };
> +
> +    &mdss_dsi1_out {
> +            remote-endpoint = <&panel1_in>;
> +            data-lanes = <0 1 2>;
> +    };
> +...
> 
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v1 1/3] dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
From: Conor Dooley @ 2026-05-21 20:22 UTC (permalink / raw)
  To: lianfeng.ouyang
  Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Mika Westerberg, Andy Shevchenko, Jan Dabros, linux-i2c,
	devicetree, linux-kernel
In-Reply-To: <20260521034340.27837-2-lianfeng.ouyang@starfivetech.com>

[-- Attachment #1: Type: text/plain, Size: 5813 bytes --]

On Thu, May 21, 2026 at 11:43:38AM +0800, lianfeng.ouyang wrote:
> From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> Add device tree bindings for the Synopsys DesignWare Core (DWC) I2C
> controller and its StarFive JHB100 implementation
> 
> The binding introduces a new compatible string: "snps,dwc-i2c", intended
> for the generic IP. It also defines two platform-specific compatibles
> for the StarFive JHB100 implementation:
> - "starfive,jhb100-dwc-i2c-master"
> - "starfive,jhb100-dwc-i2c-slave"

Do you have two different i2c controllers on the device, one which
implements only slave mode and one that only implements master?
Or can the same controller be both master or slave depending on how the
user wants to use it?

> 
> The controller supports standard I2C and SMBus protocols, programmable
> FIFO depths, and optional SMBus Alert routing. The binding documents
> the necessary clocks, resets, and timing properties.
> 
> Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> ---
>  .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 ++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> new file mode 100644
> index 000000000000..7227f24f7cbe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> @@ -0,0 +1,120 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/snps,dwc-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DWC I2C Controller
> +
> +maintainers:
> +  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Generic Synopsys DWC I2C controller
> +        const: snps,dwc-i2c

I think you should delete this, we don't want to permit avoiding using
soc-specific compatibles.

Why can't this go into the existing designware i2c binding?

> +      - description: StarFive JHB100 I2C master controller
> +        items:
> +          - const: starfive,jhb100-dwc-i2c-master
> +          - const: snps,dwc-i2c

This fallback needs to be a lot more specific about what the revision
is, so that people can figure out which fallback applies to them.

> +      - description: StarFive JHB100 I2C slave controller
> +        items:
> +          - const: starfive,jhb100-dwc-i2c-slave
> +          - const: snps,dwc-i2c
> +
> +  reg:
> +    description: DWC I2C controller memory mapped registers
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    items:
> +      - description: I2C controller reference clock source
> +      - description: APB interface clock source
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: ref
> +      - const: pclk

Please add a conditional section that sets the correct number of clocks
for your jhb100.

> +
> +  resets:
> +    maxItems: 1
> +
> +  clock-frequency:
> +    description: Desired I2C bus clock frequency in Hz
> +    enum: [100000, 400000, 1000000, 3400000]
> +    default: 400000
> +
> +  i2c-sda-hold-time-ns:
> +    description: |
> +      The property should contain the SDA hold time in nanoseconds.
> +      This value is used to compute value written into DW_IC_SDA_HOLD register.

Missing a default here.

> +
> +  i2c-scl-falling-time-ns:
> +    description: |
> +      The property should contain the SCL falling time in nanoseconds.
> +      This value is used to compute the tLOW period.
> +    default: 300
> +
> +  i2c-sda-falling-time-ns:
> +    description: |
> +      The property should contain the SDA falling time in nanoseconds.
> +      This value is used to compute the tHIGH period.
> +    default: 300
> +
> +  starfive,mctp-i2c-ms:
> +    description: |
> +      The property should contain reference to the master node associated with the slave.
> +      This value is only used in slave mode, especially for MCTP application.

This property is missing a type, but I also don't understand what it is
for. You shouldn't need to know what the i2c master is.
I assume it is meant to be a phandle? Can you share an example dts
that contains this property in use?

> +
> +  dwc-i2c-tx-fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property describes the tx fifo depth.
> +    default: 8
> +
> +  dwc-i2c-rx-fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property describes the rx fifo depth.
> +    default: 8
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

clocks and clock-names too.

Bunch of valid complaints from sashiko on this, so
pw-bot: changes-requested

Thanks,
Conor.
> +
> +examples:
> +  - |
> +    i2c@f0000 {
> +      compatible = "snps,dwc-i2c";
> +      reg = <0xf0000 0x1000>;
> +      interrupts = <11>;
> +      clock-frequency = <400000>;
> +    };
> +  - |
> +    i2c@2000 {
> +      compatible = "snps,dwc-i2c";
> +      reg = <0x2000 0x100>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      clock-frequency = <400000>;
> +      clocks = <&i2cclk>;
> +      interrupts = <0>;
> +
> +      eeprom@64 {
> +        compatible = "atmel,24c02";
> +        reg = <0x64>;
> +      };
> +    };
> +...
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/6] ARM: dts: imx: remove redundant bus-width for video-mux
From: sashiko-bot @ 2026-05-21 20:19 UTC (permalink / raw)
  To: Frank.Li; +Cc: imx, conor+dt, Frank.Li, devicetree, robh
In-Reply-To: <20260521-imx25_dts_simple_warning_2-v2-2-c6557df516a9@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch removes the `bus-width` property from IPU CSI endpoints (e.g., `&ipu1_csi1_from_ipu1_csi1_mux`) without documenting this change in the commit message, which only mentions `video-mux` endpoints. Additionally, this removal leaves behind completely empty node overrides (e.g., `&ipu1_csi1_from_ipu1_csi1_mux {}`) which should be removed entirely to prevent unnecessary clutter.
--

commit 9fb698028cc05d2c61d201637e9e7246e95e81d5
Author: Frank Li <Frank.Li@nxp.com>

ARM: dts: imx: remove redundant bus-width for video-mux

This commit removes redundant bus-width properties from various i.MX device
tree files to resolve dtbs_check warnings. The bus-width is already
specified at the remote endpoint.

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6dl-gw52xx.dts b/arch/arm/boot/dts/nxp/imx/imx6dl-gw52xx.dts
> index 9ea23dd54f3ce..62b05fe70cd97 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx6dl-gw52xx.dts
> +++ b/arch/arm/boot/dts/nxp/imx/imx6dl-gw52xx.dts
> @@ -32,12 +32,10 @@ adv7180_to_ipu1_csi1_mux: endpoint {
>  };
>  
>  &ipu1_csi1_from_ipu1_csi1_mux {
> -	bus-width = <8>;
>  };

[Severity: Low]
This isn't a bug, but removing the bus-width property here leaves behind a
completely empty node override. Should these empty node overrides be removed
entirely to prevent unnecessary clutter?

Additionally, the commit message only mentions video-mux endpoints, but this
removes the property from IPU CSI endpoints as well. Could the commit message
be updated to reflect the full scope of the changes?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-imx25_dts_simple_warning_2-v2-0-c6557df516a9@nxp.com?part=2

^ permalink raw reply

* Re: [PATCH v4] dt-bindings: iommu: riscv: Add bindings for Tenstorrent RISC-V IOMMU
From: Conor Dooley @ 2026-05-21 20:08 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Tomasz Jeznach, Joerg Roedel, Will Deacon, Robin Murphy,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, iommu, linux-riscv,
	devicetree, linux-kernel, Joel Stanley, Joerg Roedel,
	Nicholas Piggin
In-Reply-To: <20260521170652.1880662-2-fustini@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH V8 02/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding
From: Conor Dooley @ 2026-05-21 20:08 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Jonathan Cameron, Chris Morgan, linux-iio, andy, nuno.sa,
	dlechner, jean-baptiste.maneyrol, linux-rockchip, devicetree,
	heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
	Krzysztof Kozlowski
In-Reply-To: <PH0PR19MB99733879756FFB321CDCDD9B81A50E2@PH0PR19MB997338.namprd19.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

On Thu, May 21, 2026 at 12:43:09PM -0500, Chris Morgan wrote:
> On Thu, May 21, 2026 at 05:44:21PM +0100, Conor Dooley wrote:
> > On Wed, May 20, 2026 at 05:42:17PM +0100, Jonathan Cameron wrote:
> > > On Mon, 18 May 2026 15:05:17 -0500
> > > Chris Morgan <macroalpha82@gmail.com> wrote:
> > > 
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Add devicetree binding for the Invensense ICM42607 and Invensense
> > > > ICM42607P inertial measurement unit. This unit is a combined
> > > > accelerometer, gyroscope, and thermometer available via I2C or SPI.
> > > > 
> > > > This device is functionally very similar to the icm42600 series with a
> > > > very different register layout.
> > > > 
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > > Note that Sashiko has highlighted that the binding this being added to
> > > has a potential problem.
> > > 
> > > interrupts are required but interrupt-names are not.
> > > That would be fine but the binding doesn't say there is a default
> > > ordering for the interrupts - so if we don't have names we have no
> > > idea which interrupt it is.
> > > 
> > > This needs fixing - probably by adding a default
> > 
> > Worth pointing out that this isn't an issue with this particular patch,
> > the problem exists in mainline.
> 
> The driver I lovingly borrowed this code from seems to have fallback
> logic, basically picking the first interrupt if it couldn't find one
> named "INT1". I was told early on not to do this that way, so in my
> case the interrupt-names would be required (but not for the existing
> driver because of this fallback).
> 
> Should I make the requirement conditional just to my compatible
> strings?

Sure, sounds like a good idea to me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v3] dt-bindings: iommu: riscv: Add bindings for Tenstorrent RISC-V IOMMU
From: Conor Dooley @ 2026-05-21 20:05 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Tomasz Jeznach, Joerg Roedel, Will Deacon, Robin Murphy,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, iommu, linux-riscv,
	devicetree, linux-kernel, Joel Stanley, Joerg Roedel,
	Nicholas Piggin
In-Reply-To: <20260521064445.1740782-2-fustini@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On Wed, May 20, 2026 at 11:41:18PM -0700, Drew Fustini wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
> 
> Extend the binding to cover details specific to the Tenstorrent RISC-V
> IOMMU. In particular, a second register range is added which contains
> M-privileged registers, e.g., PMAs and PMPs.
> 
> The RISC-V spec S-privileged registers remain in the first register
> range and are compatible with "riscv,iommu" so the Linux driver does not
> notice any difference, but the binding will be used by OpenSBI and
> potentially other M-mode software.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Acked-by: Joerg Roedel <joerg.roedel@amd.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> [fustini: fix dt_binding_check errors]
> Signed-off-by: Drew Fustini <fustini@kernel.org>

Discussed on IRC I think, sashiko complaint to be fixed.
pw-bot: changes-requested

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string
From: Conor Dooley @ 2026-05-21 20:03 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Pawel Laszczak via B4 Relay, conor+dt, devicetree, robh
In-Reply-To: <20260521081651.D22181F000E9@smtp.kernel.org>

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

On Thu, May 21, 2026 at 08:16:51AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The schema does not enforce the presence of the `dr_mode` property when `cdns,cdnsp` is matched, despite the commit message explicitly requiring it.
> - [Medium] Array bounds (`minItems` and `maxItems`) for `reg`, `reg-names`, and `interrupt-names` are not explicitly constrained inside the conditionals.
> - [Medium] Use of the empty schema `{}` as a wildcard for compatible strings breaks dt-schema validation and explicit documentation rules.
> --
> 
> commit 1f787c873a0b8a21200340bba7dffd95f1916193
> Author: Pawel Laszczak <pawell@cadence.com>
> 
> dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string
> 
> Introduce a new generic fallback compatible string cdns,cdnsp for
> Cadence USBSSP controllers without the Dual-Role Device (DRD) register
> block. It restricts the resources to host and device configurations,
> requiring dr_mode to be explicitly set.
> 
> > diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > @@ -17,22 +17,24 @@ description:
> >  
> >  properties:
> >    compatible:
> > -    const: cdns,usb3
> > +    oneOf:
> > +      - const: cdns,usb3
> > +      - items:
> > +          - {}
> 
> [Severity: Medium]
> The commit message mentions using an empty schema wildcard {} to allow
> future vendor SoC compatibles to be prepended. Does this break dt-schema
> meta-validation during dt_binding_check? The tooling usually expects
> concrete compatible strings to be explicitly documented via an enum rather
> than using a wildcard.

Obviously this is intentional.

> > +          - const: cdns,cdnsp
> 
> [ ... ]
> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: cdns,cdnsp
> > +    then:
> > +      properties:
> > +        reg:
> > +          items:
> > +            - description: XHCI Host controller registers
> > +            - description: DEVICE controller registers
> > +        reg-names:
> > +          items:
> > +            - const: xhci
> > +            - const: dev
> 
> [Severity: Medium]
> In dt-schema, specifying an items list inside conditional branches does not
> automatically override the top-level minItems and maxItems length constraints.
> 
> Since the top-level schema allows 2 to 3 items, would omitting maxItems: 2
> for reg and reg-names in this 'then' branch allow a 3-item configuration
> to incorrectly pass validation? Similarly, does omitting minItems: 3 in the
> 'else' branch allow a 2-item configuration to pass for cdns,usb3?

This looks valid, easily fixed by adding the correct fooItems
properties.

> 
> > +        interrupts:
> > +          maxItems: 2
> > +        interrupt-names:
> > +          items:
> > +            - const: host
> > +            - const: peripheral
> > +        dr_mode:
> > +          enum: [host, peripheral]
> 
> [Severity: Medium]
> The commit message states that dr_mode must be explicitly set to either
> 'host' or 'peripheral' when this compatible is matched.
> 
> Should dr_mode be added to a required list in this block? Without it, the
> property remains optional, and device trees missing dr_mode could silently
> pass validation.


This is a good point, because the default dr_mode is otg.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/1] dt-bindings: trivial-devices: add fsl,mc1323
From: sashiko-bot @ 2026-05-21 20:02 UTC (permalink / raw)
  To: Frank.Li; +Cc: devicetree, Frank.Li, conor+dt, robh, imx
In-Reply-To: <20260521195233.1532852-1-Frank.Li@oss.nxp.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The commit message incorrectly cites a CHECK_DTBS warning for a TV encoder ('fsl,imx53-tve') instead of the ZigBee transceiver ('fsl,mc1323') that the patch actually adds.
- [Low] A complex ZigBee radio transceiver is incorrectly classified as a trivial device, masking an incomplete device tree node.
--

commit 2100488cb966e136f1c1fefe29da8444b4dcd893
Author: Frank Li <Frank.Li@nxp.com>
dt-bindings: trivial-devices: add fsl,mc1323

This commit adds the Freescale MC1323 2.4 GHz IEEE 802.15.4/ZigBee 
transceiver to the trivial-devices bindings list to resolve dtbs_check 
warnings on the i.MX53 SMD board.

> Add freescale 2.4 GHz IEEE® 802.15.4/ZigBee mc1323 to fix the below
> CHECK_DTBS warnings.
>   arch/arm/boot/dts/nxp/imx/imx53-smd.dtb: /soc/bus@60000000/tve@63ff0000: failed to match any schema with compatible: ['fsl,imx53-tve']

[Severity: Medium]
Is the CHECK_DTBS warning cited here correct for this patch?

The warning mentions a TV encoder ('fsl,imx53-tve') at a different node,
but this patch adds the ZigBee transceiver ('fsl,mc1323'). 

It looks like the wrong error log might have been pasted into the commit
message.

> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 23fd4513933a4..28b059c858dd1 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -127,6 +127,8 @@ properties:
>            - domintech,dmard09
>              # DMARD10: 3-axis Accelerometer
>            - domintech,dmard10
> +            # Freescale 2.4 GHz IEEE® 802.15.4/ZigBee
> +          - fsl,mc1323

[Severity: Low]
Should a complex ZigBee radio transceiver be classified as a trivial device?

The trivial-devices.yaml schema is intended for simple devices requiring
only compatible, reg, and optionally interrupts properties. As an IEEE
802.15.4 MAC/PHY transceiver, wouldn't the MC1323 inherently require other
hardware resources like clocks or regulators?

Adding it to the trivial list seems to bypass proper DT validation and
could mask an incomplete device tree node.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521195233.1532852-1-Frank.Li@oss.nxp.com?part=1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox