Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support
From: Andre Przywara @ 2026-05-13 21:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sanjay Chitroda, Michal Piekos, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	Maksim Kiselev, linux-iio, devicetree, linux-arm-kernel,
	linux-sunxi, linux-kernel
In-Reply-To: <agTbFbicv9yzTQb9@ashevche-desk.local>

On Wed, 13 May 2026 23:12:05 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

Hi Andy,

> On Wed, May 13, 2026 at 01:53:49PM +0200, Andre Przywara wrote:
> > On 5/13/26 13:44, Sanjay Chitroda wrote:  
> > > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:  
> 
> > > > +	if (ret <= 0)  
> > > 
> > > Thank you Michal for the change.
> > > 
> > > Have you validated the changes ?
> > > It looks while success ret would be 0 and it would give return error.

No, it doesn't. Returning 0 means no clocks found:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-devres.c#n300

> 
> Good catch!
> 
> > But devm_clk_bulk_get_all_enabled() returns the number of clocks found and
> > enabled. And since we need at least one, I think this is correct, and the
> > error message below reflects that.
> > 
> > To me that change looks good:
> > 
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>  
> 
> == 0 ???
> Doesn't look like correct code.

Not sure I follow:
devm_clk_bulk_get_all_enabled() returns the number of clocks in that
node, or a negative error value. If it returns 0, that means no clocks
have been found, which is an error in our case, since we expect at
least one clock. This is what the second part of the error message
refers to.
So we want one or two as the return value, with the current bindings,
but really anything greater than 0 is fine, from the driver's
perspective, since we don't care about the clocks beyond them being
enabled.

So am I missing something?

Cheers,
Andre


^ permalink raw reply

* [PATCH] arm64: dts: ti: k3-am62l: Enable HDMI & DSI display
From: Swamil Jain @ 2026-05-13 21:09 UTC (permalink / raw)
  To: nm, vigneshr, kristo, robh, krzk+dt, conor+dt, tomi.valkeinen
  Cc: r-sharma3, devarsht, praneeth, linux-arm-kernel, devicetree,
	linux-kernel, s-jain1

From: Devarsh Thakkar <devarsht@ti.com>

Enable DSS, DSI and HDMI related nodes and pinmuxes.

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
Signed-off-by: Swamil Jain <s-jain1@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62l-main.dtsi | 47 ++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-am62l3-evm.dts  | 58 +++++++++++++++++++++++
 2 files changed, 105 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi
index 80615ca1e01a..1d255c9138db 100644
--- a/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi
@@ -584,4 +584,51 @@ scmi_shmem: sram@0 {
 			bootph-all;
 		};
 	};
+
+	dss: dss@30200000 {
+		compatible = "ti,am62l-dss";
+		reg = <0x00 0x30200000 0x00 0x1000>, /* common */
+		      <0x00 0x30202000 0x00 0x1000>, /* vidl1 */
+		      <0x00 0x30207000 0x00 0x1000>, /* ovr1 */
+		      <0x00 0x3020a000 0x00 0x1000>, /* vp1 */
+		      <0x00 0x30201000 0x00 0x1000>; /* common1 */
+		reg-names = "common", "vidl1", "ovr1", "vp1", "common1";
+		power-domains = <&scmi_pds 39>;
+		clocks = <&scmi_clk 162>,
+			 <&scmi_clk 161>;
+		clock-names = "fck", "vp1";
+		interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+
+		dss_ports: ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	dphy_tx0: phy@301c0000 {
+		compatible = "ti,j721e-dphy";
+		reg = <0x0 0x301c0000 0x0 0x1000>;
+		clocks = <&scmi_clk 348>, <&scmi_clk 343>;
+		clock-names = "psm", "pll_ref";
+		#phy-cells = <0>;
+		power-domains = <&scmi_pds 86>;
+		assigned-clocks = <&scmi_clk 343>;
+		assigned-clock-parents = <&scmi_clk 341>;
+		assigned-clock-rates = <25000000>;
+		status = "disabled";
+	};
+
+	dsi0: dsi@30500000 {
+		compatible = "ti,j721e-dsi";
+		reg = <0x0 0x30500000 0x0 0x100000>, <0x0 0x30270000 0x0 0x100>;
+		clocks = <&scmi_clk 155>, <&scmi_clk 158>;
+		clock-names = "dsi_p_clk", "dsi_sys_clk";
+		power-domains = <&scmi_pds 38>;
+		interrupt-parent = <&gic500>;
+		interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+		phys = <&dphy_tx0>;
+		phy-names = "dphy";
+		status = "disabled";
+	};
 };
diff --git a/arch/arm64/boot/dts/ti/k3-am62l3-evm.dts b/arch/arm64/boot/dts/ti/k3-am62l3-evm.dts
index a1af4571a815..7462921e2180 100644
--- a/arch/arm64/boot/dts/ti/k3-am62l3-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62l3-evm.dts
@@ -119,6 +119,18 @@ vcc_1v8: regulator-3 {
 		regulator-always-on;
 		regulator-boot-on;
 	};
+
+	hdmi0: connector-hdmi {
+		compatible = "hdmi-connector";
+		label = "hdmi";
+		type = "a";
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&sii9022_out>;
+			};
+		};
+	};
 };
 
 &gpio0 {
@@ -192,6 +204,36 @@ exp2: gpio@23 {
 		bootph-all;
 	};
 
+	sii9022: bridge-hdmi@3b {
+		compatible = "sil,sii9022";
+		reg = <0x3b>;
+		interrupt-parent = <&exp1>;
+		interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+		#sound-dai-cells = <0>;
+		sil,i2s-data-lanes = < 0 >;
+		bootph-all;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				sii9022_in: endpoint {
+					remote-endpoint = <&dpi_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				sii9022_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
 };
 
 &i2c2 {
@@ -375,6 +417,22 @@ &usb1 {
 	pinctrl-0 = <&usb1_default_pins>;
 };
 
+&dss {
+	status = "okay";
+	bootph-all;
+};
+
+&dss_ports {
+	/* VP1: DPI Output */
+	port@0 {
+		reg = <0>;
+
+		dpi_out: endpoint {
+			remote-endpoint = <&sii9022_in>;
+		};
+	};
+};
+
 &wkup_uart0 {
 	pinctrl-0 = <&wkup_uart0_pins_default>;
 	pinctrl-names = "default";


^ permalink raw reply related

* [PATCH v1] ARM: devtree: Fix /cpus node reference leak
From: Yuho Choi @ 2026-05-13 20:57 UTC (permalink / raw)
  To: Russell King; +Cc: linux-arm-kernel, linux-kernel, Yuho Choi

of_find_node_by_path() returns a referenced device node.  In
arm_dt_init_cpu_maps(), the /cpus node is kept across CPU node parsing
but is never released on the success path or on post-acquire error
paths.

Route all exits after the /cpus lookup through a common cleanup label so
the node reference is dropped.

Fixes: a0ae02405076a ("ARM: kernel: add device tree init map function")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 arch/arm/kernel/devtree.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 3b78966e750a..88a072d3b8e1 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -93,7 +93,7 @@ void __init arm_dt_init_cpu_maps(void)
 		 */
 		if (hwid & ~MPIDR_HWID_BITMASK) {
 			of_node_put(cpu);
-			return;
+			goto out_put_cpus;
 		}
 
 		/*
@@ -107,7 +107,7 @@ void __init arm_dt_init_cpu_maps(void)
 			if (WARN(tmp_map[j] == hwid,
 				 "Duplicate /cpu reg properties in the DT\n")) {
 				of_node_put(cpu);
-				return;
+				goto out_put_cpus;
 			}
 
 		/*
@@ -149,7 +149,7 @@ void __init arm_dt_init_cpu_maps(void)
 
 	if (!bootcpu_valid) {
 		pr_warn("DT missing boot CPU MPIDR[23:0], fall back to default cpu_logical_map\n");
-		return;
+		goto out_put_cpus;
 	}
 
 	/*
@@ -162,6 +162,9 @@ void __init arm_dt_init_cpu_maps(void)
 		cpu_logical_map(i) = tmp_map[i];
 		pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
 	}
+
+out_put_cpus:
+	of_node_put(cpus);
 }
 
 bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
-- 
2.43.0



^ permalink raw reply related

* Re: [PATCH v2 11/11] iio: dac: add mcf54415 DAC
From: Andy Shevchenko @ 2026-05-13 20:28 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: Greg Ungerer, Geert Uytterhoeven, Steven King, Arnd Bergmann,
	Maxime Coquelin, Alexandre Torgue, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Greg Ungerer,
	linux-m68k, linux-kernel, linux-stm32, linux-arm-kernel,
	linux-iio
In-Reply-To: <20260513-wip-stmark2-dac-v2-11-fcdae50cf51a@baylibre.com>

On Wed, May 13, 2026 at 11:14:35AM +0200, Angelo Dureghello wrote:
> 
> Add basic version of mcf54415 DAC driver. DAC is embedded in the cpu and
> DAC configuration registers are mapped in the internal IO address space.
> 
> The DAC accepts a 12-bit digital signal and creates a monotonic 12-bit
> analog output varying from DAC_VREFL to DAC_VREFH. The DAC module
> consists of a conversion unit, an output amplifier, and the associated
> digital control blocks. Default register values for DAC_VREFL and DAC_VREFH
> are respectively 0 and 0xfff, left untouched in this initial version.
> 
> This initial version of the driver is minimalistic, "output raw" only, to
> be extended in the future. DMA and external sync are disabled, default mode
> is high speed, default format is right-justified 12bit on 16bit word.

...

> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/compiler_types.h>
> +#include <linux/delay.h>

+ err.h

> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>

...

> +struct mcf54415_dac {
> +	struct clk *clk;
> +	struct regmap *map;

I believe that regmap pointer is used more often, can you check with
bloat-o-meter that swapping these two (by the order) gives any benefit in
object size?

> +};

...

> +	.max_register = 0x1F,

No definition? What datasheet says about this? Perhaps define the MAX as per
last defined register in the datasheet?

> +};

...

> +static void mcf54415_dac_init(struct mcf54415_dac *info)
> +{
> +	/* Keeping defaults and enable DAC (bit 0 set to 0) */
> +	regmap_write(info->map, MCF54415_DAC_CR, MCF54415_DAC_CR_FILT |
> +		     FIELD_PREP(MCF54415_DAC_CR_WMLVL, 1));

Seems the whole driver ignores IO errors, why?

> +	/* DAC is ready after 12us, from RM table 40-3  */
> +	fsleep(12);
> +}

...

> +static void mcf54415_dac_exit(void *data)
> +{
> +	struct mcf54415_dac *info = data;
> +
> +	regmap_update_bits(info->map, MCF54415_DAC_CR, MCF54415_DAC_CR_PDN,
> +			   MCF54415_DAC_CR_PDN);

regmap_set_bits()

> +}

...

> +static int mcf54415_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	struct mcf54415_dac *info = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (val < 0 || val > 4095)

Do we have a definition for the resolution? I'm fine with the plain numbers,
but it's better to add a short comment to say that this is "based on the
resolution of XXX register per datasheet".

> +			return -EINVAL;
> +		regmap_write(info->map, MCF54415_DAC_DATA, val);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static int mcf54415_dac_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct mcf54415_dac *info;
> +	void __iomem *regs;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	info = iio_priv(indio_dev);
> +
> +	regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(regs))
> +		return dev_err_probe(dev, PTR_ERR(regs),
> +				     "failed to get io regs\n");

One line.

> +
> +	info->map = devm_regmap_init_mmio(dev, regs,
> +					  &mcf54415_dac_regmap_config);
> +	if (IS_ERR(info->map))
> +		return PTR_ERR(info->map);
> +
> +	info->clk = devm_clk_get_enabled(dev, "dac");
> +	if (IS_ERR(info->clk))
> +		return dev_err_probe(dev, PTR_ERR(info->clk),
> +				     "failed getting clock\n");

Also can be a single line, but this one a bit longer than above, gives
88 characters.

> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	indio_dev->name = "mcf54415";
> +	indio_dev->info = &mcf54415_dac_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mcf54415_dac_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mcf54415_dac_iio_channels);
> +
> +	mcf54415_dac_init(info);
> +
> +	ret = devm_add_action_or_reset(dev, mcf54415_dac_exit, info);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH] media: rc: sunxi-cir: unregister rc device on probe failure
From: Jernej Škrabec @ 2026-05-13 20:23 UTC (permalink / raw)
  To: Sean Young, Mauro Carvalho Chehab, 박명훈
  Cc: Myeonghun Pak, Chen-Yu Tsai, Samuel Holland, linux-media,
	linux-arm-kernel, linux-sunxi, linux-kernel, stable, Ijae Kim
