Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables
From: Shah, Tanmay @ 2026-05-12 14:53 UTC (permalink / raw)
  To: Daniel Baluta, Ben Levinsky, Bjorn Andersson, Mathieu Poirier,
	linux-remoteproc
  Cc: Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Geert Uytterhoeven, Magnus Damm, Patrice Chotard, Maxime Coquelin,
	Alexandre Torgue, imx, linux-arm-kernel, linux-kernel,
	linux-renesas-soc, linux-stm32, tanmay.shah
In-Reply-To: <3b7f009c-dc4b-4fc0-becc-4d07eb4ff016@oss.nxp.com>



On 5/12/2026 2:55 AM, Daniel Baluta wrote:
> On 5/12/26 00:18, Ben Levinsky wrote:
>> [You don't often get email from ben.levinsky@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Add a small helper around rproc_elf_load_rsc_table() for remoteproc
>> drivers that treat a missing ELF resource table as optional. The helper
>> returns success on -EINVAL and propagates other failures unchanged.
>>
>> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
>> ---
>>  drivers/remoteproc/remoteproc_internal.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
>> index 3724a47a9748..dff87e468837 100644
>> --- a/drivers/remoteproc/remoteproc_internal.h
>> +++ b/drivers/remoteproc/remoteproc_internal.h
>> @@ -146,6 +146,18 @@ static inline int rproc_mem_entry_iounmap(struct rproc *rproc,
>>         return 0;
>>  }
>>
>> +static inline int rproc_elf_load_rsc_table_optional(struct rproc *rproc,
>> +                                                   const struct firmware *fw)
>> +{
>> +       int ret;
>> +
>> +       ret = rproc_elf_load_rsc_table(rproc, fw);
>> +       if (ret == -EINVAL)
>> +               dev_dbg(&rproc->dev, "no resource table found\n");
> 
> You are changing loglevel here. Initial drivers use dev_info or dev_warn. At least I'm used
> with seeing this messages in the logs. 
> 
> So, what do you think on adding at least dev_info to this instead of dev_dbg?
> 

Actually can we leave that choice to the platform driver ? There are
many use cases where the remoteproc subsystem is used to load and start
the remote core and the firmware doesn't have the resource table. We
don't want to make info level log for such use cases, as the resource
table is not expected in the first place there.

>> +
>> +       return ret == -EINVAL ? 0 : ret;
>> +}
>> +
>>  static inline int rproc_prepare_device(struct rproc *rproc)
>>  {
>>         if (rproc->ops->prepare)
>> --
>> 2.34.1
>>
>>
> 



^ permalink raw reply

* Re: [PATCH v5 12/29] media: rockchip: rga: avoid odd frame sizes for YUV formats
From: Sven Püschel @ 2026-05-12 14:56 UTC (permalink / raw)
  To: Nicolas Dufresne, Jacob Chen, Ezequiel Garcia,
	Mauro Carvalho Chehab, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Hans Verkuil
  Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
	devicetree, kernel, sebastian.reichel
In-Reply-To: <4f5e481c8883b358ee4cef64f26f3f00f0ac7304.camel@ndufresne.ca>

Hi Nicolas,

On 5/8/26 11:18 PM, Nicolas Dufresne wrote:
> Le mardi 28 avril 2026 à 11:00 +0200, Sven Püschel a écrit :
>> Avoid odd frame sizes for YUV formats, as they may cause undefined
>> behavior. This is done in preparation for the RGA3, which hangs when the
>> output format is set to 129x129 pixel YUV420 SP (NV12).
>>
>> This requirement is documented explicitly for the RGA3 in  section 5.6.3
>> of the RK3588 TRM Part 2. For the RGA2 the RK3588 TRM Part 2
>> (section 6.1.2) and RK3568 TRM Part 2 (section 14.2) only mentions the
>> x/y offsets and stride aligning requirements. But the vendor driver for
>> the RGA2 also contains checks for the width and height to be aligned to
>> 2 bytes.
>>
>> Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
>> ---
>>   drivers/media/platform/rockchip/rga/rga.c | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
>> index f599c992829dd..77b8c7ab74274 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.c
>> +++ b/drivers/media/platform/rockchip/rga/rga.c
>> @@ -337,6 +337,19 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
>>   	struct rga_ctx *ctx = file_to_rga_ctx(file);
>>   	const struct rga_hw *hw = ctx->rga->hw;
>>   	struct rga_fmt *fmt;
>> +	struct v4l2_frmsize_stepwise frmsize = {
>> +		.min_width = hw->min_width,
>> +		.max_width = hw->max_width,
>> +		.min_height = hw->min_height,
>> +		.max_height = hw->max_height,
>> +		.step_width = 1,
>> +		.step_height = 1,
>> +	};
>> +
>> +	if (v4l2_is_format_yuv(v4l2_format_info(pix_fmt->pixelformat))) {
>> +		frmsize.step_width = 2;
>> +		frmsize.step_height = 2;
> I think its fine like this, so let's start with:
>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> But it does not feel like a hardware alignment to me. When we process in
> software these things, the minimum alignment is bound to the subsampling, since
> there is no way to store half or quarter pixels, the padded width/height
> requires a step that follow the subsampling, something like:
>
> 		frmsize.step_width = finfo->hdiv;
> 		frmsize.step_height = finfo->vdiv;

I agree that this looks better. My main intention is to be more 
conservative, as the Rockchip related code/docs seem to always ensure a 
2 pixel alignment in both directions even for formats like YUV422, where 
we shouldn't have an alignment requirement in the height. Besides the 
vendor driver and TRM mentioned in the datasheet, the librga and it's 
docs also mention an alignment of 2 (pixels?!) for all YUV formats (see 
format alignment list in [1] and Q2.5 in [2]).

Given that the driver currently doesn't have any way to get the cores 
unstuck/reset in case of a hang, I'd like to play it more safely by 
adhering to what the vendor does instead of trying to do more and 
therefore allowing some potential breaking format. E.g. I've also 
experimented with sizes of 68x2, which the librga allows as input/output 
of the RGA3 (whereas the TRM specifies a min size of 128x128), but 
quickly dropped it as it produced some interesting broken outputs.

Sincerely
     Sven


[1] 
https://codeberg.org/airockchip/librga/src/branch/main/docs/Rockchip_Developer_Guide_RGA_EN.md#image-format-alignment-instructions

[2] 
https://codeberg.org/airockchip/librga/src/branch/main/docs/Rockchip_FAQ_RGA_EN.md

>
> Nicolas
>
>
>> +	}
>>   
>>   	if (V4L2_TYPE_IS_CAPTURE(f->type)) {
>>   		const struct rga_frame *frm;
>> @@ -358,11 +371,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
>>   	if (!fmt)
>>   		fmt = &hw->formats[0];
>>   
>> -	pix_fmt->width = clamp(pix_fmt->width,
>> -			       hw->min_width, hw->max_width);
>> -	pix_fmt->height = clamp(pix_fmt->height,
>> -				hw->min_height, hw->max_height);
>> -
>> +	v4l2_apply_frmsize_constraints(&pix_fmt->width, &pix_fmt->height, &frmsize);
>>   	v4l2_fill_pixfmt_mp(pix_fmt, fmt->fourcc, pix_fmt->width, pix_fmt->height);
>>   	pix_fmt->field = V4L2_FIELD_NONE;
>>   


^ permalink raw reply

* Re: [PATCH v2] phy: fsl-imx8mq-usb: add control register regmap
From: Frank Li @ 2026-05-12 14:59 UTC (permalink / raw)
  To: Xu Yang
  Cc: vkoul, neil.armstrong, s.hauer, kernel, festevam, jun.li,
	a.fatoum, franz.schnyder, stefano.radaelli21, linux-phy, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260512101212.1498223-1-xu.yang_2@nxp.com>

On Tue, May 12, 2026 at 06:12:12PM +0800, Xu Yang wrote:
> The CR port is a simple 16-bit data/address parallel port that is
> provided for on-chip access to the control registers inside the
> USB 3.0 femtoPHY. Add control register regmap and export these
> registers by debugfs to help PHY's diagnostic.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
>  - no changes

Do you means "resend" because not change in v2?

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a1..958d114b0c83 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0+
> -/* Copyright (c) 2017 NXP. */
> +/* Copyright 2017-2026 NXP. */
>
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
> @@ -9,6 +9,7 @@
>  #include <linux/of.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/usb/typec_mux.h>
>
> @@ -55,6 +56,8 @@
>  #define PHY_CTRL6_ALT_CLK_EN		BIT(1)
>  #define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
>
> +#define PHY_CRCTL			0x30
> +
>  #define PHY_TUNE_DEFAULT		0xffffffff
>
>  #define TCA_CLK_RST			0x00
> @@ -118,6 +121,7 @@ struct imx8mq_usb_phy {
>  	void __iomem *base;
>  	struct regulator *vbus;
>  	struct tca_blk *tca;
> +	struct regmap *cr_regmap;
>  	u32 pcs_tx_swing_full;
>  	u32 pcs_tx_deemph_3p5db;
>  	u32 tx_vref_tune;
> @@ -685,6 +689,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
>
> +static const struct regmap_config imx_cr_regmap_config = {
> +	.name = "cr",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0x7,
> +};
> +
>  static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
> @@ -713,6 +725,11 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
>
> +	imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
> +						   &imx_cr_regmap_config);
> +	if (IS_ERR(imx_phy->cr_regmap))
> +		return PTR_ERR(imx_phy->cr_regmap);
> +
>  	phy_ops = of_device_get_match_data(dev);
>  	if (!phy_ops)
>  		return -EINVAL;
> --
> 2.34.1
>


^ permalink raw reply

* Re: [PATCH v3 0/6] media: synopsys: Add imx93 support
From: Alexander Stein @ 2026-05-12 15:02 UTC (permalink / raw)
  To: Michael Riesch, Mauro Carvalho Chehab, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Frank Li
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	linux-rockchip, Guoniu Zhou, Laurent Pinchart, imx, Frank Li,
	Krzysztof Kozlowski
In-Reply-To: <20260216-imx93-dw-csi2-v3-0-aabafee10923@nxp.com>

Hi Frank,

Am Montag, 16. Februar 2026, 20:18:38 CEST schrieb Frank Li:
> This 3rd time try to support DW CSI2RX support for imx93.
> 
> 1st: Create new dw csi2 driver
> https://lore.kernel.org/all/20250701-95_cam-v1-5-c5172bab387b@nxp.com/
> 
> 2nd: Based on legacy imx6's DW CSI2 constroller.
> https://lore.kernel.org/imx/20250821-95_cam-v3-0-c9286fbb34b9@nxp.com/
> 
> Now rockchip create a common DW CSI2RX driver at
> drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> 
> This time base on rockchip's implement.
> 
> i.MX93's version is newer than rockchip one. i.MX6's is more similar with
> rockchips.
> 
> But i.MX6 is too old. So start at i.MX93 firstly even it has bigger
> difference.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

I'm trying to give this a try. Did you actually manage to run a CSI pipeline?
What did you do for the D-PHY driver? (please refer to csi2->phy = devm_phy_get(dev, NULL);)

Best regards,
Alexander

> ---
> Changes in v3:
> - use dw_mipi_csi2rx_has_reg().
> - detail change see each patches's change log
> - Link to v2: https://lore.kernel.org/r/20260213-imx93-dw-csi2-v2-0-8be6039f44c6@nxp.com
> 
> Changes in v2:
> - use enum and array map to real register offset.
> - Please keep the order:
>  - #includes
>  - #defines
>  - enum and struct definitions
>  - the rest
> - detail change see each patches's change log
> - Link to v1: https://lore.kernel.org/r/20260210-imx93-dw-csi2-v1-0-69667bb86bfa@nxp.com
> 
> ---
> Frank Li (6):
>       media: synopsys: csi2rx: use devm_reset_control_get_optional_exclusive()
>       media: synopsys: csi2rx: only check errors from devm_clk_bulk_get_all()
>       media: synopsys: csi2rx: implement .get_frame_desc() callback
>       media: synopsys: csi2rx: Use enum and u32 array for register offsets
>       media: dt-bindings: add NXP i.MX93 compatible string
>       media: synopsys: csi2rx: add i.MX93 support
> 
>  .../bindings/media/rockchip,rk3568-mipi-csi2.yaml  |  47 +++-
>  drivers/media/platform/synopsys/dw-mipi-csi2rx.c   | 278 +++++++++++++++++++--
>  2 files changed, 303 insertions(+), 22 deletions(-)
> ---
> base-commit: ada3fa02f7a95623b724dfe300fce6f49cc2d75a
> change-id: 20260128-imx93-dw-csi2-b472ddcb176a
> 
> Best regards,
> --
> Frank Li <Frank.Li@nxp.com>
> 
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/




^ permalink raw reply

* Re: [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
From: Frank Li @ 2026-05-12 15:03 UTC (permalink / raw)
  To: Xu Yang
  Cc: vkoul, neil.armstrong, s.hauer, kernel, festevam, jun.li,
	a.fatoum, franz.schnyder, stefano.radaelli21, linux-phy, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260512101046.1498096-1-xu.yang_2@nxp.com>

On Tue, May 12, 2026 at 06:10:46PM +0800, Xu Yang wrote:
> USB remote wakeup need its PHY power domain to be active,
> so set PHY to be wakeup capable.

The reason should usb phy support USB wakeup.

"Set PHY wakeup capable because this PHY supports remote wakeup function." ?

Frank

>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
>  - no changes
> ---
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 958d114b0c83..50b67f1e6a90 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -750,6 +750,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  					"failed to get tca\n");
>
>  	imx8m_get_phy_tuning_data(imx_phy);
> +	device_set_wakeup_capable(dev, true);
>
>  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> --
> 2.34.1
>


^ permalink raw reply

* Re: [PATCH v5 12/29] media: rockchip: rga: avoid odd frame sizes for YUV formats
From: Nicolas Dufresne @ 2026-05-12 15:09 UTC (permalink / raw)
  To: Sven Püschel, Jacob Chen, Ezequiel Garcia,
	Mauro Carvalho Chehab, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Hans Verkuil
  Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
	devicetree, kernel, sebastian.reichel
In-Reply-To: <e6d562e7-a9d2-4a1a-a139-c6e05eb4aaef@pengutronix.de>

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

Le mardi 12 mai 2026 à 16:56 +0200, Sven Püschel a écrit :
> Hi Nicolas,
> 
> On 5/8/26 11:18 PM, Nicolas Dufresne wrote:
> > Le mardi 28 avril 2026 à 11:00 +0200, Sven Püschel a écrit :
> > > Avoid odd frame sizes for YUV formats, as they may cause undefined
> > > behavior. This is done in preparation for the RGA3, which hangs when the
> > > output format is set to 129x129 pixel YUV420 SP (NV12).
> > > 
> > > This requirement is documented explicitly for the RGA3 in  section 5.6.3
> > > of the RK3588 TRM Part 2. For the RGA2 the RK3588 TRM Part 2
> > > (section 6.1.2) and RK3568 TRM Part 2 (section 14.2) only mentions the
> > > x/y offsets and stride aligning requirements. But the vendor driver for
> > > the RGA2 also contains checks for the width and height to be aligned to
> > > 2 bytes.
> > > 
> > > Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
> > > ---
> > >   drivers/media/platform/rockchip/rga/rga.c | 19 ++++++++++++++-----
> > >   1 file changed, 14 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> > > index f599c992829dd..77b8c7ab74274 100644
> > > --- a/drivers/media/platform/rockchip/rga/rga.c
> > > +++ b/drivers/media/platform/rockchip/rga/rga.c
> > > @@ -337,6 +337,19 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> > >   	struct rga_ctx *ctx = file_to_rga_ctx(file);
> > >   	const struct rga_hw *hw = ctx->rga->hw;
> > >   	struct rga_fmt *fmt;
> > > +	struct v4l2_frmsize_stepwise frmsize = {
> > > +		.min_width = hw->min_width,
> > > +		.max_width = hw->max_width,
> > > +		.min_height = hw->min_height,
> > > +		.max_height = hw->max_height,
> > > +		.step_width = 1,
> > > +		.step_height = 1,
> > > +	};
> > > +
> > > +	if (v4l2_is_format_yuv(v4l2_format_info(pix_fmt->pixelformat))) {
> > > +		frmsize.step_width = 2;
> > > +		frmsize.step_height = 2;
> > I think its fine like this, so let's start with:
> > 
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > But it does not feel like a hardware alignment to me. When we process in
> > software these things, the minimum alignment is bound to the subsampling, since
> > there is no way to store half or quarter pixels, the padded width/height
> > requires a step that follow the subsampling, something like:
> > 
> > 		frmsize.step_width = finfo->hdiv;
> > 		frmsize.step_height = finfo->vdiv;
> 
> I agree that this looks better. My main intention is to be more 
> conservative, as the Rockchip related code/docs seem to always ensure a 
> 2 pixel alignment in both directions even for formats like YUV422, where 
> we shouldn't have an alignment requirement in the height. Besides the 
> vendor driver and TRM mentioned in the datasheet, the librga and it's 
> docs also mention an alignment of 2 (pixels?!) for all YUV formats (see 
> format alignment list in [1] and Q2.5 in [2]).
> 
> Given that the driver currently doesn't have any way to get the cores 
> unstuck/reset in case of a hang, I'd like to play it more safely by 
> adhering to what the vendor does instead of trying to do more and 
> therefore allowing some potential breaking format. E.g. I've also 
> experimented with sizes of 68x2, which the librga allows as input/output 
> of the RGA3 (whereas the TRM specifies a min size of 128x128), but 
> quickly dropped it as it produced some interesting broken outputs.

Agreed, simply preserve my Rb in next version.

cheers,
Nicolas

> 
> Sincerely
>      Sven
> 
> 
> [1] 
> https://codeberg.org/airockchip/librga/src/branch/main/docs/Rockchip_Developer_Guide_RGA_EN.md#image-format-alignment-instructions
> 
> [2] 
> https://codeberg.org/airockchip/librga/src/branch/main/docs/Rockchip_FAQ_RGA_EN.md
> 
> > 
> > Nicolas
> > 
> > 
> > > +	}
> > >   
> > >   	if (V4L2_TYPE_IS_CAPTURE(f->type)) {
> > >   		const struct rga_frame *frm;
> > > @@ -358,11 +371,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> > >   	if (!fmt)
> > >   		fmt = &hw->formats[0];
> > >   
> > > -	pix_fmt->width = clamp(pix_fmt->width,
> > > -			       hw->min_width, hw->max_width);
> > > -	pix_fmt->height = clamp(pix_fmt->height,
> > > -				hw->min_height, hw->max_height);
> > > -
> > > +	v4l2_apply_frmsize_constraints(&pix_fmt->width, &pix_fmt->height, &frmsize);
> > >   	v4l2_fill_pixfmt_mp(pix_fmt, fmt->fourcc, pix_fmt->width, pix_fmt->height);
> > >   	pix_fmt->field = V4L2_FIELD_NONE;
> > >   

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH RFC] iommu: Enable per-device SSID space for SVA
From: Jason Gunthorpe @ 2026-05-12 15:11 UTC (permalink / raw)
  To: Joonwon Kang
  Cc: robin.murphy, Alexander.Grest, amhetre, baolu.lu,
	easwar.hariharan, iommu, jacob.jun.pan, joro, jpb, kees,
	kevin.tian, linux-arm-kernel, linux-kernel, nicolinc, praan,
	smostafa, will
In-Reply-To: <20260512145138.3414002-1-joonwonkang@google.com>

On Tue, May 12, 2026 at 02:51:38PM +0000, Joonwon Kang wrote:

> Appreciate all your clarifications here. So, my understanding is that if
> our system does not support ST64BV and ST64BV0 or if our device does not
> distinguish between the posted write and the non-posted write regarding
> PASID, then we can lift the use of the global PASID space. Can I say this?

You should do what Robin said - just have your driver use a per-device
PASID that it allocates and never use the global pasid allocator.

To do this lightly re-organize the SVA code so the driver can supply
its own PASID, and in this mode we wouldn't activate the ENQCMD
features in the mm.

Jason


^ permalink raw reply

* Re: [PATCH] clk: imx: Add audio PLL debugfs for K-divider control
From: Frank Li @ 2026-05-12 15:12 UTC (permalink / raw)
  To: Jacky Bai
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd,
	Brian Masney, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, linux-clk, imx, linux-arm-kernel
In-Reply-To: <20260512-imx8m_pll_debugfs-v1-1-e1e44b21be90@nxp.com>

On Tue, May 12, 2026 at 05:08:10PM +0800, Jacky Bai wrote:
> Add debugfs support for runtime tuning of the audio PLL K divider,
> which enables fine-grained frequency adjustments for audio PLL.
> This is used for:
>   - Audio clock calibration and testing
>   - Debugging audio synchronization issues
>
> Two debug interfaces are exported to userspace:
>   - delta_k: It is used to adjust the K divider in PLL based on small
>     steps
>   - pll_parameter: It is used for get PLL's current M-divider,
>     P-divider, S-divider & K-divider setting in PLL register
>
> examples for the usage of the two interfaces:
>    1): Get the current PLL setting of dividers
>      cat /sys/kernel/debug/audio_pll_monitor/audio_pll1/pll_parameter
>
>    2): if want to adjust the K-divider by a delta_k '1', then
>      echo 1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
>
>    3): if want to adjust the K-divider by a delta_k '-1', then
>      echo -1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;