In-Reply-To: <20260424000000.558-1-mhun512@gmail.com>

Dne sreda, 13. maj 2026 ob 09:11:03 Srednjeevropski poletni čas je 박명훈 napisal(a):
> From: Myeonghun Pak <mhun512@gmail.com>
> 
> After rc_register_device() succeeds, later probe failures must undo the
> registration with rc_unregister_device(). The current error path jumps to
> the allocation cleanup label and only calls rc_free_device(), leaving the
> rc device registration and resources created by rc_register_device()
> behind.
> 
> Add a registered-device unwind label for the IRQ lookup, IRQ request, and
> hardware initialization failure paths. Keep rc_free_device() for failures
> before rc_register_device() succeeds.
> 
> Fixes: b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
> Cc: stable@vger.kernel.org
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>  drivers/media/rc/sunxi-cir.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 92ef4e7c6f..cc64a68dfe 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	ir->irq = platform_get_irq(pdev, 0);
>  	if (ir->irq < 0) {
>  		ret = ir->irq;
> -		goto exit_free_dev;
> +		goto exit_unregister_dev;
>  	}
>  
>  	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
>  	if (ret) {
>  		dev_err(dev, "failed request irq\n");
> -		goto exit_free_dev;
> +		goto exit_unregister_dev;
>  	}
>  
>  	ret = sunxi_ir_hw_init(dev);
>  	if (ret)
> -		goto exit_free_dev;
> +		goto exit_unregister_dev;
>  
>  	dev_info(dev, "initialized sunXi IR driver\n");
>  	return 0;
>  
> +exit_unregister_dev:
> +	rc_unregister_device(ir->rc);
> +	return ret;

This return should not be here.  Device must be freed too.

Best regards,
Jernej

> +
>  exit_free_dev:
>  	rc_free_device(ir->rc);
>  
> 






^ permalink raw reply

* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: David Sterba @ 2026-05-13 20:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Catalin Marinas, Will Deacon, Ard Biesheuvel,
	Huacai Chen, WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Herbert Xu, Dan Williams,
	Chris Mason, David Sterba, Arnd Bergmann, Song Liu, Yu Kuai,
	Li Nan, linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
	linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
	linux-raid
In-Reply-To: <20260513054742.GA1018@lst.de>

On Wed, May 13, 2026 at 07:47:42AM +0200, Christoph Hellwig wrote:
> On Tue, May 12, 2026 at 01:42:31PM +0200, David Sterba wrote:
> > On Tue, May 12, 2026 at 07:20:41AM +0200, Christoph Hellwig wrote:
> > > While the RAID6 algorithm could in theory support 3 devices by just
> > > copying the data disk to the two parity disks, this version is not only
> > > useless because it is a suboptimal version of 3-way mirroring, but also
> > > broken with various crashes and incorrect parity generation in various
> > > architecture-optimized implementations.  Disallow it similar to mdraid
> > > which requires at least 4 devices for RAID 6.
> > > 
> > > Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > This patch should have been sent separately as it has user visible
> > impact and can potentially break some setups.
> 
> It _is_ sent out separate.

It's public interface change of btrfs but in a patch series cleaning
up some library code, I noticed it by accident.

> > The degenerate modes of
> > raid0, 5, or 6 are explicit as a possible middle step when converting
> > profiles.  We can use a fallback implementation for this case if the
> > accelerated implementations cannot do it.
> 
> This is not about a degenerated mode.  For a degenerated RAID 6, parity
> generation uses the RAID 5 XOR routines as the second parity will be
> missing.  This is about generating two parities for a single data disk,
> which must be explicitly selected.

The calcuation is a different than what I'm concened about, changing
minimum devices from 3 to 4 is a breaking change. If the library won't
provide the xor/parity functions then we'll have to add a fallback for
this special case.


^ permalink raw reply

* Re: [PATCH v2 09/11] m68k: stmark2: use ioport.h macros for resources
From: Andy Shevchenko @ 2026-05-13 20:18 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: Greg Ungerer, Geert Uytterhoeven, Steven King, Arnd Bergmann,
	Maxime Coquelin, Alexandre Torgue, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Greg Ungerer,
	linux-m68k, linux-kernel, linux-stm32, linux-arm-kernel,
	linux-iio
In-Reply-To: <20260513-wip-stmark2-dac-v2-9-fcdae50cf51a@baylibre.com>

On Wed, May 13, 2026 at 11:14:33AM +0200, Angelo Dureghello wrote:

> Align also other resource declaration using DEFINE_RES_.

DEFINE_RES_*()

...

>  static struct resource dspi_spi0_resource[] = {
> -	[0] = {
> -		.start = MCFDSPI_BASE0,
> -		.end   = MCFDSPI_BASE0 + 0xFF,
> -		.flags = IORESOURCE_MEM,
> -		},
> -	[1] = {
> -		.start = 12,
> -		.end   = 13,
> -		.flags = IORESOURCE_DMA,
> -	},
> -	[2] = {
> -		.start = MCF_IRQ_DSPI0,
> -		.end   = MCF_IRQ_DSPI0,
> -		.flags = IORESOURCE_IRQ,
> -	},
> +	DEFINE_RES_MEM(MCFDSPI_BASE0, 0x100),
> +	DEFINE_RES_DMA(12),
> +	DEFINE_RES_DMA(13),
> +	DEFINE_RES_IRQ(MCF_IRQ_DSPI0),
>  };

This conversion drops hard indices, would it be a problem (for example,
some code modifies the fields based on the [hard coded] index...)?

If not, perhaps good to mention in the commit message and move IRQ to be the
second one?

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH v2 08/11] m68k: stmark2: add mcf5441x DAC platform devices
From: Andy Shevchenko @ 2026-05-13 20:16 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: Greg Ungerer, Geert Uytterhoeven, Steven King, Arnd Bergmann,
	Maxime Coquelin, Alexandre Torgue, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Greg Ungerer,
	linux-m68k, linux-kernel, linux-stm32, linux-arm-kernel,
	linux-iio
In-Reply-To: <20260513-wip-stmark2-dac-v2-8-fcdae50cf51a@baylibre.com>

On Wed, May 13, 2026 at 11:14:32AM +0200, Angelo Dureghello wrote:

> Add mcf5441x DAC platform devices.

...

> +static struct resource dac0_resource[] = {
> +	DEFINE_RES_MEM(MCFDAC_BASE0, 0x100),
> +};
> +
> +static struct platform_device dac0_device = {
> +	.name = "mcf54415_dac",
> +	.id = 0,
> +	.num_resources = ARRAY_SIZE(dac0_resource),
> +	.resource = dac0_resource,
> +};

It can be

static struct resource dac0_resource = DEFINE_RES_MEM(MCFDAC_BASE0, 0x100);

static struct platform_device dac0_device = {
	.name = "mcf54415_dac",
	.id = 0,
	.num_resources = 1,
	.resource = &dac0_resource,
};

...

Same for the rest. But I haven't seen the followups and don't know if those
structures are going to be expanded. The commit message doesn't say anything
about that. So I assume my suggestion valid.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support
From: Andy Shevchenko @ 2026-05-13 20:12 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Sanjay Chitroda, Michal Piekos, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	Maksim Kiselev, linux-iio, devicetree, linux-arm-kernel,
	linux-sunxi, linux-kernel
In-Reply-To: <4248ac7e-3cce-4db8-a2c1-710f2c903490@arm.com>

On Wed, May 13, 2026 at 01:53:49PM +0200, Andre Przywara wrote:
> On 5/13/26 13:44, Sanjay Chitroda wrote:
> > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:

> > > +	if (ret <= 0)
> > 
> > Thank you Michal for the change.
> > 
> > Have you validated the changes ?
> > It looks while success ret would be 0 and it would give return error.

Good catch!

> But devm_clk_bulk_get_all_enabled() returns the number of clocks found and
> enabled. And since we need at least one, I think this is correct, and the
> error message below reflects that.
> 
> To me that change looks good:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

== 0 ???
Doesn't look like correct code.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support
From: Andy Shevchenko @ 2026-05-13 20:10 UTC (permalink / raw)
  To: Michal Piekos
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Maksim Kiselev, linux-iio,
	devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <20260513-sunxi-a523-gpadc-v2-2-d5efde151dac@mmpsystems.pl>

On Wed, May 13, 2026 at 06:59:43AM +0200, Michal Piekos wrote:
> A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
> clock and module clock.
> 
> Change driver to enable all clocks.

...

>  	struct iio_dev *indio_dev;
>  	struct sun20i_gpadc_iio *info;
>  	struct reset_control *rst;
> -	struct clk *clk;
> +	struct clk_bulk_data *clks;

Try to follow reversed xmas tree order.

>  	int irq;
>  	int ret;

...

> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> +	if (ret <= 0)
> +		return dev_err_probe(
> +			dev, ret,
> +			"failed to enable clocks or no clocks defined\n");

Is this done by clang-format or so? Please, don't do wrapping on the open
parenthesis. Also note for more than 10 years checkpatch does not complain
on the trailing string literals that go over 80 characters.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH net-next 1/2] net: ti: icssg: Derive stats array lengths from ARRAY_SIZE
From: Jacob Keller @ 2026-05-13 20:00 UTC (permalink / raw)
  To: MD Danish Anwar, David CARLIER
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jonathan Corbet, Shuah Khan, Roger Quadros,
	Andrew Lunn, Meghana Malladi, Kevin Hao, Vadim Fedorenko, netdev,
	linux-doc, linux-kernel, linux-arm-kernel, Vignesh Raghavendra
In-Reply-To: <6a1f411c-d7ed-463b-abf1-277d8cc0c184@ti.com>

On 5/12/2026 2:40 AM, MD Danish Anwar wrote:
> Hi David,
> 
> On 12/05/26 1:28 pm, David CARLIER wrote:
>> Hi MD,
>>
>> On Tue, 12 May 2026 at 07:06, MD Danish Anwar <danishanwar@ti.com> wrote:
>>>
>>> Replace the manually maintained ICSSG_NUM_MIIG_STATS and
>>> ICSSG_NUM_PA_STATS constants with ARRAY_SIZE() expressions derived
>>> directly from the corresponding stat descriptor arrays, so that adding
>>> new entries to icssg_all_miig_stats[] or icssg_all_pa_stats[] no longer
>>> requires a separate update to a numeric constant.
>>>
>>> To make this self-contained, break the circular include dependency
>>> between icssg_stats.h and icssg_prueth.h:
>>>
>>>   - icssg_stats.h previously included icssg_prueth.h (transitively
>>>     pulling in icssg_switch_map.h and ETH_GSTRING_LEN).  Replace that
>>>     with direct includes of <linux/ethtool.h>, <linux/kernel.h> and
>>>     "icssg_switch_map.h".
>>>
>>>   - icssg_prueth.h now includes icssg_stats.h, giving it access to
>>>     the ARRAY_SIZE-based ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS
>>>     before they are used in the prueth_emac struct and ICSSG_NUM_STATS.
>>>
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +--
>>>  drivers/net/ethernet/ti/icssg/icssg_stats.h  | 7 ++++++-
>>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> index df93d15c5b78..e2ccecb0a0dd 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> @@ -43,6 +43,7 @@
>>>
>>>  #include "icssg_config.h"
>>>  #include "icss_iep.h"
>>> +#include "icssg_stats.h"
>>>  #include "icssg_switch_map.h"
>>>
>>>  #define PRUETH_MAX_MTU          (2000 - ETH_HLEN - ETH_FCS_LEN)
>>> @@ -57,8 +58,6 @@
>>>
>>>  #define ICSSG_MAX_RFLOWS       8       /* per slice */
>>>
>>> -#define ICSSG_NUM_PA_STATS     32
>>> -#define ICSSG_NUM_MIIG_STATS   60
>>>  /* Number of ICSSG related stats */
>>>  #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
>>>  #define ICSSG_NUM_STANDARD_STATS 31
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>> index 5ec0b38e0c67..b854eb587c1e 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>> @@ -8,10 +8,15 @@
>>>  #ifndef __NET_TI_ICSSG_STATS_H
>>>  #define __NET_TI_ICSSG_STATS_H
>>>
>>> -#include "icssg_prueth.h"
>>> +#include <linux/ethtool.h>
>>> +#include <linux/kernel.h>
>>> +#include "icssg_switch_map.h"
>>>
>>>  #define STATS_TIME_LIMIT_1G_MS    25000    /* 25 seconds @ 1G */
>>>
>>> +#define ICSSG_NUM_MIIG_STATS   ARRAY_SIZE(icssg_all_miig_stats)
>>> +#define ICSSG_NUM_PA_STATS     ARRAY_SIZE(icssg_all_pa_stats)
>>> +
>>>  struct miig_stats_regs {
>>>         /* Rx */
>>>         u32 rx_packets;
>>> --
>>> 2.34.1
>>>
>>
>> One thing that caught my eye: icssg_all_miig_stats[] and
>>   icssg_all_pa_stats[] are 'static const' arrays in icssg_stats.h with
>>   ETH_GSTRING_LEN name buffers per entry. Right now only icssg_stats.c
>>   and icssg_ethtool.c pull them in. After this patch icssg_prueth.h
>>   includes icssg_stats.h, so every .c in the driver (classifier,
>>   common, config, mii_cfg, queues, switchdev, ...) ends up with its own
>>   static-const copy of both tables.
>>
>>   Would a static_assert() work for what you're after? Something like:
>>
> 
> While adding more stats manually, The ARRAY_SIZE() approach was
> explicitly requested by maintainer [1]:
> 
> This patch is a direct response to that feedback. static_assert() would
> still require updating the numeric constant on every array change. The
> goal here is to eliminate the need of manually incrementing stats count
> whenever new stats are added
> 
> Your concern about multiple copies of table is noted and valid. Could
> you advise on the preferred way to reconcile these two requirements? I
> am happy to restructure if there is an approach that satisfies both.
> 
The way we solved this in the Intel drivers is to use a single array
which contains both the stat name as well as the offset from the
structure where the stat resides.

The stat string code just iterates over the stat list for the strings,
while the stat value code iterates the array and computes the stat
address from the offset and size and base structure pointer. Each object
that has stats has its own stat array structure.

This is probably overkill, but the advantage is that the strings and
their values are stored together and adding a new stat is as simple as
adding a new entry to that list.

I.e.

struct ice_stats {
        char stat_string[ETH_GSTRING_LEN];
        int sizeof_stat;
        int stat_offset;
};

#define ICE_STAT(_type, _name, _stat) { \
        .stat_string = _name, \
        .sizeof_stat = sizeof_field(_type, _stat), \
        .stat_offset = offsetof(_type, _stat) \
}