Nit: needn't example, it is basic usage mode for debugfs.

>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  drivers/clk/imx/clk-imx8mm.c  |  6 ++++
>  drivers/clk/imx/clk-pll14xx.c | 79 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/imx/clk.h         |  1 +
>  3 files changed, 86 insertions(+)
>
> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> index 319af4deec01c188524807d39dff92bbd08f3601..6a031d7cd031c8b5f5b01e5531ce54360724ba44 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -298,6 +298,7 @@ static struct clk_hw **hws;
>
>  static int imx8mm_clocks_probe(struct platform_device *pdev)
>  {
> +	struct clk_hw *audio_pll_hws[2];
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
>  	void __iomem *base;
> @@ -610,6 +611,11 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
>
>  	imx_register_uart_clocks();
>
> +	/* Add debug interface for audio PLLs */
> +	audio_pll_hws[0] = hws[IMX8MM_AUDIO_PLL1];
> +	audio_pll_hws[1] = hws[IMX8MM_AUDIO_PLL2];
> +	audio_pll_debug_init(audio_pll_hws, ARRAY_SIZE(audio_pll_hws));
> +
>  	return 0;
>
>  unregister_hws:
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index 39600ee22be3066683b562aa6f2a8b750d19c4cc..59f126f35474116bdad0aeda59777b9eb7f246cf 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -8,6 +8,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/clk-provider.h>
> +#include <linux/debugfs.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
>  #include <linux/io.h>
> @@ -551,3 +552,81 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
>  	return hw;
>  }
>  EXPORT_SYMBOL_GPL(imx_dev_clk_hw_pll14xx);
> +
> +/*
> + * Debugfs interface for Audio PLL runtime monitoring and control
> + *
> + * This interface allows dynamic adjustment of the Audio PLL
> + * K-divider for precise frequency tuning, particularly useful
> + * for audio applications.
> + *
> + * examples for the usage of the two interfaces:
> + *   1): Get the current PLL setting of dividers
> + *     cat /sys/kernel/debug/audio_pll_monitor/audio_pll1/pll_parameter
> + *
> + *   2): if want to adjust the K-divider by a delta_k '1', then
> + *     echo 1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
> + *
> + *   3): if want to adjust the K-divider by a delta_k '-1', then
> + *     echo -1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
> + */