#define ICE_VSI_STAT(_name, _stat) \
                ICE_STAT(struct ice_vsi, _name, _stat)
#define ICE_PF_STAT(_name, _stat) \
                ICE_STAT(struct ice_pf, _name, _stat)


Then the stats for the individial arrays are defined like this:

static const struct ice_stats ice_gstrings_vsi_stats[] = {
        ICE_VSI_STAT(ICE_RX_UNICAST, eth_stats.rx_unicast),
        ICE_VSI_STAT(ICE_TX_UNICAST, eth_stats.tx_unicast),
        ICE_VSI_STAT(ICE_RX_MULTICAST, eth_stats.rx_multicast),
        ICE_VSI_STAT(ICE_TX_MULTICAST, eth_stats.tx_multicast),
        ICE_VSI_STAT(ICE_RX_BROADCAST, eth_stats.rx_broadcast),
        ICE_VSI_STAT(ICE_TX_BROADCAST, eth_stats.tx_broadcast),
	...
};

(Note, ICE_RX_UNICAST is a macro that defines the string value.. I don't
recall who changed this to macros or why vs just having the strings be
directly in the definition...)

This is probably a lot bigger refactor to make work, and may not be
exactly suitable for your driver. I've considered "upgrading" these data
structures and logic as helpers to the core ethtool code (or perhaps
now, to libeth) but never got around to it.


^ permalink raw reply

* Re: [PATCH tty v4 2/6] serial: Replace driver usage of UPF_CONS_FLOW
From: Krzysztof Kozlowski @ 2026-05-13 19:50 UTC (permalink / raw)
  To: John Ogness, Greg Kroah-Hartman, Jiri Slaby, Alim Akhtar
  Cc: Andy Shevchenko, linux-kernel, David S. Miller,
	Ilpo Järvinen, Andy Shevchenko, Thomas Fourier, Kees Cook,
	linux-serial, linux-arm-kernel, linux-samsung-soc, sparclinux,
	Peter Griffin, Tudor Ambarus, André Draszik, Alexey Klimov
In-Reply-To: <87lddvy2dl.fsf@jogness.linutronix.de>

On 07/05/2026 11:50, John Ogness wrote:
> Hi,
> 
> (This email is particularly directed at the Samsung folks.)
> 
> Responding to Sashiko:
> 
> https://sashiko.dev/#/patchset/20260506121606.5805-1-john.ogness%40linutronix.de
> 
> On 2026-05-06, Sashiko wrote:
>>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
>>> index e27806bf2cf3e..2f94fc798cffb 100644
>>> --- a/drivers/tty/serial/samsung_tty.c
>>> +++ b/drivers/tty/serial/samsung_tty.c
>>> @@ -319,7 +319,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
>>>  	ourport->tx_enabled = 0;
>>>  	ourport->tx_in_progress = 0;
>>>  
>>> -	if (port->flags & UPF_CONS_FLOW)
>>> +	if (uart_cons_flow_enabled(port))
>>>  		s3c24xx_serial_rx_enable(port);
>>
>> This isn't a new bug introduced by this patch, but does this code
>> result in a recursive spinlock acquisition and deadlock when console
>> flow control is enabled?
>>
>> The serial core framework calls port->ops->stop_tx() with the
>> port->lock spinlock already held. When uart_cons_flow_enabled(port)
>> evaluates to true, s3c24xx_serial_stop_tx() invokes
>> s3c24xx_serial_rx_enable().
>>
>> Looking at s3c24xx_serial_rx_enable(), it unconditionally attempts to
>> acquire the exact same lock again:
>>
>> static void s3c24xx_serial_rx_enable(struct uart_port *port)
>> {
>> 	struct s3c24xx_uart_port *ourport = to_ourport(port);
>> 	unsigned long flags;
>> 	...
>> 	uart_port_lock_irqsave(port, &flags);
>> 	...
>> }
>>
>> Because spinlocks are not recursive in the kernel, it appears this
>> sequence would cause an immediate deadlock on the same CPU.
> 
> Although out of scope for my series, the reported issue is real. Perhaps
> something for Krzysztof or Alim to look into.
> 
>>> @@ -493,7 +493,7 @@ static void s3c24xx_serial_start_tx(struct uart_port *port)
>>>  	struct tty_port *tport = &port->state->port;
>>>  
>>>  	if (!ourport->tx_enabled) {
>>> -		if (port->flags & UPF_CONS_FLOW)
>>> +		if (uart_cons_flow_enabled(port))
>>>  			s3c24xx_serial_rx_disable(port);
>>
>> Similarly here, s3c24xx_serial_start_tx() is also called with
>> port->lock held, and s3c24xx_serial_rx_disable() will attempt to
>> acquire port->lock again:
>>
>> static void s3c24xx_serial_rx_disable(struct uart_port *port)
>> {
>> 	struct s3c24xx_uart_port *ourport = to_ourport(port);
>> 	unsigned long flags;
>> 	...
>> 	uart_port_lock_irqsave(port, &flags);
>> 	...
>> }
>>
>> Could this pre-existing locking issue in the samsung_tty driver be
>> addressed so that the rx enable/disable helpers do not try to take the
>> port lock when it is already held by the caller?
> 
> Also legitimate. But out of scope for my series.


Thanks for letting us know. Deadlock did not happen so far, so something
is missing in Sashiko's report. :)

I am just back from vacation, so I won't have time to dig into this, but
I will keep your email in the inbox. Cc-ing also a few Linaro folks
which are using this platform and might be able to help us here.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
From: Jamie Nguyen @ 2026-05-13 19:48 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260513-tremendous-conscious-sunfish-06eecb@sudeepholla>



> On May 13, 2026, at 10:15 AM, Sudeep Holla <sudeep.holla@kernel.org> wrote:
> 
> On Tue, May 12, 2026 at 08:28:00PM -0700, Jamie Nguyen wrote:
>> __ffa_partition_info_get_regs() walks the response with a hardcoded
>> 24-byte stride (regs += 3) even though the SPMC tells us the actual
>> per-descriptor size via PARTITION_INFO_SZ in x2[63:48]. The size is
>> read into buf_sz and then thrown away.
>> 
>> That works while every SPMC returns the FF-A v1.1 layout, but it falls
>> apart against a v1.3 SPMC returning the 48-byte descriptor. The loop
>> strides over half a descriptor at a time and ends up parsing every
>> other entry from a slice of two adjacent ones.
>> 
>> The FF-A spec (v1.2, section 18.5) says that the producer should
>> report the descriptor size, and the consumer is supposed to stride by
>> that size and ignore any trailing fields it doesn't understand. The
>> non-REGS path (__ffa_partition_info_get) does this already, and the
>> REGS path should match.
>> 
>> Use buf_sz for the stride, and bail out with -EPROTO if the SPMC
>> reports something we can't safely walk.
>> 
> 
> Can you check if the issue is addressed in -next by:
> Commit 3974ea193840 ("firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies")

Thanks for the pointer.  I tested 3974ea193840 on the same hardware
that reproduces the bug, but the descriptor-stride issue is still
present.

The relevant loop at the end of __ffa_partition_info_get_regs()
still has:

	buf_sz = PARTITION_INFO_SZ(partition_info.a2);
	if (buf_sz > sizeof(*buffer))
		buf_sz = sizeof(*buffer);
	...
	for (idx = 0; idx < nr_desc; idx++, buf++) {
		...
		regs += 3;  /* bug is here */
	}

With 48-byte descriptors the SPMC returns nr_desc = 2 per call,
which passes the new MAX_DESC = 5 bound, and the loop then runs
the 24-byte stride over 48-byte entries.

Here are the test results on the same SBIOS / same hardware:

  unpatched:               5 partitions enumerated, two with
                           garbled UUIDs and partition_ids
  3974ea193840 alone:      same misparsed enumeration
  3974ea193840 + my patch: 6 partitions, all entries correct

So the two fixes look complementary rather than overlapping.

> -- 
> Regards,
> Sudeep



^ permalink raw reply

* Re: [PATCH v3 1/3] dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme interrupts
From: Krzysztof Kozlowski @ 2026-05-13 19:47 UTC (permalink / raw)
  To: mani@kernel.org, Hongxing Zhu
  Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	bhelgaas@google.com, Frank Li, l.stach@pengutronix.de,
	lpieralisi@kernel.org, kwilczynski@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org
In-Reply-To: <t3d2s6rl25ywobis4xiwwdvjrccua2qrpwfsxqpvp6qw5pzofa@kjm4wzcpjcli>

On 12/05/2026 15:26, mani@kernel.org wrote:
>>>>>>
>>>>>> These interrupts are optional for existing variants (imx6q, imx6sx,
>>>>>> imx6qp, imx7d, imx8mq, imx8mm, imx8mp) to maintain backward
>>>>>> compatibility with existing device trees.
>>>>>>
>>>>>> For fsl,imx95-pcie, all 5 interrupts (msi, dma, intr, aer, pme) are
>>>>>> mandatory due to hardware requirements.
>>>>>>
>>>>>> This introduces an ABI requirement for fsl,imx95-pcie. The i.MX95
>>>>>> hardware requires dedicated interrupt lines for AER, PME, and
>>>>>> general controller events due to its redesigned interrupt
>>>>>> architecture. i.MX95 cannot function correctly without explicit
>>>>>> interrupt routing for error handling, power management and link event
>>> detection.
>>>>>
>>>>> fsl,imx95-pcie was added more than two years ago, so how it cannot
>>>>> function correctly? Are you saying that for two years you had here
>>>>> completely broken code?
>>>>>
>>>>> If this wasn't tested for two years, how can we believe anything is tested now?
>>>> The basic PCIe functionality has been working since the initial
>>>> fsl,imx95-pcie support. However, AER (Advanced Error Reporting) and
>>>> link up/down detection were not previously enabled. This patch-set
>>>> adds and verifies support for these advanced features.
>>>>
>>>
>>> That is not what you said in the commit msg.
>> Hi Krzysztof:
>> Sorry for the delayed response due to a holiday.
>> After reviewing this patch-set again, I'd like to suggest an alternative
>> approach: would it be possible to mark these newly added interrupts as
>> optional?
> 
> Yes, since even without these interrupts, PCIe functionality still works. Only
> issue is that it cannot report error and recover from LDn.

Yes. Please mention how the hardware or SW implementation behave when
these are missing, which is then serving as proof why they can be optional.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH] [v2] iommu, debugobjects: avoid gcc-16.1 section mismatch warnings
From: Arnd Bergmann @ 2026-05-13 19:35 UTC (permalink / raw)
  To: Miguel Ojeda, Arnd Bergmann
  Cc: Will Deacon, Joerg Roedel, Miguel Ojeda, Andrew Morton,
	Thomas Gleixner, Nathan Chancellor, linux-kbuild, stable,
	Robin Murphy, Kees Cook, linux-arm-kernel, iommu, linux-kernel