2) and 3) keep one is enough, you better descript validate range for
delta_k here, which is more valuable information.

> +#ifdef CONFIG_DEBUG_FS
> +static int pll_delta_k_set(void *data, u64 val)
> +{
> +	struct clk_pll14xx *pll = to_clk_pll14xx(data);
> +	s16 delta_k = (s16)val;
> +	u32 div_ctl1;
> +	s16 kdiv;
> +
> +	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> +	kdiv = FIELD_GET(KDIV_MASK, div_ctl1) + delta_k;
> +	writel_relaxed(FIELD_PREP(KDIV_MASK, kdiv), pll->base + DIV_CTL1);
> +
> +	return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE_SIGNED(delta_k_fops, NULL, pll_delta_k_set, "%lld\n");

generally, better also provide read for delta_k

cat /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k because user
want to check value by read it back naturally.

Frank
> +
> +static int pll_setting_show(struct seq_file *s, void *data)
> +{
> +	struct clk_pll14xx *pll = to_clk_pll14xx(s->private);
> +	u32 div_ctl0, div_ctl1;
> +	u32 mdiv, pdiv, sdiv, kdiv;
> +
> +	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> +	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> +
> +	mdiv = FIELD_GET(MDIV_MASK, div_ctl0);
> +	pdiv = FIELD_GET(PDIV_MASK, div_ctl0);
> +	sdiv = FIELD_GET(SDIV_MASK, div_ctl0);
> +	kdiv = FIELD_GET(KDIV_MASK, div_ctl1);
> +
> +	seq_printf(s, "Mdiv: 0x%x; Pdiv: 0x%x; Sdiv: 0x%x; Kdiv: 0x%x\n",
> +	mdiv, pdiv, sdiv, kdiv);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(pll_setting);
> +
> +void audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
> +{
> +	struct dentry *rootdir, *audio_pll_dir;
> +	const char *pll_name;
> +	int i;
> +
> +	rootdir = debugfs_create_dir("audio_pll_monitor", NULL);
> +
> +	for (i = 0; i < num_plls; i++) {
> +		pll_name = clk_hw_get_name(hws[i]);
> +		audio_pll_dir = debugfs_create_dir(pll_name, rootdir);
> +		debugfs_create_file_unsafe("delta_k", 0200, audio_pll_dir,
> +				hws[i], &delta_k_fops);
> +		debugfs_create_file("pll_parameter", 0444, audio_pll_dir,
> +				hws[i], &pll_setting_fops);
> +	}
> +}
> +#else /* !CONFIG_DEBUG_FS */
> +void audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +EXPORT_SYMBOL_GPL(audio_pll_debug_init);
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index aa5202f284f3d1b7c1b4bf65e2329831832b43a5..9611ab127dad6f23770c18e22e1acbe2fc22bd4e 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -487,4 +487,5 @@ struct clk_hw *imx_clk_gpr_mux(const char *name, const char *compatible,
>  			       u32 reg, const char **parent_names,
>  			       u8 num_parents, const u32 *mux_table, u32 mask);
>
> +void audio_pll_debug_init(struct clk_hw **hws, unsigned int num_plls);
>  #endif
>
> ---
> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> change-id: 20260421-imx8m_pll_debugfs-246d0fbbb617
>
> Best regards,
> --
> Jacky Bai <ping.bai@nxp.com>
>


^ permalink raw reply

* Re: [PATCH RESEND] clocksource: move NXP timer selection to drivers/clocksource
From: Frank Li @ 2026-05-12 15:17 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Russell King, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Daniel Lezcano, Thomas Gleixner, linux-arm-kernel,
	imx, linux-kernel, Enric Balletbo i Serra
In-Reply-To: <20260512-fix-nxp-timer-v1-1-565e13ef3e46@redhat.com>

On Tue, May 12, 2026 at 08:45:16AM +0200, Enric Balletbo i Serra wrote:
> From: Enric Balletbo i Serra <eballetb@redhat.com>
>
> The Kconfig logic for selecting the scheduler clocksource on
> NXP Vybrid (VF610) uses a `choice` block restricted to 32-bit ARM. This
> prevents 64-bit architectures, such as the NXP S32 family, from enabling
> the NXP Periodic Interrupt Timer (PIT) driver (CONFIG_NXP_PIT_TIMER).
>
> Relocate the NXP clocksource selection from arch/arm/mach-imx/Kconfig to
> drivers/clocksource/Kconfig. This allows the configuration to be shared
> across different architectures.
>
> Update the selection to include support for ARCH_S32 and add a "None"
> option to allow using the standard ARM Architected Timer. The Vybrid
> Global Timer option is now specifically restricted to 32-bit ARM SOC_VF610
> platforms.
>
> Fixes: bee33f22d7c3 ("clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support")

How it work when upstream clocksource/drivers/nxp-pit?

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
> ---
>  arch/arm/mach-imx/Kconfig   | 21 ---------------------
>  drivers/clocksource/Kconfig | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 6ea1bd55acf8..a361840d7a04 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -227,27 +227,6 @@ config SOC_VF610
>  	help
>  	  This enables support for Freescale Vybrid VF610 processor.
>
> -choice
> -	prompt "Clocksource for scheduler clock"
> -	depends on SOC_VF610
> -	default VF_USE_ARM_GLOBAL_TIMER
> -
> -	config VF_USE_ARM_GLOBAL_TIMER
> -		bool "Use ARM Global Timer"
> -		depends on ARCH_MULTI_V7
> -		select ARM_GLOBAL_TIMER
> -		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> -		help
> -		  Use the ARM Global Timer as clocksource
> -
> -	config VF_USE_PIT_TIMER
> -		bool "Use PIT timer"
> -		select NXP_PIT_TIMER
> -		help
> -		  Use SoC Periodic Interrupt Timer (PIT) as clocksource
> -
> -endchoice
> -
>  endif
>
>  endif
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index fd9112706545..b5c88ec65802 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -794,4 +794,35 @@ config RTK_SYSTIMER
>  	  this option only when building for a Realtek platform or for compilation
>  	  testing.
>
> +choice
> +	prompt "NXP clocksource for scheduler clock"
> +	depends on SOC_VF610 || ARCH_S32
> +	# Default to Global Timer for Vybrid (32-bit)
> +	default VF_USE_ARM_GLOBAL_TIMER if SOC_VF610
> +	# Default to None for S32 (64-bit)
> +	default VF_TIMER_NONE if ARCH_S32
> +
> +	config VF_USE_ARM_GLOBAL_TIMER
> +		bool "Use NXP Vybrid Global Timer"
> +		# This option is ONLY visible if we are on 32-bit ARM
> +		depends on ARM && SOC_VF610
> +		select ARM_GLOBAL_TIMER
> +		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> +		help
> +		  Use the NXP Vybrid Global Timer as clocksource.
> +
> +	config VF_USE_PIT_TIMER
> +		bool "Use NXP PIT timer"
> +		select NXP_PIT_TIMER
> +		help
> +		  Use NXP Periodic Interrupt Timer (PIT) as clocksource.
> +
> +	config VF_TIMER_NONE
> +		bool "None (Use standard Arch Timer)"
> +		help
> +		  Do not use any specific NXP timer driver. Use the standard
> +		  ARM Architected Timer instead.
> +
> +endchoice
> +
>  endmenu
>
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260302-fix-nxp-timer-9cb1fbd7afcd
>
> Best regards,
> --
> Enric Balletbo i Serra <eballetb@redhat.com>
>


^ permalink raw reply

* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Mathieu Poirier @ 2026-05-12 15:21 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: tanmay.shah, Arnaud POULIQUEN, Beleswar Prasad Padhi,
	Shenwei Wang, 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: <4ae35920-2539-4b12-8dea-efd407b8aaeb@lunn.ch>

On Mon, 11 May 2026 at 12:18, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > 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.
>
> How many of the participants here will be in Minneapolis next week for
> the Embedded Linux Conference? There is even a talk about this:
>
> https://osselcna2026.sched.com/event/2JQpx/building-virtual-drivers-with-rpmsg-key-design-principles-challenges-trade-offs-beleswar-prasad-padhi-texas-instruments?iframe=yes&w=100%&sidebar=yes&bg=no
>
> Maybe we can get together and decide on the final design after the
> session.
>

I will not be in Minneapolis next week.  At this point I think things
are converging into 2 main takeaways:

1) A serious refactoring of the protocol to include only what is
available in the virtio-gpio specification [1].
2) The specification of GPIO controller number in an extension of the
namespace announcement [2].

Shenwei proposed embedding the GPIO controller number in the
endpoint's source address [3], something I'm ambivalent about and
still have to look into.  I also have to read Tanmay's latest
comments.  I'm hoping to be done with all that by the end of the week.
With the above (1) and (2), a new patchset will be required to reset
this thread.

Thanks,
Mathieu

[1]. https://lwn.net/ml/all/afjyH5JT0JS2j0L5@p14s/
[2]. https://lwn.net/ml/all/afzIABSh1xtMEGbf%40p14s/
[3]. https://lwn.net/ml/all/PAXPR04MB9185BFA6E7375FAD0B15B021893C2@PAXPR04MB9185.eurprd04.prod.outlook.com/

>         Andrew


^ permalink raw reply

* Re: [PATCH v6 0/7] pinctrl: Add generic pinctrl for board-level mux chips
From: Frank Li @ 2026-05-12 15:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Peter Rosin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Rafał Miłecki, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, linux-kernel, linux-gpio, devicetree, imx,
	linux-arm-kernel, Haibo Chen, Conor Dooley, Ahmad Fatoum
In-Reply-To: <CAD++jL=eqEaTcG7P=++Md7fqosmrGbRRPDb90+=Q4q8oQ=xqRA@mail.gmail.com>

On Mon, May 11, 2026 at 10:34:35PM +0200, Linus Walleij wrote:
> On Mon, May 11, 2026 at 9:38 PM Frank Li <Frank.li@nxp.com> wrote:
>
> > I fixed a build warning by missing doc 'np',
> > https://lore.kernel.org/imx/20260507152117.240612-1-Frank.Li@nxp.com/
> >
> > Anything need me to do futher?
>
> Sorry was busy!
>
> Applied this fixup on top of the branch and merged it into my
> devel branch for v7.2!

Sorry, I missed picked up fix patch during rebase.

https://lore.kernel.org/imx/20260512144806.22447-1-Frank.Li@nxp.com/T/#u

Frank

>
> Yours,
> Linus Walleij


^ permalink raw reply

* Re: [v6 PATCH] arm64: mm: show direct mapping use in /proc/meminfo
From: Will Deacon @ 2026-05-12 15:22 UTC (permalink / raw)
  To: Yang Shi
  Cc: catalin.marinas, ryan.roberts, cl, linux-arm-kernel, linux-kernel
In-Reply-To: <20260316192933.3993042-1-yang@os.amperecomputing.com>

On Mon, Mar 16, 2026 at 12:29:33PM -0700, Yang Shi wrote:
> Since commit a166563e7ec3 ("arm64: mm: support large block mapping when
> rodata=full"), the direct mapping may be split on some machines instead
> keeping static since boot. It makes more sense to show the direct mapping
> use in /proc/meminfo than before.
> This patch will make /proc/meminfo show the direct mapping use like the
> below (4K base page size):
> DirectMap4K:       94792 kB
> DirectMap64K:     134208 kB
> DirectMap2M:     1173504 kB
> DirectMap32M:    5636096 kB
> DirectMap1G:    529530880 kB
> 
> Although just the machines which support BBML2_NOABORT can split the
> direct mapping, show it on all machines regardless of BBML2_NOABORT so
> that the users have consistent view in order to avoid confusion.
> 
> Although ptdump also can tell the direct map use, but it needs to dump
> the whole kernel page table. It is costly and overkilling. It is also
> in debugfs which may not be enabled by all distros. So showing direct
> map use in /proc/meminfo seems more convenient and has less overhead.
> 
> Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
> ---
> v6: * Rebased to v7.0-rc3
>     * Rebased on top of Anshuman's v5 "arm64/mm: Enable batched TLB flush
>       in unmap_hotplug_range()"
>     * Used const for direct map type array per Will
>     * Defined PUD size for 16K/64K even though it is not used per Will
>     * Removed the misleading comment in init_pmd() per Will
> v5: * Rebased to v6.19-rc4
>     * Fixed the build error for !CONFIG_PROC_FS
> v4: * Used PAGE_END instead of _PAGE_END(VA_BITS_MIN) per Ryan
>     * Used shorter name for the helpers and variables per Ryan
>     * Fixed accounting for memory hotunplug
> v3: * Fixed the over-accounting problems per Ryan
>     * Introduced helpers for add/sub direct map use and #ifdef them with
>       CONFIG_PROC_FS per Ryan
>     * v3 is a fix patch on top of v2
> v2: * Counted in size instead of the number of entries per Ryan
>     * Removed shift array per Ryan
>     * Use lower case "k" per Ryan
>     * Fixed a couple of build warnings reported by kernel test robot
>     * Fixed a couple of poential miscounts
> 
>  arch/arm64/mm/mmu.c | 197 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 176 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 5fb9a66f0754..7e95dbc69d57 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -29,6 +29,7 @@
>  #include <linux/mm_inline.h>
>  #include <linux/pagewalk.h>
>  #include <linux/stop_machine.h>
> +#include <linux/proc_fs.h>
>  
>  #include <asm/barrier.h>
>  #include <asm/cputype.h>
> @@ -171,6 +172,87 @@ static void init_clear_pgtable(void *table)
>  	dsb(ishst);
>  }
>  
> +enum dm_type {
> +	PTE,
> +	CONT_PTE,
> +	PMD,
> +	CONT_PMD,
> +	PUD,
> +	NR_DM_TYPE,
> +};
> +
> +#ifdef CONFIG_PROC_FS
> +static unsigned long dm_meminfo[NR_DM_TYPE];
> +
> +void arch_report_meminfo(struct seq_file *m)
> +{
> +	const char *size[NR_DM_TYPE];
> +
> +#if defined(CONFIG_ARM64_4K_PAGES)
> +	size[PTE] = "4k";
> +	size[CONT_PTE] = "64k";
> +	size[PMD] = "2M";
> +	size[CONT_PMD] = "32M";
> +	size[PUD] = "1G";
> +#elif defined(CONFIG_ARM64_16K_PAGES)
> +	size[PTE] = "16k";
> +	size[CONT_PTE] = "2M";
> +	size[PMD] = "32M";
> +	size[CONT_PMD] = "1G";
> +	size[PUD] = "64G";
> +#elif defined(CONFIG_ARM64_64K_PAGES)
> +	size[PTE] = "64k";
> +	size[CONT_PTE] = "2M";
> +	size[PMD] = "512M";
> +	size[CONT_PMD] = "16G";
> +	size[PUD] = "4T";
> +#endif
> +
> +	seq_printf(m, "DirectMap%s:	%8lu kB\n",
> +			size[PTE], dm_meminfo[PTE] >> 10);
> +	seq_printf(m, "DirectMap%s:	%8lu kB\n",
> +			size[CONT_PTE],
> +			dm_meminfo[CONT_PTE] >> 10);
> +	seq_printf(m, "DirectMap%s:	%8lu kB\n",
> +			size[PMD], dm_meminfo[PMD] >> 10);
> +	seq_printf(m, "DirectMap%s:	%8lu kB\n",
> +			size[CONT_PMD],
> +			dm_meminfo[CONT_PMD] >> 10);
> +	if (pud_sect_supported())
> +		seq_printf(m, "DirectMap%s:	%8lu kB\n",
> +			size[PUD], dm_meminfo[PUD] >> 10);
> +}
> +
> +static inline bool is_dm_addr(unsigned long addr)
> +{
> +	return (addr >= PAGE_OFFSET) && (addr < PAGE_END);
> +}

Just use __is_lm_address()?

For better or worse, the arm64 arch code tends to talk about the "linear
map" rather than the "direct map", so a little bit of renaming would be
good (i.e. s/dm/lm/). I'm fine if you want to keep the user-visible
strings as "DirectMap".

Will


^ permalink raw reply

* Re: [PATCH v4] coresight: fix missing error code when trace ID is invalid
From: Leo Yan @ 2026-05-12 15:24 UTC (permalink / raw)
  To: Jie Gan
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Tingwei Zhang, coresight, linux-arm-kernel, linux-kernel,
	Richard Cheng
In-Reply-To: <20260512-fix-trace-id-error-v4-1-eb3de789767a@oss.qualcomm.com>