In-Reply-To: <CANiq72nbRw14wdZA4GH17K22Krh4ujB_wtuv9u5RQTGtidpq0g@mail.gmail.com>

On Wed, May 13, 2026, at 17:48, Miguel Ojeda wrote:
> On Wed, May 13, 2026 at 4:54 PM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> With some more experimenting, I found that marking these functions as
>> __attribute__((noipa)) is both logical and reliable.
>>
>> In order to keep the syntax readable, add a custom macro for this in
>> include/linux/compiler_attributes.h next to other related macros and
>> use it to annotate both files.
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>

Thanks!

> If you don't mind, please add underscores (i.e. `((__noipa__))`) and
> place it after `__noinline__` to keep it sorted (the file is meant to
> be sorted by actual attribute name, though some entries were added
> that break that, but I will clean that and a couple other things up at
> some point).

Ok, changed now, will wait for other comments before resending.

       Arnd


^ permalink raw reply

* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Chia-I Wu @ 2026-05-13 19:31 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Boris Brezillon, Marcin Ślusarz, Ketil Johnsen, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
	Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
	Christian König, Steven Price, Daniel Almeida, Alice Ryhl,
	Matthias Brugger, AngeloGioacchino Del Regno, dri-devel,
	linux-doc, linux-kernel, linux-media, linaro-mm-sig,
	linux-arm-kernel, linux-mediatek, Florent Tomasin, nd
In-Reply-To: <agNJasayW8VCHTiU@e142607>

On Tue, May 12, 2026 at 8:39 AM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Tue, May 12, 2026 at 04:11:11PM +0200, Boris Brezillon wrote:
> > On Tue, 12 May 2026 14:47:27 +0100
> > Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > > On Thu, May 07, 2026 at 01:53:56PM +0200, Boris Brezillon wrote:
> > > > On Thu, 7 May 2026 11:02:26 +0200
> > > > Marcin Ślusarz <marcin.slusarz@arm.com> wrote:
> > > >
> > > > > On Tue, May 05, 2026 at 06:15:23PM +0200, Boris Brezillon wrote:
> > > > > > > @@ -277,9 +286,21 @@ int panthor_device_init(struct panthor_device *ptdev)
> > > > > > >                     return ret;
> > > > > > >     }
> > > > > > >
> > > > > > > +   /* If a protected heap name is specified but not found, defer the probe until created */
> > > > > > > +   if (protected_heap_name && strlen(protected_heap_name)) {
> > > > > >
> > > > > > Do we really need this strlen() > 0? Won't dma_heap_find() fail is the
> > > > > > name is "" already?
> > > > >
> > > > > If dma_heap_find() will fail, then the whole probe with fail too.
> > > > > This check prevents that.
> > > >
> > > > Yeah, that's also a questionable design choice. I mean, we can
> > > > currently probe and boot the FW even though we never setup the
> > > > protected FW sections, so why should we defer the probe here? Can't we
> > > > just retry the next time a group with the protected bit is created and
> > > > fail if we can find a protected heap?
> > >
> > > The problem we have with the current firmware is that it does a number of setup steps at "boot"
> > > time only. One of the steps is preparing its internal structures for when it enters protected
> > > mode and it stores them in the buffer passed in at firmware loading. We cannot later run the
> > > process when we have a group with protected mode set.
> >
> > No, but we can force a full/slow reset and have that thing
> > re-initialized, can't we? I mean, that's basically what we do when a
> > fast reset fails: we re-initialize all the sections and reset again, at
> > which point the FW should start from a fresh state, and be able to
> > properly initialize the protected-related stuff if protected sections
> > are populated. Am I missing something?
>
> Right, we can do that. For some reason I keep associating the reset with the
> error handling and not with "normal" operations.
I kind of hope we end up with either

 - panthor knows the exact heap to use and fails with EPROBE_DEFER if
the heap is missing, or
 - panthor gets a dma-buf from userspace and does the full reset
   - userspace also needs to provide a dma-buf for each protected
group for the suspend buffer

than something in-between. The latter is more ad-hoc and basically
kicks the issue to the userspace.

For the former, expressing the relation in DT seems to be the best,
but only if possible :-). Otherwise, a kconfig option (instead of
module param) should be easier to work with.

Looking at the userspace implementation, can we also have an panthor
ioctl to return the heap to userspace? A dma-heap ioctl to query the
heap size is also lacking.


>
> Best regards,
> Liviu
>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
>


^ permalink raw reply

* Re: [PATCH] Documentation: KVM: Document guest-visible compatibility expectations
From: David Woodhouse @ 2026-05-13 18:26 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Marc Zyngier, Jonathan Corbet, Shuah Khan, kvm,
	Linux Doc Mailing List, Kernel Mailing List, Linux,
	Sean Christopherson, Jim Mattson, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Raghavendra Rao Ananta, Eric Auger, Kees Cook, Arnd Bergmann,
	Nathan Chancellor, linux-arm-kernel, kvmarm, linux-kselftest
In-Reply-To: <CABgObfaM-JtNn2MuYXaiadQnLfAhTEaoHAcTG9=J6LkMcQCJ3A@mail.gmail.com>

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

On Wed, 2026-05-13 at 18:24 +0200, Paolo Bonzini wrote:
> Il mer 13 mag 2026, 15:57 David Woodhouse <dwmw2@infradead.org> ha scritto:
> > > x86 doesn't do bug-for-bug compatibility, thankfully - we have quirks
> > > but only 11 of them, or about one per year since we started adding them.
> > >   We only add quirks, generally speaking, when 1) we change the way file
> > > descriptors are initialized, 2) guests in the wild were relying on it,
> > > or 3) it prevends restoring state saved from an old kernel.  Is there
> > > anything else?
> > > 
> > > https://lore.kernel.org/kvm/e03f092dfbb7d391a6bf2797ba01e122ba080bcd.camel@infradead.org/
> > > is an example of a bug that "no SW can make any reasonable use of".
> > 
> > I actually believe that the focus on ICEBP was triggered by some weird
> > gaming software's anti-DRM mechanism, and that it *did* affect actual
> > guests in the wild?
> > 
> > But yeah, *fixing* it should not have any adverse effects. That's the
> > key.
> 
> Yep, so "bug for bug" is not it.

Of course. I'm not discriminating between 'bugs' and 'features'. In
this context I only care about guest-visible behaviour changes,
whatever the reason.

What I said was:
> > > > Once a behaviour is present in a released version of Linux/KVM, we
> > > > can't just declare it "wrong" and unilaterally impose a change in
> > > > guest-visible behaviour on *running* guests as a side-effect of a
> > > > kernel upgrade.

And yes, you're technically right to challenge that phrasing of it. It
does need the additional caveat of "...unless we are sure that changing
it in either direction underneath running guests cannot cause
problems", as discussed. That's the key for the ICEBP thing.

> > 
> > > And besides, both miss the point of *configurability* which is the basis of
> > > it all.
> > 
> > Hm, configurability *is* the point, I thought.
> 
> Yes, and configurability goes way beyond bugs/quirks, which are to
> some extent a red herring. Configurability for example says that "KVM:
> arm64: vgic: Allow userspace to set IIDR revision 1" shouldn't be
> controversial at all.

Indeed it shouldn't. And yet here we are.

> > > So we have the third case, "restoring state saved from an old kernel".
> > > If this case arises, I do believe that Arm will have to deal with it and
> > > introduce quirks or KVM_GET/SET_REG hacks.  Maybe it hasn't happened
> > > yet, lucky you.
> > 
> > We literally have those mechanisms already.
> 
> I am not talking about guest-visible changes across save/restore here,
> but rather about round-trips through userspace. For example, see the
> effect of KVM_X2APIC_API_USE_32BIT_IDS on KVM_GET/SET_LAPIC: it
> couldn't be made the default, because userspace expects to take old
> data returned by KVM_GET_LAPIC and shove it into KVM_SET_LAPIC. Sucks
> but can't be avoided.

Yes, you're right. And I fully expect and trust x86 to get that right
and not break existing userspace in any way at all.

But honestly, the bar for Arm is so low right now that anything I
physically *can* work around in userspace, I'm prepared to tolerate.

If KVM/arm did the equivalent of just changing the KVM_[SG]ET_LAPIC
data without the KVM_X2APIC_API_USE_32BIT_IDS trick, I wouldn't even
bat an eyelid; I'd just accommodate it and move on.