On Tue, May 12, 2026 at 09:56:07AM +0800, Jie Gan wrote:
> When coresight_path_assign_trace_id() cannot assign a valid trace ID,
> coresight_enable_sysfs() takes the err_path goto with ret still 0,
> returning success to the caller despite no trace session being started.
> 
> Change coresight_path_assign_trace_id() to return int, moving the
> IS_VALID_CS_TRACE_ID() check inside it so it returns -EINVAL on failure
> and 0 on success. Update both callers to propagate this return value
> directly instead of inspecting path->trace_id after the call.
> 
> Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path")
> Reviewed-by: James Clark <james.clark@linaro.org>
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>

Reviewed-by: Leo Yan <leo.yan@arm.com>


^ permalink raw reply

* Re: [PATCH v3 0/6] media: synopsys: Add imx93 support
From: Frank Li @ 2026-05-12 15:35 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Michael Riesch, Mauro Carvalho Chehab, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-media,
	linux-kernel, devicetree, linux-arm-kernel, linux-rockchip,
	Guoniu Zhou, Laurent Pinchart, imx, Krzysztof Kozlowski
In-Reply-To: <5993213.DvuYhMxLoT@steina-w>

On Tue, May 12, 2026 at 05:02:42PM +0200, Alexander Stein wrote:
> Hi Frank,
>
> Am Montag, 16. Februar 2026, 20:18:38 CEST schrieb Frank Li:
> > This 3rd time try to support DW CSI2RX support for imx93.
> >
> > 1st: Create new dw csi2 driver
> > https://lore.kernel.org/all/20250701-95_cam-v1-5-c5172bab387b@nxp.com/
> >
> > 2nd: Based on legacy imx6's DW CSI2 constroller.
> > https://lore.kernel.org/imx/20250821-95_cam-v3-0-c9286fbb34b9@nxp.com/
> >
> > Now rockchip create a common DW CSI2RX driver at
> > drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> >
> > This time base on rockchip's implement.
> >
> > i.MX93's version is newer than rockchip one. i.MX6's is more similar with
> > rockchips.
> >
> > But i.MX6 is too old. So start at i.MX93 firstly even it has bigger
> > difference.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> I'm trying to give this a try. Did you actually manage to run a CSI pipeline?
> What did you do for the D-PHY driver? (please refer to csi2->phy = devm_phy_get(dev, NULL);)

Use old thread PHY driver
https://lore.kernel.org/imx/20250701-95_cam-v1-2-c5172bab387b@nxp.com/

Frank

>
> Best regards,
> Alexander
>
> > ---
> > Changes in v3:
> > - use dw_mipi_csi2rx_has_reg().
> > - detail change see each patches's change log
> > - Link to v2: https://lore.kernel.org/r/20260213-imx93-dw-csi2-v2-0-8be6039f44c6@nxp.com
> >
> > Changes in v2:
> > - use enum and array map to real register offset.
> > - Please keep the order:
> >  - #includes
> >  - #defines
> >  - enum and struct definitions
> >  - the rest
> > - detail change see each patches's change log
> > - Link to v1: https://lore.kernel.org/r/20260210-imx93-dw-csi2-v1-0-69667bb86bfa@nxp.com
> >
> > ---
> > Frank Li (6):
> >       media: synopsys: csi2rx: use devm_reset_control_get_optional_exclusive()
> >       media: synopsys: csi2rx: only check errors from devm_clk_bulk_get_all()
> >       media: synopsys: csi2rx: implement .get_frame_desc() callback
> >       media: synopsys: csi2rx: Use enum and u32 array for register offsets
> >       media: dt-bindings: add NXP i.MX93 compatible string
> >       media: synopsys: csi2rx: add i.MX93 support
> >
> >  .../bindings/media/rockchip,rk3568-mipi-csi2.yaml  |  47 +++-
> >  drivers/media/platform/synopsys/dw-mipi-csi2rx.c   | 278 +++++++++++++++++++--
> >  2 files changed, 303 insertions(+), 22 deletions(-)
> > ---
> > base-commit: ada3fa02f7a95623b724dfe300fce6f49cc2d75a
> > change-id: 20260128-imx93-dw-csi2-b472ddcb176a
> >
> > Best regards,
> > --
> > Frank Li <Frank.Li@nxp.com>
> >
> >
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>


^ permalink raw reply

* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Liviu Dudau @ 2026-05-12 15:38 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: 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: <20260512161111.0cb7000e@fedora>

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.

Best regards,
Liviu


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


^ permalink raw reply

* Re: [PATCH v3 1/3] iio: adc: xilinx-ams: fix out-of-bounds channel lookup in event handling
From: Guilherme Ivo Bozi @ 2026-05-12 15:40 UTC (permalink / raw)
  To: Salih Erim, Conall O'Griofa, Jonathan Cameron, Michal Simek
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-arm-kernel, linux-kernel
In-Reply-To: <46bcd2ce-bfb4-4fc3-b840-dba657f39a7f@amd.com>

Hi Salih,

Replies are inline.

On Tue, May 12, 2026 at 11:22 AM Salih Erim <salih.erim@amd.com> wrote:
>
> Hi Guilherme,
>
> Replies are inline.
>
> On 4/14/2026 11:40 PM, Guilherme Ivo Bozi wrote:
> > ams_event_to_channel() may return a pointer past the end of
> > dev->channels when no matching scan_index is found. This can lead
> > to invalid memory access in ams_handle_event().
> >
> > Add a bounds check in ams_event_to_channel() and return NULL when
> > no channel is found. Also guard the caller to safely handle this
> > case.
> >
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Guilherme Ivo Bozi <guilherme.bozi@usp.br>
> > ---
> >   drivers/iio/adc/xilinx-ams.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index 124470c92529..6191cd1b29a5 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -871,6 +871,9 @@ static const struct iio_chan_spec *ams_event_to_channel(struct iio_dev *dev,
> >                  if (dev->channels[i].scan_index == scan_index)
> >                          break;
> >
> > +       if (i == dev->num_channels)
> > +               return NULL;
> > +
> The added lines use spaces for indentation instead of tabs.
I checked both locally and the raw mbox from lore.kernel.org, and the
indentation uses TAB characters consistently (^I in the diff).

To verify, I inspected the relevant hunk using cat -A:

^I^Iif (dev->channels[i].scan_index == scan_index)
^I^I^Ibreak;

+^Iif (i == dev->num_channels)
+^I^Ireturn NULL;

^Ireturn &dev->channels[i];

I could not observe any indentation issues locally or from the raw mbox.

>
> Salih
>

--
Guilherme Ivo


^ permalink raw reply

* Re: [PATCH v4] coresight: fix missing error code when trace ID is invalid
From: Suzuki K Poulose @ 2026-05-12 15:40 UTC (permalink / raw)
  To: Mike Leach, James Clark, Leo Yan, Alexander Shishkin,
	Tingwei Zhang, Jie Gan
  Cc: Suzuki K Poulose, coresight, linux-arm-kernel, linux-kernel,
	Richard Cheng
In-Reply-To: <20260512-fix-trace-id-error-v4-1-eb3de789767a@oss.qualcomm.com>


On Tue, 12 May 2026 09:56:07 +0800, Jie Gan wrote:
> When coresight_path_assign_trace_id() cannot assign a valid trace ID,
> coresight_enable_sysfs() takes the err_path goto with ret still 0,
> returning success to the caller despite no trace session being started.
> 
> Change coresight_path_assign_trace_id() to return int, moving the
> IS_VALID_CS_TRACE_ID() check inside it so it returns -EINVAL on failure
> and 0 on success. Update both callers to propagate this return value
> directly instead of inspecting path->trace_id after the call.
> 
> [...]

Applied, thanks!

[1/1] coresight: fix missing error code when trace ID is invalid
      https://git.kernel.org/coresight/c/f4526ffee6ff

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>


^ permalink raw reply

* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Mathieu Poirier @ 2026-05-12 15:41 UTC (permalink / raw)
  To: 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: <ae5cfa0f-e929-4abb-b27b-8eb27586d183@amd.com>

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.

> 
> 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 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>;
>     };
> 
>     ...
> 
>   };
> 
> };
> 
> 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 v7 2/4] firmware: ti_sci: add support for restoring IRQs during resume
From: Thomas Richard @ 2026-05-12 15:44 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Tero Kristo, Santosh Shilimkar, Michael Turquette, Stephen Boyd,
	Gregory CLEMENT, richard.genoud, Udit Kumar, Abhash Kumar,
	Thomas Petazzoni, linux-arm-kernel, linux-kernel, linux-clk,
	Dhruva Gole, Kendall Willis
In-Reply-To: <20260506141508.2bko3ntcv7rkqfvn@certainty>

Hello Nishanth,

On 5/6/26 4:15 PM, Nishanth Menon wrote:
> On 10:35-20260506, Thomas Richard (TI) wrote:
>> Some DM-Firmware are not able to restore the IRQ context after a
>> suspend-resume. The IRQ_CONTEXT_LOST firmware capability has been
>> introduced to identify this characteristic. In this case the
>> responsibility is delegated to the ti_sci driver, which maintains an
>> internal list of all requested IRQs. This list is updated on each
>> set()/free() operation, and all IRQs are restored during the resume_noirq()
>> phase.
>>
>> Reviewed-by: Dhruva Gole <d-gole@ti.com>
>> Reviewed-by: Kendall Willis <k-willis@ti.com>
>> Signed-off-by: Thomas Richard (TI) <thomas.richard@bootlin.com>
>> ---
> 
> coccicheck reports two additional warnings at this patch:
> +drivers/firmware/ti_sci.c:2495:1-7: preceding lock on line 2454
> +drivers/firmware/ti_sci.c:2420:1-7: preceding lock on line 2378

I do not see the potential deadlock in set_irq() and free_irq().
I guess it is a false positive.

> 
>>  drivers/firmware/ti_sci.c | 201 +++++++++++++++++++++++++++++++++++++++++++---
>>  drivers/firmware/ti_sci.h |   2 +
>>  2 files changed, 191 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index eaeaaae94142..531ac3aa38b8 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -12,11 +12,13 @@
>>  #include <linux/cpu.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/export.h>
>> +#include <linux/hashtable.h>
>>  #include <linux/io.h>
>>  #include <linux/iopoll.h>
>>  #include <linux/kernel.h>
>>  #include <linux/mailbox_client.h>
>>  #include <linux/module.h>
>> +#include <linux/mutex.h>
>>  #include <linux/of.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/platform_device.h>
>> @@ -87,6 +89,16 @@ struct ti_sci_desc {
>>  	int max_msg_size;
>>  };
>>  
>> +/**
>> + * struct ti_sci_irq - Description of allocated irqs
>> + * @node: Link to hash table
>> + * @desc: Description of the irq
>> + */
>> +struct ti_sci_irq {
>> +	struct hlist_node node;
>> +	struct ti_sci_msg_req_manage_irq desc;
>> +};
>> +
>>  /**
>>   * struct ti_sci_info - Structure representing a TI SCI instance
>>   * @dev:	Device pointer
>> @@ -101,6 +113,8 @@ struct ti_sci_desc {
>>   * @chan_rx:	Receive mailbox channel
>>   * @minfo:	Message info
>>   * @node:	list head
>> + * @irqs:	List of allocated irqs
>> + * @irq_lock:	Protection for irq hash list
>>   * @host_id:	Host ID
>>   * @fw_caps:	FW/SoC low power capabilities
>>   * @users:	Number of users of this instance
>> @@ -118,6 +132,8 @@ struct ti_sci_info {
>>  	struct mbox_chan *chan_rx;
>>  	struct ti_sci_xfers_info minfo;
>>  	struct list_head node;
>> +	DECLARE_HASHTABLE(irqs, 8);
>> +	struct mutex irq_lock;
> 
> No action needed: checkpatch flags this as not documented, when it is
> clearly documented above.
> 
>>  	u8 host_id;
>>  	u64 fw_caps;
>>  	/* protected by ti_sci_list_mutex */
>> @@ -2301,6 +2317,32 @@ static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
>>  	return ret;
>>  }
>>  
>> +/**
>> + * ti_sci_irq_hash() - Helper API to compute irq hash for the hash table.
>> + * @irq:	irq to hash
>> + *
>> + * Return: the computed hash value.
>> + */
>> +static int ti_sci_irq_hash(struct ti_sci_msg_req_manage_irq *irq)
>> +{
>> +	return irq->src_id ^ irq->src_index;
>> +}
>> +
>> +/**
>> + * ti_sci_irq_equal() - Helper API to compare two irqs (generic headers are not
>> + *                       compared)
>> + * @irq_a:	irq_a to compare
>> + * @irq_b:	irq_b to compare
>> + *
>> + * Return: true if the two irqs are equal, else false.
>> + */
>> +static bool ti_sci_irq_equal(struct ti_sci_msg_req_manage_irq *irq_a,
>> +			     struct ti_sci_msg_req_manage_irq *irq_b)
>> +{
>> +	return !memcmp(&irq_a->valid_params, &irq_b->valid_params,
>> +		       sizeof(*irq_a) - sizeof(irq_a->hdr));
>> +}
>> +
>>  /**
>>   * ti_sci_set_irq() - Helper api to configure the irq route between the
>>   *		      requested source and destination
>> @@ -2324,15 +2366,60 @@ static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
>>  			  u16 dst_host_irq, u16 ia_id, u16 vint,
>>  			  u16 global_event, u8 vint_status_bit, u8 s_host)
>>  {
>> +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
>> +	struct ti_sci_msg_req_manage_irq *desc;
>> +	struct ti_sci_irq *irq;
>> +	int ret;
>> +
>> +	/*
>> +	 * Lock for set_irq() and free_irq() to keep the IRQ hash list
>> +	 * consistent only if MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST is set. Else
>> +	 * IRQ hash list is not used.
>> +	 */
>> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST)
>> +		mutex_lock(&info->irq_lock);
>> +
>>  	pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
>>  		 __func__, valid_params, src_id, src_index,
>>  		 dst_id, dst_host_irq, ia_id, vint, global_event,
>>  		 vint_status_bit);
>>  
>> -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
>> -				 dst_id, dst_host_irq, ia_id, vint,
>> -				 global_event, vint_status_bit, s_host,
>> -				 TI_SCI_MSG_SET_IRQ);
>> +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
>> +				dst_id, dst_host_irq, ia_id, vint,
>> +				global_event, vint_status_bit, s_host,
>> +				TI_SCI_MSG_SET_IRQ);
>> +
>> +	if (ret || !(info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST))
>> +		goto end;
>> +
>> +	irq = kzalloc_obj(*irq, GFP_KERNEL);
>> +	if (!irq) {
>> +		ti_sci_manage_irq(handle, valid_params, src_id, src_index,
>> +				  dst_id, dst_host_irq, ia_id, vint,
>> +				  global_event, vint_status_bit, s_host,
>> +				  TI_SCI_MSG_FREE_IRQ);
>> +		ret = -ENOMEM;
>> +		goto end;
>> +	}
>> +
>> +	desc = &irq->desc;
>> +	desc->valid_params = valid_params;
>> +	desc->src_id = src_id;
>> +	desc->src_index = src_index;
>> +	desc->dst_id = dst_id;
>> +	desc->dst_host_irq = dst_host_irq;
>> +	desc->ia_id = ia_id;
>> +	desc->vint = vint;
>> +	desc->global_event = global_event;
>> +	desc->vint_status_bit = vint_status_bit;
>> +	desc->secondary_host = s_host;
>> +
>> +	hash_add(info->irqs, &irq->node, ti_sci_irq_hash(desc));
>> +
>> +end:
>> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST)
>> +		mutex_unlock(&info->irq_lock);
>> +	return ret;
>>  }
>>  
>>  /**
>> @@ -2358,15 +2445,56 @@ static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
>>  			   u16 dst_host_irq, u16 ia_id, u16 vint,
>>  			   u16 global_event, u8 vint_status_bit, u8 s_host)
>>  {
>> +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
>> +	struct ti_sci_msg_req_manage_irq irq_desc;
>> +	struct device *dev = info->dev;
>> +	struct ti_sci_irq *this_irq;
>> +	struct hlist_node *tmp_node;
>> +	int ret;
>> +
>> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST)
>> +		mutex_lock(&info->irq_lock);
>> +
>>  	pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
>>  		 __func__, valid_params, src_id, src_index,
>>  		 dst_id, dst_host_irq, ia_id, vint, global_event,
>>  		 vint_status_bit);
>>  
>> -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
>> -				 dst_id, dst_host_irq, ia_id, vint,
>> -				 global_event, vint_status_bit, s_host,
>> -				 TI_SCI_MSG_FREE_IRQ);
>> +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
>> +				dst_id, dst_host_irq, ia_id, vint,
>> +				global_event, vint_status_bit, s_host,
>> +				TI_SCI_MSG_FREE_IRQ);
>> +
>> +	if (ret || !(info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST))
>> +		goto end;
>> +
>> +	irq_desc.valid_params = valid_params;
>> +	irq_desc.src_id = src_id;
>> +	irq_desc.src_index = src_index;
>> +	irq_desc.dst_id = dst_id;
>> +	irq_desc.dst_host_irq = dst_host_irq;
>> +	irq_desc.ia_id = ia_id;
>> +	irq_desc.vint = vint;
>> +	irq_desc.global_event = global_event;
>> +	irq_desc.vint_status_bit = vint_status_bit;
>> +	irq_desc.secondary_host = s_host;
>> +
>> +	hash_for_each_possible_safe(info->irqs, this_irq, tmp_node, node,
>> +				    ti_sci_irq_hash(&irq_desc)) {
>> +		if (ti_sci_irq_equal(&irq_desc, &this_irq->desc)) {
>> +			hlist_del(&this_irq->node);
>> +			kfree(this_irq);
>> +			goto end;
>> +		}
>> +	}
>> +
>> +	dev_warn(dev, "%s: should not be here, IRQ was not found in hash list\n",
>> +		 __func__);
>> +
>> +end:
>> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST)
>> +		mutex_unlock(&info->irq_lock);
>> +	return ret;
>>  }
>>  
>>  /**
>> @@ -3847,7 +3975,10 @@ static int ti_sci_suspend_noirq(struct device *dev)
>>  static int ti_sci_resume_noirq(struct device *dev)
>>  {
>>  	struct ti_sci_info *info = dev_get_drvdata(dev);
>> -	int ret = 0;
>> +	struct ti_sci_msg_req_manage_irq *irq_desc;
>> +	struct ti_sci_irq *irq;
>> +	struct hlist_node *tmp_node;
>> +	int ret = 0, err = 0, i;
>>  	u32 source;
>>  	u64 time;
>>  	u8 pin;
>> @@ -3859,13 +3990,50 @@ static int ti_sci_resume_noirq(struct device *dev)
>>  			return ret;
>>  	}
>>  
>> +	switch (pm_suspend_target_state) {
>> +	case PM_SUSPEND_MEM:
>> +		if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST) {
>> +			hash_for_each_safe(info->irqs, i, tmp_node, irq, node) {
>> +				irq_desc = &irq->desc;
>> +				ret = ti_sci_manage_irq(&info->handle,
>> +							irq_desc->valid_params,
>> +							irq_desc->src_id,
>> +							irq_desc->src_index,
>> +							irq_desc->dst_id,
>> +							irq_desc->dst_host_irq,
>> +							irq_desc->ia_id,
>> +							irq_desc->vint,
>> +							irq_desc->global_event,
>> +							irq_desc->vint_status_bit,
>> +							irq_desc->secondary_host,
>> +							TI_SCI_MSG_SET_IRQ);
>> +				if (ret) {
>> +					dev_err(dev, "failed to restore IRQ with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
> 
> space before via? overall this print is needed, but a bit umm.. we have
> similar ones already..

Yes I wanted to have consistent messages.
Will fix the missing space.

> 
>> +						irq_desc->valid_params,
>> +						irq_desc->src_id,
>> +						irq_desc->src_index,
>> +						irq_desc->dst_id,
>> +						irq_desc->dst_host_irq,
>> +						irq_desc->ia_id,
>> +						irq_desc->vint,
>> +						irq_desc->global_event,
>> +						irq_desc->vint_status_bit);
>> +					err = ret;
>> +				}
>> +			}
>> +		}
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>>  	ret = ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode);
>>  	/* Do not fail to resume on error as the wake reason is not critical */
>>  	if (!ret)
>>  		dev_info(dev, "ti_sci: wakeup source:0x%x, pin:0x%x, mode:0x%x\n",
>>  			 source, pin, mode);
>>  
>> -	return 0;
>> +	return err;
>>  }
>>  
>>  static void ti_sci_pm_complete(struct device *dev)
>> @@ -4014,13 +4182,14 @@ static int ti_sci_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
>> -	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s%s\n",
>> +	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s%s%s\n",
>>  		info->fw_caps & MSG_FLAG_CAPS_GENERIC ? "Generic" : "",
>>  		info->fw_caps & MSG_FLAG_CAPS_LPM_PARTIAL_IO ? " Partial-IO" : "",
>>  		info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED ? " DM-Managed" : "",
>>  		info->fw_caps & MSG_FLAG_CAPS_LPM_ABORT ? " LPM-Abort" : "",
>>  		info->fw_caps & MSG_FLAG_CAPS_IO_ISOLATION ? " IO-Isolation" : "",
>> -		info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED ? " BoardConfig-Managed" : ""
>> +		info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED ? " BoardConfig-Managed" : "",
>> +		info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST ? " IRQ-Context-Lost" : ""
>>  	);
>>  
>>  	ti_sci_setup_ops(info);
>> @@ -4053,6 +4222,14 @@ static int ti_sci_probe(struct platform_device *pdev)
>>  	list_add_tail(&info->node, &ti_sci_list);
>>  	mutex_unlock(&ti_sci_list_mutex);
>>  
>> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_IRQ_CONTEXT_LOST) {
>> +		ret = devm_mutex_init(dev, &info->irq_lock);
>> +		if (ret)
>> +			return ret;
> 
> If devm_mutex_init() fails, the function returns directly rather than
> going through the out: label. All other error paths after the mailbox
> channel allocations use goto out, which calls mbox_free_channel() for
> both channels. Since mbox_request_channel_byname() is not a devm
> allocation, those channels would not be freed by the devm cleanup path
> on probe failure.
> 
> Does this leak info->chan_tx and info->chan_rx when devm_mutex_init()
> returns an error?

Indeed we should go to 'out'.

Best Regards,
Thomas


^ permalink raw reply

* Re: [PATCH v3 3/3] iio: adc: xilinx-ams: refactor alarm mapping to table-driven approach
From: Guilherme Ivo Bozi @ 2026-05-12 15:46 UTC (permalink / raw)
  To: Salih Erim
  Cc: Conall O'Griofa, Jonathan Cameron, Michal Simek,
	David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-arm-kernel, linux-kernel
In-Reply-To: <e84d98dd-c81c-418d-8cbe-5867f8bdd4c3@amd.com>

Hi Salih,

On Tue, May 12, 2026 at 11:34 AM Salih Erim <salih.erim@amd.com> wrote:
>
> Hi,
>
> On 4/14/2026 11:40 PM, Guilherme Ivo Bozi wrote:
> > Replace multiple open-coded switch statements that map between
> > scan_index, alarm bits, and register offsets with a centralized
> > table-driven approach.
> >
> > Introduce a struct-based alarm_map to describe the relationship
> > between scan indices and alarm offsets, and add a helper to
> > translate scan_index to event IDs. This removes duplicated logic
> > across ams_get_alarm_offset(), ams_event_to_channel(), and
> > ams_get_alarm_mask().
> >
> > The new approach improves maintainability, reduces code size,
> > and makes it easier to extend or modify alarm mappings in the
> > future, while preserving existing behavior.
> >
> > Signed-off-by: Guilherme Ivo Bozi <guilherme.bozi@usp.br>
> > ---
> >   drivers/iio/adc/xilinx-ams.c | 161 +++++++++++++----------------------
> >   1 file changed, 58 insertions(+), 103 deletions(-)
>
> Two issues:
>
> 1) Same tabs-vs-spaces indentation problem as patches 1 and 2.
>
> 2) In ams_event_to_channel():
>
> > +       if (event < 0 || event >= ARRAY_SIZE(alarm_map))
> > +               return NULL;
>
> The 'event' parameter is u32, so 'event < 0' is always false.
> Either drop the < 0 check or change the parameter type to int
> if you intend to handle negative values.
You are correct regarding the u32 type -- the event < 0 check is
unnecessary. I will remove it in the next revision.