> > See commit https://git.kernel.org/torvalds/c/49a1a2c70a7f which adds a
> > new guest-visible feature in revision 3, but allowed userspace to
> > restore the old behaviour by setting it to revision 2. All my patch above does, is make it possible to set it to revision 1 as
> > well. Because https://git.kernel.org/torvalds/c/d53c2c29ae0d previously
> > changed the behaviour and bumped the default to 2 *without* allowing
> > userspace to restore the prior behaviour, and we've been carrying a
> > *revert* of that patch.
> > 
> > Why would we *not* accept such a patch?
> 
> Agreed. Even ignoring your revert, there's no reason why any upgrade
> past 49a1a2c70a7f has to be from after d53c2c29ae0d.
> 
> > Marc seems terribly insistent that we SHOULD NOT
> > restore the behaviour that older KVM offered to guests, and we MUST
> > change it unconditionally underneath running guests, making these
> > registers writable on upgrade... and reverting them to read-only for
> > running guests on a rollback.
> > 
> > And there we do have a very different viewpoint.
> 
> That's the design decision I mentioned, of not starting the guest
> configuration from a clean slate. I believe it complicates things
> because you have to design from the beginning with the ability to
> rollback to old versions and to potentially detect conflicts
> introduced by the rollback. This is exactly why
> KVM_X86_QUIRK_STUFF_FEATURE_MSRS was introduced: "KVM's initialization
> of feature MSRs during vCPU creation results in a failed save/restore
> of PERF_CAPABILITIES. If userspace configures the VM to _not_ have a
> PMU, because KVM initializes the vCPU's PERF_CAPABILITIES, trying to
> save/restore the non-zero value will be rejected by the destination."
> (https://lkml.org/lkml/2024/8/2/1032)

No, I don't think this is like that. In that case, IIUC it was at least
*possible* for userspace to manually filter out capabilities and adjust
things. But it kind of sucked if we *made* userspace do that and broke
things for existing userspace, so of *course* x86 did better.

I'm not even *dreaming* about a world where KVM/arm meets that bar.

> For Arm, however, it may be too late to change it; if not, I'll
> happily watch you argue with Marc about it. 

I'm not even going to try. You're right that it's the better option,
and it most certainly *isn't* too late for Arm to choose to be a stable
and mature platform providing continuity to userspace like x86 does.

But we are *so* far from that right now; we're fighting even to have
the *possibility* for userspace to remain compatible — even if
userspace *is* updated to know everything that the latest kernel
changed underneath it.

> But even without that,
> this doc patch (and the idea that "Where a new kernel introduces a
> guest-visible change, it provides a mechanism for userspace to select
> the previous behaviour") should be uncontroversial.

Indeed. And again, if you really want then you can add the caveat
discussed above, "unless you're really sure it won't make *any*
difference to the zoo of possible guests running Linux, Windows,
FreeBSD, or any number of random home-grown or network appliance
operating systems".

Although I didn't think it really needed spelling out in the doc, just
as I didn't think it needed spelling out earlier today (although you
called my sentence nonsense purely because it lacked that obvious
caveat, AFAICT).


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: arm: rockchip: Add LCKFB Taishan Pi 3M
From: Rob Herring (Arm) @ 2026-05-13 19:21 UTC (permalink / raw)
  To: Liu Changjie
  Cc: devicetree, conor+dt, heiko, linux-arm-kernel, krzk+dt,
	linux-rockchip, linux-kernel
In-Reply-To: <MN0PR19MB6091C07C729B83E594722E84AC3C2@MN0PR19MB6091.namprd19.prod.outlook.com>


On Thu, 07 May 2026 11:07:52 +0800, Liu Changjie wrote:
> Add the compatible string for LCKFB Taishan Pi 3M, a board based on
> the Rockchip RK3576 SoC.
> 
> Signed-off-by: Liu Changjie <liucj1228@outlook.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>



^ permalink raw reply

* [PATCH v2 2/2] arm64: dts: Add Airoha AN7583 SoC and AN7583 Evaluation Board
From: Lorenzo Bianconi @ 2026-05-13 19:21 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felix Fietkau,
	John Crispin, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Christian Marangi, devicetree, linux-arm-kernel, linux-mediatek,
	Lorenzo Bianconi
In-Reply-To: <20260513-airoha-7583-v2-0-ee0d82b37ce7@kernel.org>

Introduce the Airoha AN7583 SoC's dtsi and the Airoha AN7583 Evaluation
Board's dts file.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 arch/arm64/boot/dts/airoha/Makefile       |   2 +-
 arch/arm64/boot/dts/airoha/an7583-evb.dts |  23 ++++++
 arch/arm64/boot/dts/airoha/an7583.dtsi    | 133 ++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/airoha/Makefile b/arch/arm64/boot/dts/airoha/Makefile
index ebea112ce1d7..6027978a35c2 100644
--- a/arch/arm64/boot/dts/airoha/Makefile
+++ b/arch/arm64/boot/dts/airoha/Makefile
@@ -1,2 +1,2 @@
 # SPDX-License-Identifier: GPL-2.0-only
-dtb-$(CONFIG_ARCH_AIROHA) += en7581-evb.dtb
+dtb-$(CONFIG_ARCH_AIROHA) += en7581-evb.dtb an7583-evb.dtb
diff --git a/arch/arm64/boot/dts/airoha/an7583-evb.dts b/arch/arm64/boot/dts/airoha/an7583-evb.dts
new file mode 100644
index 000000000000..fa260f6e41c1
--- /dev/null
+++ b/arch/arm64/boot/dts/airoha/an7583-evb.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/dts-v1/;
+
+#include "an7583.dtsi"
+
+/ {
+	model = "Airoha AN7583 Evaluation Board";
+	compatible = "airoha,an7583-evb", "airoha,an7583";
+
+	aliases {
+		serial0 = &uart1;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200 earlycon";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory@80000000 {
+		device_type = "memory";
+		reg = <0x0 0x80000000 0x0 0x20000000>;
+	};
+};
diff --git a/arch/arm64/boot/dts/airoha/an7583.dtsi b/arch/arm64/boot/dts/airoha/an7583.dtsi
new file mode 100644
index 000000000000..a82ed916e61d
--- /dev/null
+++ b/arch/arm64/boot/dts/airoha/an7583.dtsi
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		atf@80000000 {
+			no-map;
+			reg = <0x0 0x80000000 0x0 0x200000>;
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+
+				core1 {
+					cpu = <&cpu1>;
+				};
+			};
+		};
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0>;
+			enable-method = "psci";
+			next-level-cache = <&l2>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x1>;
+			enable-method = "psci";
+			next-level-cache = <&l2>;
+		};
+
+		l2: l2-cache {
+			compatible = "cache";
+			cache-size = <0x80000>;
+			cache-line-size = <64>;
+			cache-level = <2>;
+			cache-unified;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	clk25m: clock-25000000 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <25000000>;
+		clock-output-names = "clkxtal";
+	};
+
+	i2c_clock: clock-20000000 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <20000000>;
+		clock-output-names = "i2c_clock";
+	};
+
+	sys_hclk: clock-100000000 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <100000000>;
+		clock-output-names = "sys_hclk";
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		gic: interrupt-controller@9000000 {
+			compatible = "arm,gic-v3";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			reg = <0x0 0x09000000 0x0 0x20000>,
+			      <0x0 0x09080000 0x0 0x80000>,
+			      <0x0 0x09400000 0x0 0x2000>,
+			      <0x0 0x09500000 0x0 0x2000>,
+			      <0x0 0x09600000 0x0 0x20000>;
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		uart1: serial@1fbf0000 {
+			compatible = "ns16550a";
+			reg = <0x0 0x1fbf0000 0x0 0x30>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <1843200>;
+		};
+
+		watchdog@1fbf0100 {
+			compatible = "airoha,an7583-wdt", "airoha,en7581-wdt";
+			reg = <0x0 0x1fbf0100 0x0 0x38>;
+
+			clocks = <&sys_hclk>;
+			clock-names = "bus";
+		};
+	};
+};

-- 
2.54.0



^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: arm64: dts: airoha: Add an7583 entry
From: Lorenzo Bianconi @ 2026-05-13 19:21 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felix Fietkau,
	John Crispin, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Christian Marangi, devicetree, linux-arm-kernel, linux-mediatek,
	Lorenzo Bianconi
In-Reply-To: <20260513-airoha-7583-v2-0-ee0d82b37ce7@kernel.org>

Introduce Airoha AN7583 entry in Airoha dts binding.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/devicetree/bindings/arm/airoha.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/airoha.yaml b/Documentation/devicetree/bindings/arm/airoha.yaml
index 7c38c08dbf3f..6506c64af4f3 100644
--- a/Documentation/devicetree/bindings/arm/airoha.yaml
+++ b/Documentation/devicetree/bindings/arm/airoha.yaml
@@ -26,6 +26,10 @@ properties:
           - enum:
               - airoha,en7581-evb
           - const: airoha,en7581
+      - items:
+          - enum:
+              - airoha,an7583-evb
+          - const: airoha,an7583
 
 additionalProperties: true
 

-- 
2.54.0



^ permalink raw reply related

* [PATCH v2 0/2] Introduce Airoha AN7583 SoC and AN7583 Evaluation Board
From: Lorenzo Bianconi @ 2026-05-13 19:21 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felix Fietkau,
	John Crispin, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Christian Marangi, devicetree, linux-arm-kernel, linux-mediatek,
	Lorenzo Bianconi

Introduce the Airoha AN7583 SoC's dtsi and the Airoha AN7583 Evaluation
Board's dts file.

---
Changes in v2:
- Fix RAM size for AN7583 evb
- Remove unused properties in an7583.dtsi
- Use ns16550a as compatible string for an7583 uart
- Add airoha,an7583-wdt compatible string for an7583 watchdog
- Link to v1: https://lore.kernel.org/r/20260509-airoha-7583-v1-0-e5ad085b258e@kernel.org

---
Lorenzo Bianconi (2):
      dt-bindings: arm64: dts: airoha: Add an7583 entry
      arm64: dts: Add Airoha AN7583 SoC and AN7583 Evaluation Board

 Documentation/devicetree/bindings/arm/airoha.yaml |   4 +
 arch/arm64/boot/dts/airoha/Makefile               |   2 +-
 arch/arm64/boot/dts/airoha/an7583-evb.dts         |  23 ++++
 arch/arm64/boot/dts/airoha/an7583.dtsi            | 133 ++++++++++++++++++++++
 4 files changed, 161 insertions(+), 1 deletion(-)
---
base-commit: e6490a169f6d5f5bdea7a2e8a673890d43afadc0
change-id: 20260509-airoha-7583-145246084016

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>



^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: iio: adc: Add GPADC for Allwinner A523
From: Conor Dooley @ 2026-05-13 19:15 UTC (permalink / raw)
  To: Michal Piekos
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Maksim Kiselev, linux-iio,
	devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <20260513-sunxi-a523-gpadc-v2-1-d5efde151dac@mmpsystems.pl>

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

On Wed, May 13, 2026 at 06:59:42AM +0200, Michal Piekos wrote:
> Add support for the GPADC for the Allwinner A523. It differs from the
> D1/T113s/R329/T507 by having two clocks.
> 
> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>

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 v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Shah, Tanmay @ 2026-05-13 19:05 UTC (permalink / raw)
  To: Mathieu Poirier, tanmay.shah
  Cc: Arnaud POULIQUEN, Beleswar Prasad Padhi, Shenwei Wang,
	Andrew Lunn, Linus Walleij, Bartosz Golaszewski, Jonathan Corbet,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Frank Li, Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
	devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	dl-linux-imx, Bartosz Golaszewski
In-Reply-To: <CANLsYkz9-+1o4ek0f5jS=-G1nPpp7BkCmNE5cin1zRY_e6Me-A@mail.gmail.com>