>
> Salih

--
Guilherme Ivo


^ permalink raw reply

* Re: [PATCH v17 24/24] KVM: selftests: Add guest_memfd testcase to fault-in on !mmap()'d memory
From: Sean Christopherson @ 2026-05-12 15:53 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: Zenghui Yu, Paolo Bonzini, Marc Zyngier, Oliver Upton, kvm,
	linux-arm-kernel, kvmarm, linux-kernel, Ira Weiny, Gavin Shan,
	Shivank Garg, Vlastimil Babka, Xiaoyao Li, David Hildenbrand,
	Fuad Tabba, Ackerley Tng, Tao Chan, James Houghton
In-Reply-To: <c17d864a-42fc-afbe-345e-c728e2735d0c@huawei.com>

On Tue, May 12, 2026, Zenghui Yu wrote:
> Hi Sean,
> 
> On 2026/4/18 0:47, Sean Christopherson wrote:
> > 
> > Does this fix things for you?  If so, I'll send a proper patch.
> > 
> > diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> > index ec7644aae999..c682bda722f9 100644
> > --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> > +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> > @@ -510,9 +510,15 @@ static void test_guest_memfd_guest(void)
> >                     "Default VM type should support INIT_SHARED, supported flags = 0x%x",
> >                     vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS));
> >  
> > +       /*
> > +        * Use the guest's page size for all accesses, e.g. to avoid having to
> > +        * map multiple pages, but create the guest_memfd instance with the max
> > +        * of the host or guest page size, as KVM requires gmem files to be
> > +        * aligned to the host page size.
> > +        */
> >         size = vm->page_size;
> > -       fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP |
> > -                                            GUEST_MEMFD_FLAG_INIT_SHARED);
> > +       fd = vm_create_guest_memfd(vm, max(size, page_size), GUEST_MEMFD_FLAG_MMAP |
> > +                                                            GUEST_MEMFD_FLAG_INIT_SHARED);
> >         vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
> >  
> >         mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> 
> This results in another failure at
> 
> |	vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size,
> NULL, fd, 0);

Gah, the memslot size also needs to be a multiple of the host page size.  I
actually tested on a CONFIG_ARM64_64K_PAGES=y kernel this time, patch incoming...

Thanks!


^ permalink raw reply

* Re: [PATCH v3 1/3] iio: adc: xilinx-ams: fix out-of-bounds channel lookup in event handling
From: Jonathan Cameron @ 2026-05-12 15:54 UTC (permalink / raw)
  To: Guilherme Ivo Bozi
  Cc: Salih Erim, Conall O'Griofa, Michal Simek, David Lechner,
	Nuno Sá, Andy Shevchenko, linux-iio, linux-arm-kernel,
	linux-kernel
In-Reply-To: <CAJgF-xum3OQ36keRW5gCnTTbnUhsHZUzL41xKn0hjJ3GqgyOZg@mail.gmail.com>

On Tue, 12 May 2026 12:40:05 -0300
Guilherme Ivo Bozi <guilherme.bozi@usp.br> wrote:

> Hi Salih,
> 
> Replies are inline.
> 
> On Tue, May 12, 2026 at 11:22 AM Salih Erim <salih.erim@amd.com> wrote:
> >
> > Hi Guilherme,
> >
> > Replies are inline.
> >
> > On 4/14/2026 11:40 PM, Guilherme Ivo Bozi wrote:  
> > > ams_event_to_channel() may return a pointer past the end of
> > > dev->channels when no matching scan_index is found. This can lead
> > > to invalid memory access in ams_handle_event().
> > >
> > > Add a bounds check in ams_event_to_channel() and return NULL when
> > > no channel is found. Also guard the caller to safely handle this
> > > case.
> > >
> > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > Signed-off-by: Guilherme Ivo Bozi <guilherme.bozi@usp.br>
> > > ---
> > >   drivers/iio/adc/xilinx-ams.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > index 124470c92529..6191cd1b29a5 100644
> > > --- a/drivers/iio/adc/xilinx-ams.c
> > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > @@ -871,6 +871,9 @@ static const struct iio_chan_spec *ams_event_to_channel(struct iio_dev *dev,
> > >                  if (dev->channels[i].scan_index == scan_index)
> > >                          break;
> > >
> > > +       if (i == dev->num_channels)
> > > +               return NULL;
> > > +  
> > The added lines use spaces for indentation instead of tabs.  
> I checked both locally and the raw mbox from lore.kernel.org, and the
> indentation uses TAB characters consistently (^I in the diff).
> 
> To verify, I inspected the relevant hunk using cat -A:
> 
> ^I^Iif (dev->channels[i].scan_index == scan_index)
> ^I^I^Ibreak;
> 
> +^Iif (i == dev->num_channels)
> +^I^Ireturn NULL;
> 
> ^Ireturn &dev->channels[i];
> 
> I could not observe any indentation issues locally or from the raw mbox.
FWIW they look good to me as well. Salih, I'd guess you have a local issue.
b4 (on git.kernel.org) is really handy for ensuring none of those occur!

Jonathan

> 
> >
> > Salih
> >  
> 
> --
> Guilherme Ivo



^ permalink raw reply

* Re: [PATCH 1/5] arm_mpam: Parse the rest of the ACPI table
From: Andre Przywara @ 2026-05-12 16:13 UTC (permalink / raw)
  To: Ben Horgan, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla,
	Catalin Marinas, Will Deacon, Rafael J . Wysocki, Len Brown,
	James Morse, Reinette Chatre, Fenghua Yu
  Cc: Jonathan Cameron, linux-acpi, linux-arm-kernel, linux-kernel
In-Reply-To: <f12b8d73-7c8e-43e7-b9ce-d80a7c3f10c8@arm.com>

Hi Ben,

On 5/8/26 12:21, Ben Horgan wrote:
> Hi Andre,
> 
> On 4/29/26 15:13, Andre Przywara wrote:
>> From: James Morse <james.morse@arm.com>
>>
>> The MPAM ACPI table lists the MPAM MSCs and indicates which resources
>> in the system they control. Not everything this table can describe is
>> supported by resctrl, e.g. memory-side-caches.
>>
>> Add the additional table parsing to avoid reporting these as 'unknown'
>> to the MPAM driver. This allows class+component hierarchys to be built.
>>
>> Until resctrl has support for any of these resources, users would be
>> in-kernel managers of a resource/PARTID or perf to query bandwidth
>> counters on a resource resctrl is unaware of.
> 
> Does this patch give us anything needed for MPAM-Fb? It seems to just add
> parsing for things we don't support in the MPAM driver.

I think you are right, this just parses more resource types from the MSC 
table, but is otherwise unrelated to MPAM-Fb.
I probably just cargo-culted this patch all the time ;-)
Will drop it now.