On 5/13/2026 11:34 AM, Mathieu Poirier wrote:
> On Tue, 12 May 2026 at 11:20, Shah, Tanmay <tanmays@amd.com> wrote:
>>
>>
>>
>> On 5/12/2026 10:41 AM, Mathieu Poirier wrote:
>>> On Mon, May 11, 2026 at 04:35:46PM -0500, Shah, Tanmay wrote:
>>>>
>>>>
>>>> On 5/11/2026 12:58 PM, Mathieu Poirier wrote:
>>>>> On Mon, 11 May 2026 at 10:47, Shah, Tanmay <tanmays@amd.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 5/5/2026 10:52 AM, Shah, Tanmay wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 5/5/2026 4:28 AM, Arnaud POULIQUEN wrote:
>>>>>>>> Hi Tanmay,
>>>>>>>>
>>>>>>>> On 5/4/26 21:19, Shah, Tanmay wrote:
>>>>>>>>>
>>>>>>>>> Hello all,
>>>>>>>>>
>>>>>>>>> I have started reviewing this work as well.
>>>>>>>>> Thanks Shenwei for this work.
>>>>>>>>>
>>>>>>>>> I have gone through only the current revision, and would like to provide
>>>>>>>>> idea on how to achieve GPIO number multiplexing with the RPMsg protocol.
>>>>>>>>> Also, have some bindings related question.
>>>>>>>>>
>>>>>>>>> Please see below:
>>>>>>>>>
>>>>>>>>> On 4/30/2026 11:40 AM, Arnaud POULIQUEN wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 4/30/26 14:56, Beleswar Prasad Padhi wrote:
>>>>>>>>>>> Hello Arnaud,
>>>>>>>>>>>
>>>>>>>>>>> On 30/04/26 13:05, Arnaud POULIQUEN wrote:
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> On 4/29/26 21:20, Mathieu Poirier wrote:
>>>>>>>>>>>>> On Wed, 29 Apr 2026 at 12:07, Padhi, Beleswar <b-padhi@ti.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Mathieu,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 4/29/2026 11:03 PM, Mathieu Poirier wrote:
>>>>>>>>>>>>>>> On Wed, 29 Apr 2026 at 10:53, Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>> From: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>>>>>>>>>>>>>> Sent: Wednesday, April 29, 2026 10:42 AM
>>>>>>>>>>>>>>>>> To: Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>>>>>>>>>>> Cc: Andrew Lunn <andrew@lunn.ch>; Padhi, Beleswar <b-
>>>>>>>>>>>>>>>>> padhi@ti.com>; Linus
>>>>>>>>>>>>>>>>> Walleij <linusw@kernel.org>; Bartosz Golaszewski
>>>>>>>>>>>>>>>>> <brgl@kernel.org>; Jonathan
>>>>>>>>>>>>>>>>> Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
>>>>>>>>>>>>>>>>> Krzysztof Kozlowski
>>>>>>>>>>>>>>>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn
>>>>>>>>>>>>>>>>> Andersson
>>>>>>>>>>>>>>>>> <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
>>>>>>>>>>>>>>>>> <s.hauer@pengutronix.de>; Shuah Khan
>>>>>>>>>>>>>>>>> <skhan@linuxfoundation.org>; linux-
>>>>>>>>>>>>>>>>> gpio@vger.kernel.org; linux-doc@vger.kernel.org; linux-
>>>>>>>>>>>>>>>>> kernel@vger.kernel.org;
>>>>>>>>>>>>>>>>> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>>>>>>>>>>>>>>>>> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
>>>>>>>>>>>>>>>>> devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org;
>>>>>>>>>>>>>>>>> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-
>>>>>>>>>>>>>>>>> linux-imx <linux-
>>>>>>>>>>>>>>>>> imx@nxp.com>; Bartosz Golaszewski <brgl@bgdev.pl>
>>>>>>>>>>>>>>>>> Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic
>>>>>>>>>>>>>>>>> rpmsg GPIO driver
>>>>>>>>>>>>>>>>> On Tue, Apr 28, 2026 at 03:24:59PM +0000, Shenwei Wang wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>>>> From: Andrew Lunn <andrew@lunn.ch>
>>>>>>>>>>>>>>>>>>> Sent: Monday, April 27, 2026 3:49 PM
>>>>>>>>>>>>>>>>>>> To: Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>>>>>>>>>>>>> Cc: Padhi, Beleswar <b-padhi@ti.com>; Linus Walleij
>>>>>>>>>>>>>>>>>>> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
>>>>>>>>>>>>>>>>>>> Jonathan
>>>>>>>>>>>>>>>>>>> Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
>>>>>>>>>>>>>>>>>>> Krzysztof
>>>>>>>>>>>>>>>>>>> Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>>>>>>>>>>>>>>>>>>> <conor+dt@kernel.org>;
>>>>>>>>>>>>>>>>>>> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
>>>>>>>>>>>>>>>>>>> <mathieu.poirier@linaro.org>; Frank Li <frank.li@nxp.com>;
>>>>>>>>>>>>>>>>>>> Sascha
>>>>>>>>>>>>>>>>>>> Hauer <s.hauer@pengutronix.de>; Shuah Khan
>>>>>>>>>>>>>>>>>>> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
>>>>>>>>>>>>>>>>>>> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix
>>>>>>>>>>>>>>>>>>> Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>>>>>>>>>>>>>>>>>>> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
>>>>>>>>>>>>>>>>>>> devicetree@vger.kernel.org; linux- remoteproc@vger.kernel.org;
>>>>>>>>>>>>>>>>>>> imx@lists.linux.dev; linux-arm- kernel@lists.infradead.org;
>>>>>>>>>>>>>>>>>>> dl-linux-imx <linux-imx@nxp.com>; Bartosz Golaszewski
>>>>>>>>>>>>>>>>>>> <brgl@bgdev.pl>
>>>>>>>>>>>>>>>>>>> Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic
>>>>>>>>>>>>>>>>>>> rpmsg
>>>>>>>>>>>>>>>>>>> GPIO driver
>>>>>>>>>>>>>>>>>>>>> struct virtio_gpio_response {
>>>>>>>>>>>>>>>>>>>>>             __u8 status;
>>>>>>>>>>>>>>>>>>>>>             __u8 value;
>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>> It is the same message format. Please see the message
>>>>>>>>>>>>>>>>>>>> definition
>>>>>>>>>>>>>>>>>>> (GET_DIRECTION) below:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> +   +-----+-----+-----+-----+-----+----+
>>>>>>>>>>>>>>>>>>>> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
>>>>>>>>>>>>>>>>>>>> +   | 1   | 2   |port |line | err | dir|
>>>>>>>>>>>>>>>>>>>> +   +-----+-----+-----+-----+-----+----+
>>>>>>>>>>>>>>>>>>> Sorry, but i don't see how two u8 vs six u8 are the same
>>>>>>>>>>>>>>>>>>> message format.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Some changes to the message format are necessary.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Virtio uses two communication channels (virtqueues): one for
>>>>>>>>>>>>>>>>>> requests and
>>>>>>>>>>>>>>>>> replies, and a second one for events.
>>>>>>>>>>>>>>>>>> In contrast, rpmsg provides only a single communication
>>>>>>>>>>>>>>>>>> channel, so a
>>>>>>>>>>>>>>>>>> type field is required to distinguish between different kinds
>>>>>>>>>>>>>>>>>> of messages.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Since rpmsg replies and events share the same message format,
>>>>>>>>>>>>>>>>>> an additional
>>>>>>>>>>>>>>>>> line is introduced to handle both cases.
>>>>>>>>>>>>>>>>>> Finally, rpmsg supports multiple GPIO controllers, so a port
>>>>>>>>>>>>>>>>>> field is added to
>>>>>>>>>>>>>>>>> uniquely identify the target controller.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I have commented on this before - RPMSG is already providing
>>>>>>>>>>>>>>>>> multiplexing
>>>>>>>>>>>>>>>>> capability by way of endpoints.  There is no need for a port
>>>>>>>>>>>>>>>>> field.  One endpoint,
>>>>>>>>>>>>>>>>> one GPIO controller.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> You still need a way to let the remote side know which port the
>>>>>>>>>>>>>>>> endpoint maps to, either
>>>>>>>>>>>>>>>> by embedding the port information in the message (the current
>>>>>>>>>>>>>>>> way), or by sending it
>>>>>>>>>>>>>>>> separately.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> An endpoint is created with every namespace request.  There
>>>>>>>>>>>>>>> should be
>>>>>>>>>>>>>>> one namespace request for every GPIO controller, which yields a
>>>>>>>>>>>>>>> unique
>>>>>>>>>>>>>>> endpoint for each controller and eliminates the need for an extra
>>>>>>>>>>>>>>> field to identify them.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Right, but this can still be done by just having one namespace
>>>>>>>>>>>>>> request.
>>>>>>>>>>>>>> We can create new endpoints bound to an existing namespace/
>>>>>>>>>>>>>> channel by
>>>>>>>>>>>>>> invoking rpmsg_create_ept(). This is what I suggested here too:
>>>>>>>>>>>>>> https://lore.kernel.org/all/29485742-6e49-482e-
>>>>>>>>>>>>>> b73d-228295daaeec@ti.com/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will look at your suggestion (i.e link above) later this week or
>>>>>>>>>>>>> next week.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> My mental model looks like this for the complete picture:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. namespace/channel#1 = rpmsg-io
>>>>>>>>>>>>>>        a. ept1 -> gpio-controller@1
>>>>>>>>>>>>>>        b. ept2 -> gpio-controller@2
>>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If my understanding of what gpio-controller is right, than this won't
>>>>>>>>> work. We need one rpmsg channel per gpio-controller, and in most cases
>>>>>>>>> there will be only one GPIO-controller on the remote side. If there are
>>>>>>>>> multiple or multiple instances of same controller, than we need separate
>>>>>>>>> channel name for that controller just like we would have separate device
>>>>>>>>> on the Linux.
>>>>>>>>
>>>>>>>> As done in ehe rpmsg_tty driver it could be instantiated several times with
>>>>>>>> the same channel/service name. This would imply a specific rpmsg to
>>>>>>>> retreive
>>>>>>>> the gpio controller index from the remote side.
>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I've asked for one endpoint per GPIO controller since the very
>>>>>>>>>>>>> beginning.  I don't yet have a strong opinion on whether to use one
>>>>>>>>>>>>> namespace request per GPIO controller or a single request that spins
>>>>>>>>>>>>> off multiple endpoints.  I'll have to look at your link and
>>>>>>>>>>>>> reflect on
>>>>>>>>>>>>> that.  Regardless of how we proceed on that front, multiplexing needs
>>>>>>>>>>>>> to happen at the endpoint level rather than the packet level.
>>>>>>>>>>>>> This is
>>>>>>>>>>>>> the only way this work can move forward.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I would be more in favor of Mathieu’s proposal: “An endpoint is
>>>>>>>>>>>> created with every namespace request.”
>>>>>>>>>>>>
>>>>>>>>>>>> If the endpoint is created only on the Linux side, how do we match
>>>>>>>>>>>> the Linux endpoint address with the local port field on the remote
>>>>>>>>>>>> side?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Simply by sending a message to the remote containing the newly created
>>>>>>>>>>> endpoint and the port idx. Note that is this done just one time, after
>>>>>>>>>>> this
>>>>>>>>>>> Linux need not have the port field in the message everytime its sending
>>>>>>>>>>> a message.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> With a multi-namespace approach, the namespace could be rpmsg-io-
>>>>>>>>>>>> [addr], where [addr] corresponds to the GPIO controller address in
>>>>>>>>>>>> the DT. This would:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> You will face the same problem in this case also that you asked above:
>>>>>>>>>>> "how do we match the Linux endpoint address with the local port field
>>>>>>>>>>> on the remote side?"
>>>>>>>>>>
>>>>>>>>>> Sorry I probably introduced confusion here
>>>>>>>>>> my sentence should be;
>>>>>>>>>>   With a multi-namespace approach, the namespace could be rpmsg-io-
>>>>>>>>>> [port],
>>>>>>>>>>   where [port] corresponds to the GPIO controller port in the DT.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> For instance:
>>>>>>>>>>
>>>>>>>>>>        rpmsg {
>>>>>>>>>>          rpmsg-io {
>>>>>>>>>>            #address-cells = <1>;
>>>>>>>>>>            #size-cells = <0>;
>>>>>>>>>>
>>>>>>>>>>            gpio@25 {
>>>>>>>>>>              compatible = "rpmsg-gpio";
>>>>>>>>>>              reg = <25>;
>>>>>>>>>>              gpio-controller;
>>>>>>>>>>              #gpio-cells = <2>;
>>>>>>>>>>              #interrupt-cells = <2>;
>>>>>>>>>>              interrupt-controller;
>>>>>>>>>>            };
>>>>>>>>>>
>>>>>>>>>>            gpio@32 {
>>>>>>>>>>              compatible = "rpmsg-gpio";
>>>>>>>>>>              reg = <32>;
>>>>>>>>>>              gpio-controller;
>>>>>>>>>>              #gpio-cells = <2>;
>>>>>>>>>>              #interrupt-cells = <2>;
>>>>>>>>>>              interrupt-controller;
>>>>>>>>>>            };
>>>>>>>>>>          };
>>>>>>>>>>        };
>>>>>>>>>>
>>>>>>>>>>   rpmsg-io-25  would match with gpio@25
>>>>>>>>>>   rpmsg-io-32  would match with gpio@32
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The problem with this approach is, we will endup creating way too many
>>>>>>>>> RPMsg devices/channels. i.e. one channel per one GPIO. That limits how
>>>>>>>>> many GPIOs can be handled by remote from memory perspective. At
>>>>>>>>> somepoint we might just run-out of number ept & channels created by the
>>>>>>>>> remote. As of now, open-amp library supports 128 epts I think.
>>>>>>>>
>>>>>>>> Right, I proposed a solution in my previous answer to Beleswar who has
>>>>>>>> the same concern.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Because the endpoint that is created on a namespace request is also
>>>>>>>>>>> dynamic in nature. How will the remote know which endpoint addr
>>>>>>>>>>> Linux allocated for a namespace that it announced?
>>>>>>>>>>>
>>>>>>>>>>> As an example/PoC, I created a firmware example which announces
>>>>>>>>>>> 2 name services to Linux, one is the standard "rpmsg_chrdev" and
>>>>>>>>>>> the other is a TI specific name service "ti.ipc4.ping-pong". You can
>>>>>>>>>>> see it created 2 different addresses (0x400 and 0x401) for each of
>>>>>>>>>>> the name service request from the same firmware:
>>>>>>>>>>>
>>>>>>>>>>> root@j784s4-evm:~# dmesg | grep virtio0 | grep -i channel
>>>>>>>>>>> [    9.290275] virtio_rpmsg_bus virtio0: creating channel
>>>>>>>>>>> ti.ipc4.ping-pong addr 0xd
>>>>>>>>>>> [    9.311230] virtio_rpmsg_bus virtio0: creating channel rpmsg_chrdev
>>>>>>>>>>> addr 0xe
>>>>>>>>>>> [    9.496645] rpmsg_chrdev virtio0.rpmsg_chrdev.-1.14: DEBUG: Channel
>>>>>>>>>>> formed from src = 0x400 to dst = 0xe
>>>>>>>>>>> [    9.707255] rpmsg_client_sample virtio0.ti.ipc4.ping-pong.-1.13:
>>>>>>>>>>> new channel: 0x401 -> 0xd!
>>>>>>>>>>>
>>>>>>>>>>> So in this case, rpmsg-io-1 can have different ept addr than rpmsg-io-2
>>>>>>>>>>> Back to same problem. Simple solution is to reply to remote with the
>>>>>>>>>>> created ept addr and the index.
>>>>>>>>>>
>>>>>>>>>> That why I would like to suggest to use the name service field to
>>>>>>>>>> identify the port/controller, instead of the endpoint address.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> - match the RPMsg probe with the DT,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> We can probe from all controllers with a single name service
>>>>>>>>>>> announcement too.
>>>>>>>>>>>
>>>>>>>>>>>> - provide a simple mapping between the port and the endpoint on both
>>>>>>>>>>>> sides,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> We are trying to get rid of this mapping from Linux side to adapt
>>>>>>>>>>> the gpio-virtio design.
>>>>>>>>>>>
>>>>>>>>>>>> - allow multiple endpoints on the remote side,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> We can support this as well with single nameservice model.
>>>>>>>>>>> There is no limitation. Remote has to send a message with
>>>>>>>>>>> its newly created ept that's all.
>>>>>>>>>>>
>>>>>>>>>>>> - provide a simple discovery mechanism for remote capabilities.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> A single announcement: "rpmsg-io" is also discovery mechanism.
>>>>>>>>>>>
>>>>>>>>>>> Feel free to let me know if you have concerns with any of the
>>>>>>>>>>> suggestions!
>>>>>>>>>>
>>>>>>>>>> My only concern, whatever the solution, is that we find a smart
>>>>>>>>>> solution to associate the correct endpoint with the correct GPIO
>>>>>>>>>> port/controller defined in the DT.
>>>>>>>>>>
>>>>>>>>>> I may have misunderstood your solution. Could you please help me
>>>>>>>>>> understand your proposal by explaining how you would handle three
>>>>>>>>>> GPIO ports defined in the DT, considering that the endpoint
>>>>>>>>>> addresses on the Linux side can be random?
>>>>>>>>>> If I assume there is a unique endpoint on the remote side,
>>>>>>>>>> I do not understand how you can match, on the firmware side,
>>>>>>>>>> the Linux endpoint address to the GPIO port.
>>>>>>>>>>
>>>>>>>>>> Thanks and Regards,Arnaud
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Beleswar
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Arnaud
>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. namespace/channel#2 = rpmsg-i2c
>>>>>>>>>>>>>>        a. ept1 -> i2c@1
>>>>>>>>>>>>>>        b. ept2 -> i2c@2
>>>>>>>>>>>>>>        c. ept3 -> i2c@3
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> etc...
>>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Just want to clear-up few terms before I jump to the solution:
>>>>>>>>>
>>>>>>>>> **RPMsg channel/device**:
>>>>>>>>>    - These are devices announced by the remote processor, and created by
>>>>>>>>> linux. They are created at: /sys/bus/rpmsg/devices
>>>>>>>>>    - The channel format: <name>.<src ept>.<dst ept>
>>>>>>>>>
>>>>>>>>> **RPMsg endpoint**:
>>>>>>>>>    - Endpoint is differnt than channel. Single channel can have multiple
>>>>>>>>> endpoints, and represented in the linux with: /dev/rpmsg? devices.
>>>>>>>>>
>>>>>>>>> To create endpoint device, we have rpmsg_create_ept API, which takes
>>>>>>>>> channel information as input, which has src-ept, dst-ept.
>>>>>>>>>
>>>>>>>>> Following is proposed solution:
>>>>>>>>>
>>>>>>>>> 1) Assign RPMsg channel/device per rpmsg-gpio controller (Not per GPIO
>>>>>>>>> pin/port).
>>>>>>>>>    - In our case that would be, single rpmsg-io node. (That makes me
>>>>>>>>> question if bindings are correct or not).
>>>>>>>>>
>>>>>>>>> 2) Assign GPIO number as src ept.
>>>>>>>>>
>>>>>>>>> i.e. *rpmsg-io.<GPIO number>.<dst ept>*. Do not randomly assign src
>>>>>>>>> endpoint.
>>>>>>>>>
>>>>>>>>> Now, RPMSG channel by spec reserves first 1024 endpoints [1], so we can
>>>>>>>>> add 1024 offset to the GPIO number:
>>>>>>>>>
>>>>>>>>> so, when calling rpmsg_create_ept() API, we assing src_endpoint as:
>>>>>>>>> (GPIO_NUMBER + RPMSG_RESERVED_ADDRESSES)
>>>>>>>>>
>>>>>>>>> Now on the remote side, there is single channel and only single-endpoint
>>>>>>>>> is needed that is mapped to the rpmsg-io channel callback.
>>>>>>>>>
>>>>>>>>> That callback will receive all the payloads from the Linux, which will
>>>>>>>>> have src-ept i.e. (RPMSG_RESERVED_ADDRESSES + GPIO_NUMBER).
>>>>>>>>
>>>>>>>>
>>>>>>>> Interesting approach. I also tried to find a similar solution.
>>>>>>>>
>>>>>>>> The question here is: how can we guarantee continuous addresses? Given
>>>>>>>> the static and dynamic allocation of endpoint addresses that are
>>>>>>>> implemented, my conclusion was that it is not reliable enough.
>>>>>>>>
>>>>>>>> but perhaps I missed something...
>>>>>>>>
>>>>>>>>>
>>>>>>>>> It can retrieve GPIO_NUMBER easily, and convert to appropriate pin based
>>>>>>>>> on platform specific logic.
>>>>>>>>>
>>>>>>>>> This doesn't need PORT information at all. Also it makes sure that
>>>>>>>>> remote is using only single-endpoint so not much memory is used.
>>>>>>>>>
>>>>>>>>> *Example*:
>>>>>>>>> If only rpmsg-gpio channel is created by the remote side, than following
>>>>>>>>> is the representation of the devices when GPIO 25, 26, 27 is assigned to
>>>>>>>>> the rpmsg-io controller:
>>>>>>>>>
>>>>>>>>> Linux                                                      Remote
>>>>>>>>>
>>>>>>>>> rpmsg-channel: rpmsg-gpio.0x400.0x400
>>>>>>>>>
>>>>>>>>> /dev/rpmsg0 - GPIO25 ept (rpmsg-gpio.0x419.0x400)-|
>>>>>>>>>                                                    |
>>>>>>>>> /dev/rpmsg1 - GPIO26 ept (rpmsg-gpio.0x41a.0x400)-|-> rpmsg-gpio.*.0x400
>>>>>>>>>                                                    |
>>>>>>>>> /dev/rpmsg2 - GPIO27 ept (rpmsg-gpio.0x41b.0x400)-|  0x400 ept callback.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *On remote side*:
>>>>>>>>>
>>>>>>>>> ept_0x400_callback(..., int src_ept, ...,)
>>>>>>>>> {
>>>>>>>>>     int gpio_num = src_ept - RPMSG_RESERVED_ADDRESSES;
>>>>>>>>>     // platform specific logic to convert gpio num to proper pin,
>>>>>>>>>     // just like you would convert gpio num to pin on a linux gpio
>>>>>>>>> controller.
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> My question on the binding:
>>>>>>>>>
>>>>>>>>> Why each GPIO is represented with the separate node? I think rpmsg-gpio
>>>>>>>>> can be represented just any other GPIO controller? Please let me know if
>>>>>>>>> I am missing something. So rpmsg channel/rpmsg device is not created per
>>>>>>>>> GPIO, but per controller. GPIO number multiplexing should be done with
>>>>>>>>> rpmsg src ept, that removes the need of having each GPIO as a separate
>>>>>>>>> node.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> rpmsg_gpio: rpmsg-gpio@0 {
>>>>>>>>>         compatible = "rpmsg-gpio";
>>>>>>>>>         reg = <0>;
>>>>>>>>>         gpio-controller;
>>>>>>>>>         #gpio-cells = <2>;
>>>>>>>>>         #interrupt-cells = <2>;
>>>>>>>>>         interrupt-controller;
>>>>>>>>>     };
>>>>>>>>>
>>>>>>>>> Then in DT, use like regular GPIO, but with the rpmsg-gpio controller:
>>>>>>>>>
>>>>>>>>> rpmsg-gpios = <&rpmsg_gpio (GPIO NUM) (flags)>;
>>>>>>>>>
>>>>>>>>> If the intent to create separate gpio nodes was only for the channel
>>>>>>>>> creation, then it's not really needed.
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> https://github.com/torvalds/linux/
>>>>>>>>> blob/6d35786de28116ecf78797a62b84e6bf3c45aa5a/drivers/rpmsg/
>>>>>>>>> virtio_rpmsg_bus.c#L136
>>>>>>>>>
>>>>>>>>
>>>>>>>> It is already the case. bindings declare GPIO controllers, not directly
>>>>>>>> GPIOs in:
>>>>>>>>
>>>>>>>> [PATCH v13 2/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode
>>>>>>>> support
>>>>>>>>
>>>>>>>> The discussion is around having an unique RPmsg endpoint for all
>>>>>>>> GPIO controller or one RPmsg endpoint per GPIO controller.
>>>>>>>>
>>>>>>>
>>>>>>> Endpoint where remote side or linux side?
>>>>>>>
>>>>>>> If unique endpoint on remote side per gpio controller then it makes sense.
>>>>>>>
>>>>>>> Unique endpoint on linux side doesn't make sense. Instead, unique
>>>>>>> channel per gpio controller makes sense, and each channel will have
>>>>>>> multiple endpoints on linux side. As I replied to Beleswar on the other
>>>>>>> email, I will copy past my answer here too:
>>>>>>>
>>>>>>>
>>>>>>> To be more specific:
>>>>>>>
>>>>>>> Linux:                               remote:
>>>>>>>
>>>>>>> ch1: rpmsg-gpio.-1.1024 ->     gpio-controller@1024
>>>>>>>     - gpio-line ept1
>>>>>>>     - gpio-line ept2    ->     They all map to same callback_ept_1024.
>>>>>>>     - gpio-line ept3
>>>>>>>
>>>>>>> ch2: rpmsg-gpio.-1.1025 ->     gpio-controller@1025
>>>>>>>     - gpio-line ept1
>>>>>>>     - gpio-line ept2    ->     They all map to same callback_ept_1025.
>>>>>>>     - gpio-line ept3
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Mathieu,
>>>>>>
>>>>>> So upon more brain storming in this approach I found limitation:
>>>>>>
>>>>>> This approach won't work if host OS is any other OS but Linux. For
>>>>>> example, if the remote OS is zephyr/baremetal using open-amp, then Only
>>>>>> Linux <-> zephyr combination will work, and we won't be able to re-use
>>>>>> this approach for zephyr <-> zephyr use case. The concept of rpmsg
>>>>>> channel/device exist only in the linux kernel implementation. This
>>>>>> brings another question: Should the protocol we decide work on other use
>>>>>> cases as well? Or Linux must be the Host OS for this protocol ?
>>>>>>
>>>>>
>>>>> Linux and Zephyr are very distinct OS, each with their own subsystems
>>>>> and characteristics.  The design we choose here involves RPMSG and,
>>>>> inherently, Linux.  We can't make decisions based on what may
>>>>> potentially happen in Zephyr.
>>>>>
>>>>>>
>>>>>> I think your & Arnaud's proposed approach of single endpoint per
>>>>>> gpio-controller on both side makes more sense, as it will work
>>>>>> regardless of any OS on host or remote side.
>>>>>>
>>>>>
>>>>> Arnaud, Beleswar, Andrew and I are all advocating for one endpoint per
>>>>> GPIO controller.  The remaining issue it about the best way to work
>>>>> out source and destination addresses between Linux and the remote
>>>>> processor.  I'm running out of time for today but I'll return to this
>>>>> thread with a final analysis by the end of the week.
>>>>>
>>>>
>>>> Okay. Then that means multiple endpoints on Linux side can be considered.
>>>
>>> If there are multiple GPIO controllers then yes, there will be more than one
>>> endpoint.  At this time I do now want to condiser other bus architectures (i2c,
>>> spi, ...) to avoid muddying an already difficult conversation.
>>>
>>>>
>>>> If we decide to go single-endpoint per device on both side, then for
>>>> that here is the proposal to represent src ept and dst ept:
>>>
>>> I do not understand what you mean by "per device" - please be more specific.
>>>
>>
>> "per device" I mean, per rpmsg device/channel. In our case that would be
>> per gpio-controller.
>>
>>>>
>>>> When we represent any device under rpmsg bus node, I think it should be
>>>> considered remote's view of the adddress space. So ideally we can
>>>> convert it to Linux view of the address space, via 'ranges' property.
>>>
>>> There is no address space to consider since there is no GPIO controller memory
>>> space to access.  All that is done by the driver (remote processor) and
>>> completely hidden from Linux by rpmsg-virtio-gpio.
>>>
>>
>> So IMHO the dt-binding is the representation of the device hardware and
>> is independent of how driver will access it. Any gpio-controller device
>> node, we are just representing how gpio-controller hardware on the
>> remote side looks like, and what is the corresponding view of the linux is.
>>
>> The rpmsg-gpio driver is different than the platform gpio controller
>> driver mainly in two ways:
>>
>> 1) How the driver is probed: rpmsg-gpio driver will be probed when
>> corresponding rpmsg channel/device name-service announcment will happen
>> from the remote side.
>>
> 
> I agree.
> 
>> 2) The GPIO Ops are not performed on the hardware directly, but it's
>> done via rpmsg commands on the remote side.
>>
> 
> I agree.
> 
>> However, the GPIO controller hardware remains the same. So bindings
>> shoudln't change.
>>
> 
> That is where I have a different point of view.  There is no need to
> have information in the bindings the kernel won't use.  We are
> advertizing virtio-gpio devices and as such should use virtio-gpio
> bindings.  The only thing that changes is the transport method, i.e,
> encapsulated in RPMSG rather than directly over virtqueues.
> 

I do not have deep knowledge of virtio-gpio devices, but in the bindings
example "virtio-gpio.yaml", there is 'reg' property available for
virtio-mmio transport:
https://github.com/torvalds/linux/blob/1f63dd8ca0dc05a8272bb8155f643c691d29bb11/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml#L47


Also, I am actually asking to use 'reg' property to retrieve endpoint
information in the rpmsg-gpio driver. please see below:


>> IMHO That means, if I want to move any existing GPIO-controller to the
>> remote side, and want the rpmsg-gpio driver to handle it then, all I
>> need to change is the compatible string of the current gpio-controller
>> device node. The rest of the address space should remain the same, and
>> leave ranges property empty. If the remote core has different view of
>> the address space, then the device should contain remote's view and
>> parent bus (rpmsg-io bus) should provide linux view via 'ranges' property.
>>
>> That is just the device hw representation in the device-tree as rpmsg
>> device. Same for any other type of the controller: i2c, spi etc.
>>
>> Thanks,
>> Tanmay
>>
>>
>>>>
>>>> So bindings should include 'ranges' property in the parent node. Then
>>>> linux view of the start address becomes src ept, and remote view of the
>>>> start address becomes dest ept. The remote view of the start address is
>>>> expected to be the static src endpoint on the remote side.
>>>>
>>>> Following representation of the rpmsg devices (gpio, i2c, spi or any other):
>>>>
>>>> rpmsg {
>>>>   #address-cells = <1>;
>>>>   #size-cells = <1>;
>>>>
>>>>   rpmsg-io {
>>>>     compatible = "rpmsg-io-bus";
>>>>     ranges = <remote_view_addr(dst ept) linux_view_addr(src ept) size>;
>>>>     #address-cells = <1>;
>>>>     #size-cells = <1>;
>>>>
>>>>     gpio@remote_view_addr(or dst ept) {
>>>>       compatible = "rpmsg-io";
>>>>       reg = <remote_view_addr addr_space_size>;
>>>>       gpio-controller;
>>>>       #gpio-cells = <2>;
>>>>       interrupt-controller;
>>>>       #interrupt-cells = <2>;
>>>>     };
>>>>

If we have 'reg' property as explained in above example, then we can use
remote view of the start address of the device as dest-endpoint, and
linux view of the start address of device (gpio-controller in our case)
as src ept.

We use 'reg' property like this:

/* Get remote view of the start addr of gpio-controller */
of_property_read_reg(dev_node, 0, &dest_ept, &size);

/* Get linux view of the start addr of gpio-controller */
of_address_to_resource(dev_node, 0, &res);
src_ept = res.start;

When sending rpmsg command we can use above endpoint informations
rpmsg_sendto() API.

Note that remote has already done name service announcement using
dst-ept (i.e. remote's view of the start address of the gpio-controller)
by this time.

Thanks,
Tanmay

>>>>     ...
>>>>
>>>>   };
>>>>
>>>> };
>>>>
>>>> Example device-tree:
>>>>
>>>> rpmsg {
>>>>   #address-cells = <1>;
>>>>   #size-cells = <1>;
>>>>
>>>>   rpmsg-io {
>>>>     compatible = "rpmsg-io-bus";
>>>>     ranges = <0x10000 0x50000 0x1000>,
>>>>              <0x20000 0x60000 0x1000>;
>>>>     #address-cells = <1>;
>>>>     #size-cells = <1>;
>>>>
>>>>     gpio@10000 {
>>>>       compatible = "rpmsg-io";
>>>>       reg = <0x10000 0x1000>;
>>>>       gpio-controller;
>>>>       #gpio-cells = <2>;
>>>>       interrupt-controller;
>>>>       #interrupt-cells = <2>;
>>>>     };
>>>>
>>>>     gpio@20000 {
>>>>       compatible = "rpmsg-io";
>>>>       reg = <0x20000 0x1000>;
>>>>       gpio-controller;
>>>>       #gpio-cells = <2>;
>>>>       interrupt-controller;
>>>>       #interrupt-cells = <2>;
>>>>     };
>>>>
>>>>   };
>>>>
>>>> };
>>>>
>>>>
>>>> Thanks,
>>>> Tanmay
>>>>
>>>>
>>>>>> To be more specific this will look like following:
>>>>>>
>>>>>> Host (Linux)                       Remote (baremetal/RTOS)
>>>>>>
>>>>>> rpmsg ch/device 1:
>>>>>>     - rpmsg ept 1   <------>     rpmsg ept 1 gpio-controller 0
>>>>>>
>>>>>> rpmsg ch/device 2:
>>>>>>      - rpmsg ept 2   <------>     rpmsg ept 2 gpio-controller 1
>>>>>>
>>>>>>
>>>>>> The question is, how to decide src ept, and dest ept on both sides?
>>>>>> I still think it should be static endpoints.
>>>>>>
>>>>>> I will get back with more reasoning on that.
>>>>>>
>>>>>>> On the remote side, we have to hardcode Which rpmsg controller is mapped
>>>>>>> to which endpoint.
>>>>>>>
>>>>>>>> Or did I misunderstand your questions?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Arnaud
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I gave this patch more time yesterday, and I think the 'reg' property
>>>>>>> should represent remote endpoint, instead of the gpio-controller index.
>>>>>>>
>>>>>>> So in this approach remote implementation is expected to provide
>>>>>>> hard-coded (static) endpoints for each gpio-controller instance, and
>>>>>>> that same number should be represented with the 'reg' property.
>>>>>>>
>>>>>>> On remote side:
>>>>>>>
>>>>>>> #define RPMSG_GPIO_0_CONTROLLER_EPT (RPMSG_RESERVED_ADDRESSES + 1) // 1024
>>>>>>>
>>>>>>> ept_1024_callback() {
>>>>>>>
>>>>>>>       // handle appropriate gpio port ()
>>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> On linux side:
>>>>>>>
>>>>>>> So new representation of controller:
>>>>>>>
>>>>>>>  rpmsg_gpio_0:   gpio@1024 {
>>>>>>>              compatible = "rpmsg-gpio";
>>>>>>>              reg = <1024>;
>>>>>>>              gpio-controller;
>>>>>>>              #gpio-cells = <2>;
>>>>>>>              #interrupt-cells = <2>;
>>>>>>>              interrupt-controller;
>>>>>>>           };
>>>>>>>
>>>>>>>  rpmsg_gpio_1:   gpio@1025 {
>>>>>>>              compatible = "rpmsg-gpio";
>>>>>>>              reg = <1025>;
>>>>>>>              gpio-controller;
>>>>>>>              #gpio-cells = <2>;
>>>>>>>              #interrupt-cells = <2>;
>>>>>>>              interrupt-controller;
>>>>>>>           };
>>>>>>>
>>>>>>> gpios = <&rpmsg_gpio_0 (GPIO NUM or PIN) flags>,
>>>>>>>       <&rpmsg_gpio_1 (GPIO NUM or PIN) flags>;
>>>>>>>
>>>>>>> Now in the linux driver:
>>>>>>>
>>>>>>> You can easily retrieve destination endpoint when we want to send the
>>>>>>> command to the gpio controller via device's "reg" property.
>>>>>>>
>>>>>>> This approach also provides built-in security as well. Because now
>>>>>>> gpio-controller instance is hardcoded with the endpoint callback, it
>>>>>>> can't be modified/addressed without changing the 'reg' property.
>>>>>>>
>>>>>>> Just like you wouldn't change device address for the instance of the
>>>>>>> gpio-controller right?
>>>>>>>
>>>>>>> This approach can be easily adapted to all the other rpmsg controllers
>>>>>>> as well.
>>>>>>>
>>>>>>> So, dynamic endpoint allocation doesn't make sense in this case. Dynamic
>>>>>>> endpoint allocation makes more sense for user-space apps which don't
>>>>>>> really care about endpoints and only payloads.
>>>>>>>
>>>>>>> But, here we are multiplexing device-addresses with endpoints, and so it
>>>>>>> has to be fixed, and presented via 'reg' property. So, firmware can't
>>>>>>> change device-address without Linux knowing it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Tanmay
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>>>>>>> This way device groups are isolated with each channel/namespace, and
>>>>>>>>>>>>>> instances within each device groups are also respected with specific
>>>>>>>>>>>>>> endpoints.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Beleswar
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>



^ permalink raw reply

* Re: [PATCH v2] media: dt-bindings: mediatek: Constrain iommus
From: Rob Herring @ 2026-05-13 19:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chun-Kuang Hu, Philipp Zabel, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, dri-devel, linux-mediatek, devicetree,
	linux-kernel, linux-arm-kernel, Conor Dooley
In-Reply-To: <20250821065900.17430-2-krzysztof.kozlowski@linaro.org>

On Thu, Aug 21, 2025 at 08:59:01AM +0200, Krzysztof Kozlowski wrote:
> Lists should have fixed constraints, because binding must be specific in
> respect to hardware.  Add missing constraints to number of iommus in
> Mediatek media devices and remove completely redundant and obvious
> description.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Changes in v2:
> 1. Ack
> 2. Mention dropping description
> ---
>  .../bindings/display/mediatek/mediatek,ovl-2l.yaml           | 5 ++---
>  .../devicetree/bindings/display/mediatek/mediatek,ovl.yaml   | 5 ++---
>  .../devicetree/bindings/display/mediatek/mediatek,rdma.yaml  | 4 +---
>  .../devicetree/bindings/display/mediatek/mediatek,wdma.yaml  | 4 +---
>  4 files changed, 6 insertions(+), 12 deletions(-)

Applied, thanks.

Rob


^ permalink raw reply

* Re: (subset) [PATCH V14 00/12] pci-imx6: Add support for parsing the reset property in new Root Port binding
From: Frank Li @ 2026-05-13 18:56 UTC (permalink / raw)
  To: Sherry Sun
  Cc: Manivannan Sadhasivam, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, bhelgaas@google.com, Hongxing Zhu,
	l.stach@pengutronix.de, imx@lists.linux.dev,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <VI0PR04MB121148EDDDA11C03C57EB4D3B92062@VI0PR04MB12114.eurprd04.prod.outlook.com>

On Wed, May 13, 2026 at 02:01:57AM +0000, Sherry Sun wrote:
> > Subject: Re: (subset) [PATCH V14 00/12] pci-imx6: Add support for parsing the
> > reset property in new Root Port binding
> >
> >
> > On Wed, 22 Apr 2026 17:35:37 +0800, Sherry Sun wrote:
> > > This patch set adds support for parsing the reset property in new Root
> > > Port binding in pci-imx6 driver, similar to the implementation in the
> > > qcom pcie driver[1].
> > >
> > > Also introduce generic helper functions to parse Root Port device tree
> > > nodes and extract common properties like reset GPIOs. This allows
> > > multiple PCI host controller drivers to share the same parsing logic.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [01/12] dt-bindings: PCI: fsl,imx6q-pcie: Add reset GPIO in Root Port node
> >         commit: 556c5f7aa06d3e8119d35f40b6602d3a547cef9e
> > [02/12] PCI: host-generic: Add common helpers for parsing Root Port
> > properties
> >         commit: 6cfd1e2fae1bbaa9258794ba9f3128afb752245c
> > [03/12] PCI: imx6: Assert PERST# before enabling regulators
> >         commit: 97eacf72d1754eeff8624114074929606fc3f2bb
> > [04/12] PCI: imx6: Add support for parsing the reset property in new Root
> > Port binding
> >         commit: 40b7f61a1a4d7fd18188f3f87e15ff5a90ce1d31
> >
>
> Hi Frank,
> Can you please help pick the remaining imx dts patches, thanks!

Thanks for reminder, please wait for few days to let it appeared linux-next.

Frank
>
> Best Regards
> Sherry


^ 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