Cheers,
Andre.

> 
> Thanks,
> 
> Ben
> 
>>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>   drivers/acpi/arm64/mpam.c | 91 +++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 88 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
>> index 84963a20c3e7..99c2bdbb3314 100644
>> --- a/drivers/acpi/arm64/mpam.c
>> +++ b/drivers/acpi/arm64/mpam.c
>> @@ -95,17 +95,51 @@ static void acpi_mpam_parse_irqs(struct platform_device *pdev,
>>   		res[(*res_idx)++] = DEFINE_RES_IRQ_NAMED(irq, "error");
>>   }
>>   
>> -static int acpi_mpam_parse_resource(struct mpam_msc *msc,
>> +#define UUID_MPAM_INTERCONNECT_TABLE		"fe2bd645-033b-49e6-9479-2e0b8b21d1cd"
>> +
>> +struct acpi_mpam_interconnect_descriptor_table {
>> +	u8	type_uuid[16];
>> +	u32	num_descriptors;
>> +};
>> +
>> +struct acpi_mpam_interconnect_descriptor {
>> +	u32	source_id;
>> +	u32	destination_id;
>> +	u8	link_type;
>> +	u8	reserved[3];
>> +};
>> +
>> +static int acpi_mpam_parse_resource(struct acpi_mpam_msc_node *tbl_msc,
>> +				    struct mpam_msc *msc,
>>   				    struct acpi_mpam_resource_node *res)
>>   {
>> +	struct acpi_mpam_interconnect_descriptor_table *tbl_int_tbl;
>> +	struct acpi_mpam_interconnect_descriptor *tbl_int;
>> +	guid_t int_tbl_uuid, spec_uuid;
>>   	int level, nid;
>>   	u32 cache_id;
>> +	off_t offset;
>>   
>> +	/*
>> +	 * Class IDs are somewhat arbitrary, but need to be co-ordinated.
>> +	 * 0-N are caches,
>> +	 * 64, 65: Interconnect, but ideally these would appear between the
>> +	 *     classes the controls are adjacent to.
>> +	 * 128: SMMU,
>> +	 * 192-192+level: Memory Side Caches, nothing checks that N is a
>> +	 *                small number.
>> +	 * 255: Memory Controllers
>> +	 *
>> +	 * ACPI devices would need a class id allocated based on the _HID.
>> +	 *
>> +	 * Classes that the mpam driver can't currently plumb into resctrl
>> +	 * are registered as UNKNOWN.
>> +	 */
>>   	switch (res->locator_type) {
>>   	case ACPI_MPAM_LOCATION_TYPE_PROCESSOR_CACHE:
>>   		cache_id = res->locator.cache_locator.cache_reference;
>>   		level = find_acpi_cache_level_from_id(cache_id);
>> -		if (level <= 0) {
>> +		if (level <= 0 || level >= 64) {
>>   			pr_err_once("Bad level (%d) for cache with id %u\n", level, cache_id);
>>   			return -EINVAL;
>>   		}
>> @@ -120,6 +154,57 @@ static int acpi_mpam_parse_resource(struct mpam_msc *msc,
>>   		}
>>   		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_MEMORY,
>>   				       MPAM_CLASS_ID_DEFAULT, nid);
>> +	case ACPI_MPAM_LOCATION_TYPE_SMMU:
>> +		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_UNKNOWN,
>> +				       128, res->locator.smmu_locator.smmu_interface);
>> +	case ACPI_MPAM_LOCATION_TYPE_MEMORY_CACHE:
>> +		cache_id = res->locator.mem_cache_locator.reference;
>> +		level = res->locator.mem_cache_locator.level;
>> +		if (192 + level >= 255) {
>> +			pr_err_once("Bad level for memory side cache with reference %u\n",
>> +				    cache_id);
>> +			return -EINVAL;
>> +		}
>> +
>> +		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_CACHE,
>> +				       192 + level, cache_id);
>> +
>> +	case ACPI_MPAM_LOCATION_TYPE_INTERCONNECT:
>> +		/* Find the descriptor table, and check it lands in the parent msc */
>> +		offset = res->locator.interconnect_ifc_locator.inter_connect_desc_tbl_off;
>> +		if (offset >= tbl_msc->length) {
>> +			pr_err_once("Bad offset for interconnect descriptor on msc %u\n",
>> +				    tbl_msc->identifier);
>> +			return -EINVAL;
>> +		}
>> +		tbl_int_tbl = ACPI_ADD_PTR(struct acpi_mpam_interconnect_descriptor_table,
>> +					   tbl_msc, offset);
>> +		guid_parse(UUID_MPAM_INTERCONNECT_TABLE, &spec_uuid);
>> +		import_guid(&int_tbl_uuid, tbl_int_tbl->type_uuid);
>> +		if (guid_equal(&spec_uuid, &int_tbl_uuid)) {
>> +			pr_err_once("Bad UUID for interconnect descriptor on msc %u\n",
>> +				    tbl_msc->identifier);
>> +			return -EINVAL;
>> +		}
>> +
>> +		offset += sizeof(*tbl_int_tbl);
>> +		offset += tbl_int_tbl->num_descriptors * sizeof(*tbl_int);
>> +		if (offset >= tbl_msc->length) {
>> +			pr_err_once("Bad num_descriptors for interconnect descriptor on msc %u\n",
>> +				    tbl_msc->identifier);
>> +			return -EINVAL;
>> +		}
>> +
>> +		tbl_int = ACPI_ADD_PTR(struct acpi_mpam_interconnect_descriptor,
>> +				       tbl_int_tbl, sizeof(*tbl_int_tbl));
>> +		cache_id = tbl_int->source_id;
>> +
>> +		/* Unknown link type? */
>> +		if (tbl_int->link_type != 0 && tbl_int->link_type == 1)
>> +			return 0;
>> +
>> +		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_UNKNOWN,
>> +				       64 + tbl_int->link_type, cache_id);
>>   	default:
>>   		/* These get discovered later and are treated as unknown */
>>   		return 0;
>> @@ -150,7 +235,7 @@ int acpi_mpam_parse_resources(struct mpam_msc *msc,
>>   			return -EINVAL;
>>   		}
>>   
>> -		err = acpi_mpam_parse_resource(msc, resource);
>> +		err = acpi_mpam_parse_resource(tbl_msc, msc, resource);
>>   		if (err)
>>   			return err;
>>   
> 



^ permalink raw reply

* Re: [PATCH] pmdomain: ti_sci: add wakeup constraint to parent devices of wakeup source
From: Kendall Willis @ 2026-05-12 16:17 UTC (permalink / raw)
  To: Vitor Soares
  Cc: Nishanth Menon, Tero Kristo, Santosh Shilimkar, Ulf Hansson,
	Kevin Hilman, Dhruva Gole, linux-arm-kernel, linux-pm,
	linux-kernel, stable, tomi.valkeinen, sebin.francis, devarsht,
	vigneshr, vishalm, vitor.soares
In-Reply-To: <becb54adc0bea88578c8fe4c7c1b7b68bf5cc6d4.camel@gmail.com>

On 17:51-20260511, Vitor Soares wrote:
> Hi Kendall,
> 
> On Wed, 2026-05-06 at 22:16 -0500, Kendall Willis wrote:
> > Set wakeup constraint for any device in a wakeup path. All parent devices
> > of a wakeup device should not be turned off during suspend. This ensures
> > the wakeup device is kept on while the system is suspended.
> > 
> 
> Thanks for the patch.
> 
> I tested it on our Verdin AM62P. As expected, suspend now fails cleanly with "-
> 19" when an SDIO WiFi module is registered as a wakeup source, instead of
> crashing on resume:
> 
> ti-sci 44043000.system-controller: PM: failed to suspend: error -19
> 
> I did not test the IO daisy chain wakeup path, since that is out of scope for
> this patch.
> 
> Best regards,
> Vitor Soares
> 

Hi Vitor,

Thanks for testing the patch! Could you add your Tested-by tag?

Best,
Kendall


^ permalink raw reply

* [PATCH v3] locking/hqlock_core: restore missing call in contention detection logic
From: Fedorov Nikita @ 2026-05-12 16:25 UTC (permalink / raw)
  To: 1234567weewee457
  Cc: fedorov.nikita, stepanov.anatoly, linux-kernel, linux-arm-kernel,
	linux-arch, linux-mm, virtualization
In-Reply-To: <20260512162503.1843025-1-fedorov.nikita@h-partners.com>

A call was accidentally dropped while splitting the hqspinlock patch series.
It did not break the system functionality,
but caused the lock to stay in QSPINLOCK mode.

Restore the missing call.

Sorry for missing this in the previous posting. I rechecked the whole path after fixing.

Signed-off-by: Nikita Fedorov <fedorov.nikita@h-partners.com>
---
 kernel/locking/hqlock_core.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/locking/hqlock_core.h b/kernel/locking/hqlock_core.h
index 74f92ac3d8ad..226a0bb9a667 100644
--- a/kernel/locking/hqlock_core.h
+++ b/kernel/locking/hqlock_core.h
@@ -580,6 +580,8 @@ static inline void hqlock_clear_tail_handoff(struct qspinlock *lock, u32 val,
 
 		hqlock_handoff(lock, node, next, tail, handoff_info);
 	} else {
+		update_counters_qspinlock(qnode);
+
 		if ((val & _Q_TAIL_MASK) == tail) {
 			if (low_contention_try_clear_tail(lock, val, node))
 				return;
-- 
2.34.1



^ permalink raw reply related


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