* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree
From: Arnd Bergmann @ 2026-05-11 19:45 UTC (permalink / raw)
To: Simon Horman, Arnd Bergmann
Cc: Netdev, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski,
Benoît Cousson, David S . Miller, Dmitry Torokhov,
Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg,
Kevin Hilman, krzk+dt, Linus Walleij, Paolo Abeni, Rob Herring,
Roger Quadros, Tony Lindgren, linux-wireless, devicetree,
linux-kernel, linux-arm-kernel, open list:GPIO SUBSYSTEM,
Linux-OMAP, Krzysztof Kozlowski
In-Reply-To: <20260511161243.49098-3-horms@kernel.org>
On Mon, May 11, 2026, at 18:12, Simon Horman wrote:
>
> Simon says: As a pre-existing issue I don't think we need to be concerned
> about this in the scope of this patch-set. But it may
> be a potential for follow-up.
>
> This problem wasn't introduced by this patch, but I noticed a pre-existing
> issue in the context here. Does the p54spi_spi_read() function perform an SPI
> transfer using a stack-allocated buffer?
>
> Looking at p54spi_spi_read(), the addr variable is allocated on the stack and
> passed as a transfer buffer to spi_sync(). The SPI framework maps transfer
> buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with
> CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and
> cannot be safely DMA-mapped.
>
> Could this cause memory corruption or a kernel panic during the transfer?
I already commented on this earlier: This is indeed a theoretical
bug, but it works in practice because DMA-capable SPI controllers
tend to all use MMIO for short transfers instead of DMA.
SPI is very fragile this way, and I do have some patches to improve
that overall, but it seems out of scope for this one driver.
>> -
>> - gpio_direction_output(p54spi_gpio_power, 0);
>> - gpio_direction_input(p54spi_gpio_irq);
>> -
>> - ret = request_irq(gpio_to_irq(p54spi_gpio_irq),
>> - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi",
>> - priv->spi);
>> + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi);
>
> Simon says: This one seems more concerning in the context of this patch-set.
>
> Is there missing validation for spi->irq before calling request_irq()?
>
> If the device tree node lacks an interrupts property, the SPI core may set
> spi->irq to 0 or a negative error code.
>
> Passing this directly to request_irq() without validation can successfully
> hijack a core system interrupt like the system timer on some architectures,
> leading to system instability or deadlocks.
>
> Should there be a check like if (spi->irq <= 0) to fail the probe gracefully?
I also commented on this: request_irq() already fails gracefully
with -EINVAL when presented with an invalid IRQ. IRQ 0 is guaranteed
to be invalid on any target that uses devicetree.
Arnd
^ permalink raw reply
* Re: [PATCH v6 0/7] pinctrl: Add generic pinctrl for board-level mux chips
From: Frank Li @ 2026-05-11 19:38 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++jLmXvnf6sSWsGe+++u37ONJpFFvxFMUkLdvvxiiaWuf9gQ@mail.gmail.com>
On Tue, May 05, 2026 at 02:50:17PM +0200, Linus Walleij wrote:
> Hi Frank!
>
> On Tue, May 5, 2026 at 1:55 AM Frank Li <Frank.Li@nxp.com> wrote:
>
> > Add a generic pinctrl binding for board-level pinmux chips that are
> > controlled through the multiplexer subsystem.
> >
> > On some boards, especially development boards, external mux chips are used
> > to switch SoC signals between different peripherals (e.g. MMC and UART).
> > The mux select lines are often driven by a GPIO expander over I2C,
> > as illustrated below:
> >
> > ┌──────┐ ┌─────┐
> > │ SOC │ │ │ ┌───────┐
> > │ │ │ │───►│ MMC │
> > │ │ │ MUX │ └───────┘
> > │ ├─────►│ │ ┌───────┐
> > │ │ │ │───►│ UART │
> > │ │ └─────┘ └───────┘
> > │ │ ▲
> > │ │ ┌────┴──────────────┐
> > │ I2C ├───►│ GPIO Expander │
> > └──────┘ └───────────────────┘
> >
> > Traditionally, gpio-hog is used to configure the onboard mux at boot.
> > However, the GPIO expander may probe later than consumer devices such as
> > MMC. As a result, the MUX might not be configured when the peripheral
> > driver probes, leading to initialization failures or data transfer errors.
> >
> > Introduce a generic pinctrl binding that models the board-level MUX as a
> > pin control provider and builds proper device links between the MUX, its
> > GPIO controller, and peripheral devices. This ensures correct probe
> > ordering and reliable mux configuration.
> >
> > The implementation leverages the standard multiplexer subsystem, which
> > provides broad support for onboard mux controllers and avoids the need for
> > per-driver custom MUX handling
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> I put the pinctrl patches on an immutable branch for testing again,
> if nothing explodes I will try to merge it for linux-next ASAP!
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?
Frank
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: mfd: st,stmpe: fix PWM schema and drop legacy binding
From: Uwe Kleine-König @ 2026-05-11 19:13 UTC (permalink / raw)
To: Manish Baing
Cc: lee, linusw, robh, krzk+dt, conor+dt, mcoquelin.stm32,
alexandre.torgue, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel, linux-pwm
In-Reply-To: <20260509193928.19030-1-manishbaing2789@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 863 bytes --]
On Sat, May 09, 2026 at 07:39:28PM +0000, Manish Baing wrote:
> The st,stmpe-pwm binding is already covered by the MFD schema in
> Documentation/devicetree/bindings/mfd/st,stmpe.yaml. However, the
> PWM subnode was missing a 'required' properties block. This allowed
> Device Tree nodes to pass validation even if the 'compatible'
> string was omitted. This omission could lead to probe failures
> at runtime.
>
> Fix the schema by adding the missing 'required' block and
> remove the obsolete and redundant text binding file.
>
> Signed-off-by: Manish Baing <manishbaing2789@gmail.com>
Reviewed-by: Uwe Kleine-König <ukleinek@kernel.org>
Lee: If you want to apply that patch, that's fine for me (take my
Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
for that). If I should take it, I'm happily taking your's :-)
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] clk: rockchip: Use flexible array for clock table
From: Rosen Penev @ 2026-05-11 19:08 UTC (permalink / raw)
To: Brian Masney
Cc: linux-clk, Michael Turquette, Stephen Boyd, Heiko Stuebner,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support, open list
In-Reply-To: <agH-ivEdw12_3OJz@redhat.com>
On Mon, May 11, 2026 at 9:06 AM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Rosen,
>
> On Sun, May 10, 2026 at 08:22:52PM -0700, Rosen Penev wrote:
> > Store the clock lookup table in the Rockchip clock provider
> > allocation instead of allocating it separately.
> >
> > This ties the table lifetime directly to the provider and removes a
> > separate allocation failure path while preserving the clk_onecell_data
> > lookup interface.
> >
> > Assisted-by: Codex:GPT-5.5
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> > drivers/clk/rockchip/clk.c | 17 ++++-------------
> > drivers/clk/rockchip/clk.h | 2 ++
> > 2 files changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> > index ee8c79b938d3..dbb4b6f33abb 100644
> > --- a/drivers/clk/rockchip/clk.c
> > +++ b/drivers/clk/rockchip/clk.c
> > @@ -359,26 +359,21 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
> > unsigned long nr_clks, bool has_late_clocks)
> > {
> > struct rockchip_clk_provider *ctx;
> > - struct clk **clk_table;
> > struct clk *default_clk_val;
> > int i;
> >
> > default_clk_val = ERR_PTR(has_late_clocks ? -EPROBE_DEFER : -ENOENT);
> >
> > - ctx = kzalloc_obj(struct rockchip_clk_provider);
> > + ctx = kzalloc_flex(*ctx, clk_table, nr_clks);
> > if (!ctx)
> > return ERR_PTR(-ENOMEM);
> >
> > - clk_table = kzalloc_objs(struct clk *, nr_clks);
> > - if (!clk_table)
> > - goto err_free;
> > -
> > for (i = 0; i < nr_clks; ++i)
> > - clk_table[i] = default_clk_val;
> > + ctx->clk_table[i] = default_clk_val;
> >
> > - ctx->reg_base = base;
> > - ctx->clk_data.clks = clk_table;
> > ctx->clk_data.clk_num = nr_clks;
> > + ctx->clk_data.clks = ctx->clk_table;
>
> Where's the __counted_by? Also struct clk_onecell_data is embedded
> inside struct rockchip_clk_provider, and I'm not sure offhand how
> that'll work.
Not present. __counted_by does not work with embedded struct members.
No need to add one IMO.
>
> Brian
>
>
> > + ctx->reg_base = base;
> > ctx->cru_node = np;
> > spin_lock_init(&ctx->lock);
> >
> > @@ -388,10 +383,6 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
> > "rockchip,grf");
> >
> > return ctx;
> > -
> > -err_free:
> > - kfree(ctx);
> > - return ERR_PTR(-ENOMEM);
> > }
> >
> > struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
> > diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> > index 9e3503e2ffc2..d4033bf750f5 100644
> > --- a/drivers/clk/rockchip/clk.h
> > +++ b/drivers/clk/rockchip/clk.h
> > @@ -604,6 +604,7 @@ struct rockchip_aux_grf {
> > * @grf: regmap of the general-register-files syscon
> > * @aux_grf_table: hashtable of auxiliary GRF regmaps, indexed by grf_type
> > * @lock: maintains exclusion between callbacks for a given clock-provider.
> > + * @clk_table: clock lookup table.
> > */
> > struct rockchip_clk_provider {
> > void __iomem *reg_base;
> > @@ -612,6 +613,7 @@ struct rockchip_clk_provider {
> > struct regmap *grf;
> > DECLARE_HASHTABLE(aux_grf_table, GRF_HASH_ORDER);
> > spinlock_t lock;
> > + struct clk *clk_table[];
> > };
> >
> > struct rockchip_pll_rate_table {
> > --
> > 2.54.0
> >
>
^ permalink raw reply
* Re: [PATCH v4 02/15] mm: Make empty_zero_page __ro_after_init
From: Jann Horn @ 2026-05-11 19:01 UTC (permalink / raw)
To: Kees Cook
Cc: Ard Biesheuvel, linux-arm-kernel, linux-kernel, will,
catalin.marinas, mark.rutland, Ard Biesheuvel, Ryan Roberts,
Anshuman Khandual, Liz Prucka, Seth Jenkins, Mike Rapoport,
David Hildenbrand, Andrew Morton, linux-mm, linux-hardening
In-Reply-To: <202605111144.349EF737E@keescook>
On Mon, May 11, 2026 at 8:45 PM Kees Cook <kees@kernel.org> wrote:
> On Fri, May 08, 2026 at 07:02:51PM +0200, Jann Horn wrote:
> > On Mon, Apr 27, 2026 at 5:44 PM Ard Biesheuvel <ardb+git@google.com> wrote:
> > > The empty zero page is used to back any kernel or user space mapping
> > > that is supposed to remain cleared, and so the page itself is never
> > > supposed to be modified.
> > >
> > > So make it __ro_after_init rather than __page_aligned_bss: on most
> > > architectures, this ensures that both the kernel's mapping of it and any
> > > aliases that are accessible via the kernel direct (linear) map are
> > > mapped read-only, and cannot be used (inadvertently or maliciously) to
> > > corrupt the contents of the zero page.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > Reviewed-by: Jann Horn <jannh@google.com>
> >
> > Sorry, I should have looked at this properly earlier instead of ending
> > up duplicating this patch with
> > <https://lore.kernel.org/all/20260508-ro-zeropage-v1-1-9808abc20b49@google.com/>.
>
> As you mention in your testing of the patch, could we add an LKDTM test
> that does the same to catch any regressions?
Shouldn't be too hard - LKDTM crashtypes don't accept arguments, so we
couldn't easily pass in a userspace pointer, I think, but we could
make a crashtype that allocates anon memory with do_mmap() (under
mmap_lock), then GUP on the allocated userspace address, then writes
into the obtained address with something like atomic_add(0, <pointer
from kmap(page)>) to trigger a write access without actually changing
memory contents...
^ permalink raw reply
* Re: [PATCH 09/16] arm64: dts: intel: Add EL2 virtual timer interrupt
From: Dinh Nguyen @ 2026-05-11 19:00 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel, linux-acpi, linux-kernel,
devicetree
Cc: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Catalin Marinas,
Will Deacon, Rafael J. Wysocki, Mark Rutland, Daniel Lezcano,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Ge Gordon,
BST Linux Kernel Upstream Group, Jesper Nilsson, Lars Persson,
Alim Akhtar, Ivaylo Ivanov, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Matthias Brugger,
AngeloGioacchino Del Regno, Thierry Reding, Jonathan Hunter,
Bjorn Andersson, Konrad Dybcio, Andreas Färber,
Heiko Stuebner, Shawn Lin, Orson Zhai, Baolin Wang, Michal Simek
In-Reply-To: <20260507125544.2903406-10-maz@kernel.org>
On 5/7/26 07:55, Marc Zyngier wrote:
> The ARMv8.2 based CPUs used in the agilex5 SoC are missing the EL2 virtual
> timer interrupt. Add it.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index 02e62d954e949..6db2d48b9bad3 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> @@ -155,7 +155,8 @@ timer {
> interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 12 IRQ_TYPE_LEVEL_LOW>;
> };
>
> usbphy0: usbphy {
Acked-by: Dinh Nguyen <dinguyen@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 02/15] mm: Make empty_zero_page __ro_after_init
From: Kees Cook @ 2026-05-11 18:45 UTC (permalink / raw)
To: Jann Horn
Cc: Ard Biesheuvel, linux-arm-kernel, linux-kernel, will,
catalin.marinas, mark.rutland, Ard Biesheuvel, Ryan Roberts,
Anshuman Khandual, Liz Prucka, Seth Jenkins, Mike Rapoport,
David Hildenbrand, Andrew Morton, linux-mm, linux-hardening
In-Reply-To: <CAG48ez1DJ88a0pBCE-Q0VXyDuJgng8zdunb38g4b4JPU88exww@mail.gmail.com>
On Fri, May 08, 2026 at 07:02:51PM +0200, Jann Horn wrote:
> On Mon, Apr 27, 2026 at 5:44 PM Ard Biesheuvel <ardb+git@google.com> wrote:
> > The empty zero page is used to back any kernel or user space mapping
> > that is supposed to remain cleared, and so the page itself is never
> > supposed to be modified.
> >
> > So make it __ro_after_init rather than __page_aligned_bss: on most
> > architectures, this ensures that both the kernel's mapping of it and any
> > aliases that are accessible via the kernel direct (linear) map are
> > mapped read-only, and cannot be used (inadvertently or maliciously) to
> > corrupt the contents of the zero page.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Reviewed-by: Jann Horn <jannh@google.com>
>
> Sorry, I should have looked at this properly earlier instead of ending
> up duplicating this patch with
> <https://lore.kernel.org/all/20260508-ro-zeropage-v1-1-9808abc20b49@google.com/>.
As you mention in your testing of the patch, could we add an LKDTM test
that does the same to catch any regressions?
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 0/6] phy: rockchip: samsung-hdptx: Clock fixes and API transition cleanups
From: Cristian Ciocaltea @ 2026-05-11 18:31 UTC (permalink / raw)
To: Vinod Koul
Cc: Neil Armstrong, Heiko Stuebner, Algea Cao, Dmitry Baryshkov,
kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <agIBa7qxIGgqCYZ1@vaman>
On 5/11/26 7:18 PM, Vinod Koul wrote:
> On 10-05-26, 11:55, Cristian Ciocaltea wrote:
>> Hi Vinod,
>>
>> On 5/10/26 10:36 AM, Vinod Koul wrote:
>>> On 27-02-26, 22:48, Cristian Ciocaltea wrote:
>>>> This series provides a set of bug fixes and cleanups for the Rockchip
>>>> Samsung HDPTX PHY driver.
>>>>
>>>> The first part of the series (i.e. PATCH 1 & 2) addresses clock rate
>>>> calculation and synchronization issues. Specifically, it fixes edge
>>>> cases where the PHY PLL is pre-programmed by an external component (like
>>>> a bootloader) or when changing the color depth (bpc) while keeping the
>>>> modeline constant. Because the Common Clock Framework .set_rate()
>>>> callback might not be invoked if the pixel clock remains unchanged, this
>>>> previously led to out-of-sync states between CCF and the actual HDMI PHY
>>>> configuration.
>>>>
>>>> The second part focuses on code cleanups and modernizing the register
>>>> access. Now that dw_hdmi_qp driver has fully switched to using
>>>> phy_configure(), we can drop the deprecated TMDS rate setup workarounds
>>>> and the restrict_rate_change flag logic. Finally, it refactors the
>>>> driver to consistently use standard bitfield macros.
>>>
>>> Sorry looks like I have missed to review this one.
>>> Can you please rebase on phy/fixes and send...
>>
>> I've just verified and it applies cleanly on top of phy/fixes.
>> Do you still need a resend?
>
> Yes please, it didnt apply for me
Oh, I used the following branch, hopefully it's the right one:
https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git/log/?h=fixes
Regardless, I submitted v2, rebased on the above, while providing a few minor
changes:
https://lore.kernel.org/all/20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com/
Thanks,
Cristian
^ permalink raw reply
* [PATCH 3/3] pinctrl: mediatek: fix SPDX comment style in header
From: Mayur Kumar @ 2026-05-11 18:30 UTC (permalink / raw)
To: sean.wang, linusw, matthias.bgg, angelogioacchino.delregno
Cc: linux-mediatek, linux-gpio, linux-kernel, linux-arm-kernel,
Mayur Kumar
Header files should use the C-style '/*' block comment for SPDX
license identifiers. Correct the style in pinctrl-mtk-mt8365.h
to satisfy checkpatch requirements.
Signed-off-by: Mayur Kumar <kmayur809@gmail.com>
---
drivers/pinctrl/mediatek/pinctrl-mtk-mt8365.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-mt8365.h b/drivers/pinctrl/mediatek/pinctrl-mtk-mt8365.h
index 39e17532c46..fc22f412bd7 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-mt8365.h
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-mt8365.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (C) 2018 MediaTek Inc.
* Author: Zhiyong Tao <zhiyong.tao@mediatek.com>
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] pinctrl: actions: fix SPDX comment style in header
From: Mayur Kumar @ 2026-05-11 18:30 UTC (permalink / raw)
To: linusw, afaerber, mani
Cc: linux-gpio, linux-arm-kernel, linux-actions, linux-kernel,
Mayur Kumar
Header files should use the C-style '/*' block comment for SPDX
license identifiers. Correct the style in pinctrl-owl.h
to satisfy checkpatch requirements.
Signed-off-by: Mayur Kumar <kmayur809@gmail.com>
---
drivers/pinctrl/actions/pinctrl-owl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/actions/pinctrl-owl.h b/drivers/pinctrl/actions/pinctrl-owl.h
index dae2e8363fd..feee7ad7e27 100644
--- a/drivers/pinctrl/actions/pinctrl-owl.h
+++ b/drivers/pinctrl/actions/pinctrl-owl.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0+
+/* SPDX-License-Identifier: GPL-2.0+ */
/*
* OWL SoC's Pinctrl definitions
*
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] MAINTAINERS: change maintainers for macb Ethernet driver
From: Théo Lebrun @ 2026-05-11 18:26 UTC (permalink / raw)
To: patchwork-bot+netdevbpf, Nicolas Ferre
Cc: conor.dooley, claudiu.beznea, netdev, alexandre.belloni,
andrew+netdev, davem, edumazet, kuba, pabeni, linux-arm-kernel,
linux-kernel
In-Reply-To: <177828000904.878212.12911947829161473740.git-patchwork-notify@kernel.org>
On Sat May 9, 2026 at 12:40 AM CEST, wrote:
> This patch was applied to netdev/net.git (main)
> by Jakub Kicinski <kuba@kernel.org>:
>
> On Thu, 7 May 2026 14:04:44 +0200 you wrote:
>> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>>
>> I would like to hand over the macb maintenance to Théo, as I'm unable to
>> keep up with the recent flow of patches for this driver. After speaking
>> with Claudiu, he indicated that he is in the same position as me.
>> To help with this work, Conor has agreed to act as a reviewer.
>>
>> [...]
>
> Here is the summary with links:
> - MAINTAINERS: change maintainers for macb Ethernet driver
> https://git.kernel.org/netdev/net/c/0a549298f452
>
> You are awesome, thank you!
Acked-by: Théo Lebrun <theo.lebrun@bootlin.com>
Oh wait, I am too late.
Thanks for you work during all those years to both of you,
and thanks for the kind words Nicolas.
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH v2 5/6] phy: rockchip: samsung-hdptx: Simplify GRF access with FIELD_PREP_WM16()
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
The 16 most significant bits of the general-purpose register (GRF) are
used as a write-enable mask for the remaining 16 bits.
Make use of the recently introduced FIELD_PREP_WM16() macro to avoid
open-coding the bit shift operations and improve code readability.
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 52 +++++++++++------------
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 5c3a9b4b1737..611425e44b26 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (c) 2021-2022 Rockchip Electronics Co., Ltd.
- * Copyright (c) 2024 Collabora Ltd.
+ * Copyright (c) 2024-2026 Collabora Ltd.
*
* Author: Algea Cao <algea.cao@rock-chips.com>
* Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
@@ -10,6 +10,7 @@
#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/delay.h>
+#include <linux/hw_bitfield.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -949,7 +950,9 @@ static void rk_hdptx_pre_power_up(struct rk_hdptx_phy *hdptx)
reset_control_assert(hdptx->rsts[RST_CMN].rstc);
reset_control_assert(hdptx->rsts[RST_INIT].rstc);
- val = (HDPTX_I_PLL_EN | HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN) << 16;
+ val = (FIELD_PREP_WM16(HDPTX_I_PLL_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 0));
regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
}
@@ -960,8 +963,8 @@ static int rk_hdptx_post_enable_lane(struct rk_hdptx_phy *hdptx)
reset_control_deassert(hdptx->rsts[RST_LANE].rstc);
- val = (HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN) << 16 |
- HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN;
+ val = (FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 1) |
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 1));
regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
/* 3 lanes FRL mode */
@@ -990,16 +993,15 @@ static int rk_hdptx_post_enable_pll(struct rk_hdptx_phy *hdptx)
u32 val;
int ret;
- val = (HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN) << 16 |
- HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN;
+ val = (FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 1) |
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 1));
regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
usleep_range(10, 15);
reset_control_deassert(hdptx->rsts[RST_INIT].rstc);
usleep_range(10, 15);
- val = HDPTX_I_PLL_EN << 16 | HDPTX_I_PLL_EN;
- regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(HDPTX_I_PLL_EN, 1));
usleep_range(10, 15);
reset_control_deassert(hdptx->rsts[RST_CMN].rstc);
@@ -1037,7 +1039,9 @@ static void rk_hdptx_phy_disable(struct rk_hdptx_phy *hdptx)
reset_control_assert(hdptx->rsts[RST_CMN].rstc);
reset_control_assert(hdptx->rsts[RST_INIT].rstc);
- val = (HDPTX_I_PLL_EN | HDPTX_I_BIAS_EN | HDPTX_I_BGR_EN) << 16;
+ val = (FIELD_PREP_WM16(HDPTX_I_PLL_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 0));
regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
}
@@ -1135,7 +1139,7 @@ static int rk_hdptx_frl_lcpll_cmn_config(struct rk_hdptx_phy *hdptx)
rk_hdptx_pre_power_up(hdptx);
- regmap_write(hdptx->grf, GRF_HDPTX_CON0, LC_REF_CLK_SEL << 16);
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(LC_REF_CLK_SEL, 0));
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_common_cmn_init_seq);
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_frl_lcpll_cmn_init_seq);
@@ -1178,8 +1182,7 @@ static int rk_hdptx_frl_lcpll_ropll_cmn_config(struct rk_hdptx_phy *hdptx)
rk_hdptx_pre_power_up(hdptx);
/* ROPLL input reference clock from LCPLL (cascade mode) */
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- (LC_REF_CLK_SEL << 16) | LC_REF_CLK_SEL);
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(LC_REF_CLK_SEL, 1));
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_common_cmn_init_seq);
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_frl_lcpll_ropll_cmn_init_seq);
@@ -1218,7 +1221,7 @@ static int rk_hdptx_tmds_ropll_cmn_config(struct rk_hdptx_phy *hdptx)
rk_hdptx_pre_power_up(hdptx);
- regmap_write(hdptx->grf, GRF_HDPTX_CON0, LC_REF_CLK_SEL << 16);
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(LC_REF_CLK_SEL, 0));
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_common_cmn_init_seq);
rk_hdptx_multi_reg_write(hdptx, rk_hdptx_tmds_cmn_init_seq);
@@ -1336,11 +1339,9 @@ static void rk_hdptx_dp_reset(struct rk_hdptx_phy *hdptx)
FIELD_PREP(LN_TX_DRV_EI_EN_MASK, 0));
regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x0));
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_BIAS_EN << 16 | FIELD_PREP(HDPTX_I_BIAS_EN, 0x0));
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_BGR_EN << 16 | FIELD_PREP(HDPTX_I_BGR_EN, 0x0));
+ FIELD_PREP_WM16(HDPTX_I_PLL_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 0) |
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 0));
}
static int rk_hdptx_phy_consumer_get(struct rk_hdptx_phy *hdptx)
@@ -1616,9 +1617,8 @@ static int rk_hdptx_dp_aux_init(struct rk_hdptx_phy *hdptx)
FIELD_PREP(OVRD_SB_VREG_EN_MASK, 0x1));
regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_BGR_EN << 16 | FIELD_PREP(HDPTX_I_BGR_EN, 0x1));
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_BIAS_EN << 16 | FIELD_PREP(HDPTX_I_BIAS_EN, 0x1));
+ FIELD_PREP_WM16(HDPTX_I_BGR_EN, 1) |
+ FIELD_PREP_WM16(HDPTX_I_BIAS_EN, 1));
usleep_range(20, 25);
reset_control_deassert(hdptx->rsts[RST_INIT].rstc);
@@ -1665,7 +1665,7 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
if (mode == PHY_MODE_DP) {
regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x1));
+ FIELD_PREP_WM16(HDPTX_MODE_SEL, 1));
for (lane = 0; lane < 4; lane++) {
regmap_update_bits(hdptx->regmap, LANE_REG(031e) + 0x400 * lane,
@@ -1693,7 +1693,7 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
if (!ret) {
regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x0));
+ FIELD_PREP_WM16(HDPTX_MODE_SEL, 0));
if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
ret = rk_hdptx_frl_lcpll_mode_config(hdptx);
@@ -1828,8 +1828,7 @@ static int rk_hdptx_phy_set_rate(struct rk_hdptx_phy *hdptx,
u32 bw, status;
int ret;
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x0));
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(HDPTX_I_PLL_EN, 0));
switch (dp->link_rate) {
case 1620:
@@ -1885,8 +1884,7 @@ static int rk_hdptx_phy_set_rate(struct rk_hdptx_phy *hdptx,
regmap_update_bits(hdptx->regmap, CMN_REG(0095), DP_TX_LINK_BW_MASK,
FIELD_PREP(DP_TX_LINK_BW_MASK, bw));
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x1));
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0, FIELD_PREP_WM16(HDPTX_I_PLL_EN, 1));
ret = regmap_read_poll_timeout(hdptx->grf, GRF_HDPTX_STATUS,
status, FIELD_GET(HDPTX_O_PLL_LOCK_DONE, status),
--
2.53.0
^ permalink raw reply related
* [PATCH v2 6/6] phy: rockchip: samsung-hdptx: Consistently use bitfield macros
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
Make the code more robust and improve readability by using the available
bitfield macros (e.g. FIELD_PREP, FIELD_GET) whenever possible, instead
of open coding the related bit operations.
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 24 ++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 611425e44b26..2bd794360661 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -53,6 +53,12 @@
/* CMN_REG(001e) */
#define LCPLL_PI_EN_MASK BIT(5)
#define LCPLL_100M_CLK_EN_MASK BIT(0)
+/* CMN_REG(0022) */
+#define ANA_LCPLL_PMS_PDIV_MASK GENMASK(7, 4)
+#define ANA_LCPLL_PMS_REFDIV_MASK GENMASK(3, 0)
+/* CMN_REG(0023) */
+#define LCPLL_PMS_SDIV_RBR_MASK GENMASK(7, 4)
+#define LCPLL_PMS_SDIV_HBR_MASK GENMASK(3, 0)
/* CMN_REG(0025) */
#define LCPLL_PMS_IQDIV_RSTN_MASK BIT(4)
/* CMN_REG(0028) */
@@ -1157,9 +1163,11 @@ static int rk_hdptx_frl_lcpll_cmn_config(struct rk_hdptx_phy *hdptx)
regmap_write(hdptx->regmap, CMN_REG(0020), cfg->pms_mdiv);
regmap_write(hdptx->regmap, CMN_REG(0021), cfg->pms_mdiv_afc);
regmap_write(hdptx->regmap, CMN_REG(0022),
- (cfg->pms_pdiv << 4) | cfg->pms_refdiv);
+ FIELD_PREP(ANA_LCPLL_PMS_PDIV_MASK, cfg->pms_pdiv) |
+ FIELD_PREP(ANA_LCPLL_PMS_REFDIV_MASK, cfg->pms_refdiv));
regmap_write(hdptx->regmap, CMN_REG(0023),
- (cfg->pms_sdiv << 4) | cfg->pms_sdiv);
+ FIELD_PREP(LCPLL_PMS_SDIV_RBR_MASK, cfg->pms_sdiv) |
+ FIELD_PREP(LCPLL_PMS_SDIV_HBR_MASK, cfg->pms_sdiv));
regmap_write(hdptx->regmap, CMN_REG(002a), cfg->sdm_deno);
regmap_write(hdptx->regmap, CMN_REG(002b), cfg->sdm_num_sign);
regmap_write(hdptx->regmap, CMN_REG(002c), cfg->sdm_num);
@@ -1229,8 +1237,10 @@ static int rk_hdptx_tmds_ropll_cmn_config(struct rk_hdptx_phy *hdptx)
regmap_write(hdptx->regmap, CMN_REG(0051), cfg->pms_mdiv);
regmap_write(hdptx->regmap, CMN_REG(0055), cfg->pms_mdiv_afc);
regmap_write(hdptx->regmap, CMN_REG(0059),
- (cfg->pms_pdiv << 4) | cfg->pms_refdiv);
- regmap_write(hdptx->regmap, CMN_REG(005a), cfg->pms_sdiv << 4);
+ FIELD_PREP(ANA_ROPLL_PMS_PDIV_MASK, cfg->pms_pdiv) |
+ FIELD_PREP(ANA_ROPLL_PMS_REFDIV_MASK, cfg->pms_refdiv));
+ regmap_write(hdptx->regmap, CMN_REG(005a),
+ FIELD_PREP(ROPLL_PMS_SDIV_RBR_MASK, cfg->pms_sdiv));
regmap_update_bits(hdptx->regmap, CMN_REG(005e), ROPLL_SDM_EN_MASK,
FIELD_PREP(ROPLL_SDM_EN_MASK, cfg->sdm_en));
@@ -2177,7 +2187,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
ret = regmap_read(hdptx->regmap, CMN_REG(0023), &val);
if (ret)
return 0;
- lcpll_hw.pms_sdiv = val & 0xf;
+ lcpll_hw.pms_sdiv = FIELD_GET(LCPLL_PMS_SDIV_HBR_MASK, val);
ret = regmap_read(hdptx->regmap, CMN_REG(002B), &val);
if (ret)
@@ -2197,7 +2207,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
ret = regmap_read(hdptx->regmap, CMN_REG(002D), &val);
if (ret)
return 0;
- lcpll_hw.sdc_n = (val & LCPLL_SDC_N_MASK) >> 1;
+ lcpll_hw.sdc_n = FIELD_GET(LCPLL_SDC_N_MASK, val);
for (i = 0; i < ARRAY_SIZE(rk_hdptx_frl_lcpll_cfg); i++) {
const struct lcpll_config *cfg = &rk_hdptx_frl_lcpll_cfg[i];
@@ -2258,7 +2268,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
ret = regmap_read(hdptx->regmap, CMN_REG(0086), &val);
if (ret)
return 0;
- ropll_hw.pms_sdiv = ((val & PLL_PCG_POSTDIV_SEL_MASK) >> 4) + 1;
+ ropll_hw.pms_sdiv = FIELD_GET(PLL_PCG_POSTDIV_SEL_MASK, val) + 1;
bpc = (FIELD_GET(PLL_PCG_CLK_SEL_MASK, val) << 1) + 8;
fout = PLL_REF_CLK * ropll_hw.pms_mdiv;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/6] phy: rockchip: samsung-hdptx: Drop restrict_rate_change handling
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
Since commit 6efbd0f46dd8 ("phy: rockchip: samsung-hdptx: Restrict
altering TMDS char rate via CCF"), adjusting the rate via the Common
Clock Framework API has been disallowed.
To avoid breaking existing users until switching to the PHY config API,
it introduced a temporary exception to the rule, controlled via the
'restrict_rate_change' flag.
As the API transition completed, remove the now deprecated exception
logic.
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 42 +++++------------------
1 file changed, 8 insertions(+), 34 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index b5354a24b6eb..5c3a9b4b1737 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -414,7 +414,6 @@ struct rk_hdptx_phy {
/* clk provider */
struct clk_hw hw;
bool pll_config_dirty;
- bool restrict_rate_change;
atomic_t usage_count;
@@ -2074,7 +2073,6 @@ static int rk_hdptx_phy_configure(struct phy *phy, union phy_configure_opts *opt
if (ret) {
dev_err(hdptx->dev, "invalid hdmi params for phy configure\n");
} else {
- hdptx->restrict_rate_change = true;
hdptx->pll_config_dirty = true;
dev_dbg(hdptx->dev, "%s %s rate=%llu bpc=%u\n", __func__,
@@ -2301,41 +2299,17 @@ static int rk_hdptx_phy_clk_determine_rate(struct clk_hw *hw,
struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
/*
- * Invalidate current clock rate to ensure rk_hdptx_phy_clk_set_rate()
- * will be invoked to commit PLL configuration.
+ * For uncommitted PLL configuration, invalidate the current clock rate
+ * to ensure rk_hdptx_phy_clk_set_rate() will be always invoked.
+ * Otherwise, restrict the rate according to the PHY link setup.
*/
- if (hdptx->pll_config_dirty) {
+ if (hdptx->pll_config_dirty)
req->rate = 0;
- return 0;
- }
-
- if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL) {
+ else if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
req->rate = hdptx->hdmi_cfg.rate;
- return 0;
- }
-
- /*
- * FIXME: Temporarily allow altering TMDS char rate via CCF.
- * To be dropped as soon as the RK DW HDMI QP bridge driver
- * switches to make use of phy_configure().
- */
- if (!hdptx->restrict_rate_change && req->rate != hdptx->hdmi_cfg.rate) {
- struct phy_configure_opts_hdmi hdmi = {
- .tmds_char_rate = req->rate,
- };
-
- int ret = rk_hdptx_phy_verify_hdmi_config(hdptx, &hdmi, &hdptx->hdmi_cfg);
-
- if (ret)
- return ret;
- }
-
- /*
- * The TMDS char rate shall be adjusted via phy_configure() only,
- * hence ensure rk_hdptx_phy_clk_set_rate() won't be invoked with
- * a different rate argument.
- */
- req->rate = DIV_ROUND_CLOSEST_ULL(hdptx->hdmi_cfg.rate * 8, hdptx->hdmi_cfg.bpc);
+ else
+ req->rate = DIV_ROUND_CLOSEST_ULL(hdptx->hdmi_cfg.rate * 8,
+ hdptx->hdmi_cfg.bpc);
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/6] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
Since commit ba9c2fe18c17 ("drm/rockchip: dw_hdmi_qp: Switch to
phy_configure()") the TMDS rate setup doesn't rely anymore on the
unconventional usage of the bus width, instead it is managed exclusively
through the HDMI PHY configuration API.
Drop the now obsolete workaround to retrieve the TMDS character rate via
phy_get_bus_width() during power_on().
While at it, get rid of the extra call to rk_hdptx_phy_consumer_put() by
moving the statement at the end of the function.
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 26 ++++++-----------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 12c259d9544c..b5354a24b6eb 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -1660,22 +1660,6 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
enum phy_mode mode = phy_get_mode(phy);
int ret, lane;
- if (mode != PHY_MODE_DP) {
- if (!hdptx->hdmi_cfg.rate && hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL) {
- /*
- * FIXME: Temporary workaround to setup TMDS char rate
- * from the RK DW HDMI QP bridge driver.
- * Will be removed as soon the switch to the HDMI PHY
- * configuration API has been completed on both ends.
- */
- hdptx->hdmi_cfg.rate = phy_get_bus_width(hdptx->phy) & 0xfffffff;
- hdptx->hdmi_cfg.rate *= 100;
- }
-
- dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
- hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
- }
-
ret = rk_hdptx_phy_consumer_get(hdptx);
if (ret)
return ret;
@@ -1701,9 +1685,10 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
rk_hdptx_dp_pll_init(hdptx);
ret = rk_hdptx_dp_aux_init(hdptx);
- if (ret)
- rk_hdptx_phy_consumer_put(hdptx, true);
} else {
+ dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
+ hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
+
if (hdptx->pll_config_dirty)
ret = rk_hdptx_pll_cmn_config(hdptx);
@@ -1715,11 +1700,12 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
ret = rk_hdptx_frl_lcpll_mode_config(hdptx);
else
ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
- } else {
- rk_hdptx_phy_consumer_put(hdptx, true);
}
}
+ if (ret)
+ rk_hdptx_phy_consumer_put(hdptx, true);
+
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/6] phy: rockchip: samsung-hdptx: Handle uncommitted PHY config changes
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
Any changes to the PHY link rate and/or color depth done via the HDMI
PHY configuration API are not immediately programmed into the hardware,
but are delayed until the PHY usage count gets incremented from 0 to 1,
that is when it is powered on or when the PLL clock exposed through
the CCF API is prepared, whichever comes first.
Since the clock might remain in prepared state after subsequent PHY
config changes, the programming can also be triggered via
clk_ops.set_rate(). However, from the clock consumer perspective (i.e.
VOP2 display controller), the (pixel) clock rate doesn't vary with bpc,
as that is handled internally by the PHY and reflected in the TDMS
character rate only.
As a consequence, changing the bpc while preserving the modeline may
lead to out-of-sync issues between CCF and HDMI PHY config state,
because the .set_rate() callback is not invoked when clock rate remains
constant. This may also happen when the PHY PLL has been pre-programmed
by an external entity, e.g. the bootloader, which is actually a
regression introduced by the recent FRL patches.
Introduce a pll_config_dirty flag to keep track of uncommitted PHY
config changes and use it in clk_ops.determine_rate() to invalidate the
current clock rate (as known by CCF) and, consequently, ensure those
changes are programmed into hardware via clk_ops.set_rate().
Moreover, proceed with a similar fix in phy_ops.power_on() callback, to
handle the scenario where the CCF API is not used due to operating in
FRL mode, while the clock is still in a prepared state and thus
preventing rk_hdptx_phy_consumer_get() to apply the updated PHY
configuration.
Fixes: de5dba833118 ("phy: rockchip: samsung-hdptx: Add HDMI 2.1 FRL support")
Fixes: 9d0ec51d7c22 ("phy: rockchip: samsung-hdptx: Add high color depth management")
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 85 +++++++++++++----------
1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 7fb1c22318bb..12c259d9544c 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -413,6 +413,7 @@ struct rk_hdptx_phy {
/* clk provider */
struct clk_hw hw;
+ bool pll_config_dirty;
bool restrict_rate_change;
atomic_t usage_count;
@@ -1260,13 +1261,19 @@ static int rk_hdptx_tmds_ropll_cmn_config(struct rk_hdptx_phy *hdptx)
static int rk_hdptx_pll_cmn_config(struct rk_hdptx_phy *hdptx)
{
+ int ret;
+
if (hdptx->hdmi_cfg.rate <= HDMI20_MAX_RATE)
- return rk_hdptx_tmds_ropll_cmn_config(hdptx);
+ ret = rk_hdptx_tmds_ropll_cmn_config(hdptx);
+ else if (hdptx->hdmi_cfg.rate == FRL_8G4L_RATE)
+ ret = rk_hdptx_frl_lcpll_ropll_cmn_config(hdptx);
+ else
+ ret = rk_hdptx_frl_lcpll_cmn_config(hdptx);
- if (hdptx->hdmi_cfg.rate == FRL_8G4L_RATE)
- return rk_hdptx_frl_lcpll_ropll_cmn_config(hdptx);
+ if (!ret)
+ hdptx->pll_config_dirty = false;
- return rk_hdptx_frl_lcpll_cmn_config(hdptx);
+ return ret;
}
static int rk_hdptx_frl_lcpll_mode_config(struct rk_hdptx_phy *hdptx)
@@ -1347,25 +1354,22 @@ static int rk_hdptx_phy_consumer_get(struct rk_hdptx_phy *hdptx)
return 0;
ret = regmap_read(hdptx->grf, GRF_HDPTX_STATUS, &status);
- if (ret)
- goto dec_usage;
-
- if (status & HDPTX_O_PLL_LOCK_DONE)
- dev_warn(hdptx->dev, "PLL locked by unknown consumer!\n");
+ if (ret) {
+ atomic_dec(&hdptx->usage_count);
+ return ret;
+ }
if (mode == PHY_MODE_DP) {
rk_hdptx_dp_reset(hdptx);
} else {
- ret = rk_hdptx_pll_cmn_config(hdptx);
- if (ret)
- goto dec_usage;
+ /*
+ * Ignore PLL config errors at this point as pll_config_dirty
+ * was not reset and, therefore, operation will be retried.
+ */
+ rk_hdptx_pll_cmn_config(hdptx);
}
return 0;
-
-dec_usage:
- atomic_dec(&hdptx->usage_count);
- return ret;
}
static int rk_hdptx_phy_consumer_put(struct rk_hdptx_phy *hdptx, bool force)
@@ -1700,16 +1704,20 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
if (ret)
rk_hdptx_phy_consumer_put(hdptx, true);
} else {
- regmap_write(hdptx->grf, GRF_HDPTX_CON0,
- HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x0));
+ if (hdptx->pll_config_dirty)
+ ret = rk_hdptx_pll_cmn_config(hdptx);
- if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
- ret = rk_hdptx_frl_lcpll_mode_config(hdptx);
- else
- ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
+ if (!ret) {
+ regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+ HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x0));
- if (ret)
+ if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
+ ret = rk_hdptx_frl_lcpll_mode_config(hdptx);
+ else
+ ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
+ } else {
rk_hdptx_phy_consumer_put(hdptx, true);
+ }
}
return ret;
@@ -2081,7 +2089,10 @@ static int rk_hdptx_phy_configure(struct phy *phy, union phy_configure_opts *opt
dev_err(hdptx->dev, "invalid hdmi params for phy configure\n");
} else {
hdptx->restrict_rate_change = true;
- dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
+ hdptx->pll_config_dirty = true;
+
+ dev_dbg(hdptx->dev, "%s %s rate=%llu bpc=%u\n", __func__,
+ hdptx->hdmi_cfg.mode ? "FRL" : "TMDS",
hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
}
@@ -2303,8 +2314,19 @@ static int rk_hdptx_phy_clk_determine_rate(struct clk_hw *hw,
{
struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
- if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
- return hdptx->hdmi_cfg.rate;
+ /*
+ * Invalidate current clock rate to ensure rk_hdptx_phy_clk_set_rate()
+ * will be invoked to commit PLL configuration.
+ */
+ if (hdptx->pll_config_dirty) {
+ req->rate = 0;
+ return 0;
+ }
+
+ if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL) {
+ req->rate = hdptx->hdmi_cfg.rate;
+ return 0;
+ }
/*
* FIXME: Temporarily allow altering TMDS char rate via CCF.
@@ -2336,17 +2358,6 @@ static int rk_hdptx_phy_clk_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
- unsigned long long link_rate = rate;
-
- if (hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL)
- link_rate = DIV_ROUND_CLOSEST_ULL(rate * hdptx->hdmi_cfg.bpc, 8);
-
- /* Revert any unlikely link rate change since determine_rate() */
- if (hdptx->hdmi_cfg.rate != link_rate) {
- dev_warn(hdptx->dev, "Reverting unexpected rate change from %llu to %llu\n",
- link_rate, hdptx->hdmi_cfg.rate);
- hdptx->hdmi_cfg.rate = link_rate;
- }
/*
* The link rate would be normally programmed in HW during
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/6] phy: rockchip: samsung-hdptx: Clock fixes and API transition cleanups
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
This series provides a set of bug fixes and cleanups for the Rockchip
Samsung HDPTX PHY driver.
The first part of the series (i.e. PATCH 1 & 2) addresses clock rate
calculation and synchronization issues. Specifically, it fixes edge
cases where the PHY PLL is pre-programmed by an external component (like
a bootloader) or when changing the color depth (bpc) while keeping the
modeline constant. Because the Common Clock Framework .set_rate()
callback might not be invoked if the pixel clock remains unchanged, this
previously led to out-of-sync states between CCF and the actual HDMI PHY
configuration.
The second part focuses on code cleanups and modernizing the register
access. Now that dw_hdmi_qp driver has fully switched to using
phy_configure(), we can drop the deprecated TMDS rate setup workarounds
and the restrict_rate_change flag logic. Finally, it refactors the
driver to consistently use standard bitfield macros.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v2:
- Collected Tested-by tags from Thomas and Simon
- Fixed a typo in commit description of patch 1
- Added a comment in patch 2 explaining why PLL config errors are
ignored for rk_hdptx_phy_consumer_get()
- Added a missed FIELD_GET conversion for lcpll_hw.pms_sdiv in patch 6
- Rebased onto latest phy/fixes
- Link to v1: https://lore.kernel.org/r/20260227-hdptx-clk-fixes-v1-0-f998f2762d0f@collabora.com
---
Cristian Ciocaltea (6):
phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc
phy: rockchip: samsung-hdptx: Handle uncommitted PHY config changes
phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround
phy: rockchip: samsung-hdptx: Drop restrict_rate_change handling
phy: rockchip: samsung-hdptx: Simplify GRF access with FIELD_PREP_WM16()
phy: rockchip: samsung-hdptx: Consistently use bitfield macros
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 216 ++++++++++------------
1 file changed, 95 insertions(+), 121 deletions(-)
---
base-commit: a4058c09dd6e28ec33316fd6eb45ddae4cab1f31
change-id: 20260227-hdptx-clk-fixes-47426632f862
^ permalink raw reply
* [PATCH v2 1/6] phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc
From: Cristian Ciocaltea @ 2026-05-11 18:21 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov
Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
Thomas Niederprüm, Simon Wright
In-Reply-To: <20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com>
The PHY PLL can be programmed by an external component, e.g. the
bootloader, just before the recalc_rate() callback is invoked during
devm_clk_hw_register() in the probe path.
Therefore rk_hdptx_phy_clk_recalc_rate() finds the PLL enabled and
attempts to compute the clock rate, while making use of the bpc value
from the HDMI PHY configuration, which always defaults to 8 because
phy_configure() was not run at that point. As a consequence, the
(re)calculated rate is incorrect when the actual bpc was higher than 8.
Do not rely on any of the hdmi_cfg members when computing the clock rate
and, instead, read the required input data (i.e. bpc), directly from the
hardware registers.
Fixes: 3481fc04d969 ("phy: rockchip: samsung-hdptx: Compute clk rate from PLL config")
Tested-by: Thomas Niederprüm <dubito@online.de>
Tested-by: Simon Wright <simon@symple.nz>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 2d973bc37f07..7fb1c22318bb 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -2168,7 +2168,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
struct lcpll_config lcpll_hw;
struct ropll_config ropll_hw;
u64 fout, sdm;
- u32 mode, val;
+ u32 mode, bpc, val;
int ret, i;
ret = regmap_read(hdptx->regmap, CMN_REG(0008), &mode);
@@ -2266,6 +2266,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
if (ret)
return 0;
ropll_hw.pms_sdiv = ((val & PLL_PCG_POSTDIV_SEL_MASK) >> 4) + 1;
+ bpc = (FIELD_GET(PLL_PCG_CLK_SEL_MASK, val) << 1) + 8;
fout = PLL_REF_CLK * ropll_hw.pms_mdiv;
if (ropll_hw.sdm_en) {
@@ -2280,7 +2281,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
fout = fout + sdm;
}
- return div_u64(fout * 2, ropll_hw.pms_sdiv * 10);
+ return div_u64(fout * 2 * 8, ropll_hw.pms_sdiv * 10 * bpc);
}
static unsigned long rk_hdptx_phy_clk_recalc_rate(struct clk_hw *hw,
@@ -2288,19 +2289,13 @@ static unsigned long rk_hdptx_phy_clk_recalc_rate(struct clk_hw *hw,
{
struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
u32 status;
- u64 rate;
int ret;
ret = regmap_read(hdptx->grf, GRF_HDPTX_CON0, &status);
if (ret || !(status & HDPTX_I_PLL_EN))
return 0;
- rate = rk_hdptx_phy_clk_calc_rate_from_pll_cfg(hdptx);
-
- if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
- return rate;
-
- return DIV_ROUND_CLOSEST_ULL(rate * 8, hdptx->hdmi_cfg.bpc);
+ return rk_hdptx_phy_clk_calc_rate_from_pll_cfg(hdptx);
}
static int rk_hdptx_phy_clk_determine_rate(struct clk_hw *hw,
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Andrew Lunn @ 2026-05-11 18:18 UTC (permalink / raw)
To: Mathieu Poirier
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: <CANLsYkz8HpM_8eS7DjN_jmYs4T7P9xY0jYmdiAx5WO=_1zvFpQ@mail.gmail.com>
> 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.
Andrew
^ permalink raw reply
* Re: [PATCH v2 14/15] dt-bindings: display/lvds-codec: add ti,sn65lvds93
From: Hugo Villeneuve @ 2026-05-11 15:44 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: krzk, robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
rfoss, Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
mripard, tzimmermann, airlied, simona, Frank.Li, s.hauer, kernel,
festevam, shawnguo, laurent.pinchart+renesas, antonin.godard,
devicetree, linux-kernel, dri-devel, imx, linux-arm-kernel,
Hugo Villeneuve, Krzysztof Kozlowski
In-Reply-To: <20260305180651.1827087-15-hugo@hugovil.com>
Hi,
On Thu, 5 Mar 2026 13:06:29 -0500
Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Add compatible string for TI SN65LVDS93. Similar to
> SN65LVDS83 but with an industrial temperature range.
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Now that this series landed in linux-next/master, except for this
patch, we now have an error since it is required:
https://lore.kernel.org/oe-kbuild-all/202605071909.lXKPelNA-lkp@intel.com/
Hugo.
> ---
> Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index 4f52e35d02537..f2cb74b86cc05 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -37,6 +37,7 @@ properties:
> - ti,ds90c185 # For the TI DS90C185 FPD-Link Serializer
> - ti,ds90c187 # For the TI DS90C187 FPD-Link Serializer
> - ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter
> + - ti,sn75lvds93 # For the TI SN75LVDS93 FlatLink transmitter
> - const: lvds-encoder # Generic LVDS encoder compatible fallback
> - items:
> - enum:
> --
> 2.47.3
>
>
Hugo Villeneuve <hugo@hugovil.com>
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Mathieu Poirier @ 2026-05-11 17:58 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: <08c815f5-ccbe-404a-95a1-a549aac8a55d@amd.com>
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.
> 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] Documentation: KVM: Document guest-visible compatibility expectations
From: David Woodhouse @ 2026-05-11 17:53 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, kvm, linux-doc,
linux-kernel, Sean Christopherson, Jim Mattson, Marc Zyngier
Cc: Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Raghavendra Rao Ananta, Eric Auger,
Kees Cook, Arnd Bergmann, Nathan Chancellor, linux-arm-kernel,
kvmarm, linux-kselftest
In-Reply-To: <baff82ca-6321-4b16-aa61-b2d6d60b6535@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]
On Mon, 2026-05-11 at 18:56 +0200, Paolo Bonzini wrote:
> On 5/11/26 18:38, David Woodhouse wrote:
> > Not *everything* is in CPUID; one recent exception that comes to mind
> > is the SUPPRESS_EOI_BROADCAST quirk. But on x86 we preserve the
> > existing behaviour of older kernels — even when that behaviour doesn't
> > make much sense, as with SUPPRESS_EOI_BROADCAST where older KVM would
> > *advertise* the feature, but not actually *implement* it. Nevertheless,
> > that remains the default behaviour of future kernels unless userspace
> > explicitly opts in to fully enable (or disable) the feature.
> >
> > But this documentation update isn't even asking for that compatible-by-
> > default behaviour, even though that is the right thing to do. It's only
> > asking that it be *possible* to reinstate the old behaviour, for
> > userspace that *knows* about the change and explicitly wants to go back
> > to the old way to remain compatible.
>
> Yep, these are the "quirks"---if it's too early for Arm to commit to
> that, I guess it's fine.
>
> However, independent of this patch which I (obviously) believe is a good
> idea, I'd like to understand how far it is, assuming 1) no quirks 2)
> same CPU host.
It generally works out on arm64, although it's obviously a lot more
work than x86 which makes an effort to get this stuff right.
When we upgrade the kernel we do a lot of in-guest testing to find the
stuff that "broke", like cache reporting:
https://lore.kernel.org/all/254ca48a67779ccf9b9f60e2bb5796a305c03f95.camel@infradead.org/
... and the GICD_IIDR thing which I reposted today:
https://lore.kernel.org/all/20260511113558.3325004-2-dwmw2@infradead.org/
Those are the ones I came up against recently because someone had just
*reverted* the offending commits local in a previous kernel upgrade,
and I'm trying to fix it *properly* this time around and not carry the
reverts forward for ever.
And fix the expectations too, of course. Being told that we shouldn't
*expect* to be able to upgrade and roll back the kernel while remaining
compatible is... not OK.
> By the way, you didn't Cc Marc...
Ah crap, I meant to. Thanks for spotting that!
I must have screwed up when I combined and dedeuplicated the
get_maintainer.pl output with the recipients of the IIDR patch series.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* [PATCH] thermal: imx: do not split quoted string across lines
From: Mayur Kumar @ 2026-05-11 17:42 UTC (permalink / raw)
To: rafael, daniel.lezcano, Frank.Li, s.hauer, rui.zhang, lukasz.luba,
festevam
Cc: linux-pm, imx, linux-arm-kernel, linux-kernel, kernel,
Mayur Kumar
The checkpatch tool warns against splitting quoted strings across
multiple lines. Join the dev_info message into a single line to
improve the ability to grep for the message in the source.
Signed-off-by: Mayur Kumar <kmayur809@gmail.com>
---
drivers/thermal/imx_thermal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 38c993d1bcb..5aaacbc5347 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -693,8 +693,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
goto clk_disable;
}
- dev_info(dev, "%s CPU temperature grade - max:%dC"
- " critical:%dC passive:%dC\n", data->temp_grade,
+ dev_info(dev, "%s CPU temperature grade - max:%dC critical:%dC passive:%dC\n",
+ data->temp_grade,
data->temp_max / 1000, trips[IMX_TRIP_CRITICAL].temperature / 1000,
trips[IMX_TRIP_PASSIVE].temperature / 1000);
--
2.34.1
^ permalink raw reply related
* [PATCH v7 1/3] Revert "treewide: Fix probing of devices in DT overlays"
From: Herve Codina @ 2026-05-11 15:57 UTC (permalink / raw)
To: Andrew Lunn, Rob Herring, Saravana Kannan, Geert Uytterhoeven,
Kalle Niemi, Matti Vaittinen, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Wolfram Sang, Mark Brown,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Len Brown, Shawn Guo
Cc: Wolfram Sang, driver-core, linux-kernel, imx, linux-arm-kernel,
linux-i2c, devicetree, linux-spi, linux-acpi, Allan Nielsen,
Horatiu Vultur, Daniel Machon, Steen Hegelund, Luca Ceresoli,
Thomas Petazzoni, Saravana Kannan, Herve Codina
In-Reply-To: <20260511155755.34428-1-herve.codina@bootlin.com>
From: Saravana Kannan <saravanak@google.com>
This reverts commit 1a50d9403fb90cbe4dea0ec9fd0351d2ecbd8924.
While the commit fixed fw_devlink overlay handling for one case, it
broke it for another case. So revert it and redo the fix in a separate
patch.
Fixes: 1a50d9403fb9 ("treewide: Fix probing of devices in DT overlays")
Reported-by: Herve Codina <herve.codina@bootlin.com>
Closes: https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/
Closes: https://lore.kernel.org/all/20240221095137.616d2aaa@bootlin.com/
Closes: https://lore.kernel.org/lkml/20240312151835.29ef62a0@bootlin.com/
Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/lkml/20240411235623.1260061-2-saravanak@google.com/
[Herve: Fix conflicts due to f72e77c33e4b ("device property: Make
modifications of fwnode "flags" thread safe")]
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Mark Brown <broonie@kernel.org>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
drivers/bus/imx-weim.c | 6 ------
drivers/i2c/i2c-core-of.c | 5 -----
drivers/of/dynamic.c | 1 -
drivers/of/platform.c | 5 -----
drivers/spi/spi.c | 5 -----
5 files changed, 22 deletions(-)
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index f735e0462c55..87070155b057 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -327,12 +327,6 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
"Failed to setup timing for '%pOF'\n", rd->dn);
if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
- /*
- * Clear the flag before adding the device so that
- * fw_devlink doesn't skip adding consumers to this
- * device.
- */
- fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
dev_err(&pdev->dev,
"Failed to create child device '%pOF'\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 354a88d0599e..30b48a428c0b 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -176,11 +176,6 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
return NOTIFY_OK;
}
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
client = of_i2c_register_device(adap, rd->dn);
if (IS_ERR(client)) {
dev_err(&adap->dev, "failed to create client for '%pOF'\n",
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index ade288372101..aa450425ec1e 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -225,7 +225,6 @@ static void __of_attach_node(struct device_node *np)
np->sibling = np->parent->child;
np->parent->child = np;
of_node_clear_flag(np, OF_DETACHED);
- fwnode_set_flag(&np->fwnode, FWNODE_FLAG_NOT_DEVICE);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index a42224f9d1a8..53bca8c6f781 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -744,11 +744,6 @@ static int of_platform_notify(struct notifier_block *nb,
if (of_node_check_flag(rd->dn, OF_POPULATED))
return NOTIFY_OK;
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
/* pdev_parent may be NULL when no bus platform device */
pdev_parent = of_find_device_by_node(parent);
pdev = of_platform_device_create(rd->dn, NULL,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 104279858f56..889e1eecc757 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -5003,11 +5003,6 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
return NOTIFY_OK;
}
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
spi = of_register_spi_device(ctlr, rd->dn);
put_device(&ctlr->dev);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] cpu/hotplug: Fix NULL kobject warning in cpuhp_smt_enable()
From: Catalin Marinas @ 2026-05-11 17:37 UTC (permalink / raw)
To: Jinjie Ruan
Cc: will, punit.agrawal, rafael.j.wysocki, fengchengwen, chenl311,
suzuki.poulose, maz, timothy.hayes, lpieralisi, mrigendra.chaubey,
arnd, sudeep.holla, yangyicong, jic23, pierre.gondois,
linux-arm-kernel, linux-kernel, james.morse
In-Reply-To: <20260427023507.1247418-1-ruanjinjie@huawei.com>
On Mon, Apr 27, 2026 at 10:35:07AM +0800, Jinjie Ruan wrote:
> On arm64, when booting with `maxcpus` greater than the number of present
> CPUs (e.g., QEMU -smp cpus=4,maxcpus=8), some CPUs are marked as 'present'
> but have not yet been registered via register_cpu(). Consequently,
> the per-cpu device objects for these CPUs are not yet initialized.
[...]
> Fix this by:
>
> 1. When booting with ACPI, checking the ACPI_MADT_ENABLED flag in the GICC
> entry before calling set_cpu_present() during SMP initialization.
>
> 2. Properly managing the present mask in acpi_map_cpu() and
> acpi_unmap_cpu() to support actual CPU hotplug events, This aligns with
> other architectures like x86 and LoongArch.
I had a chat with James earlier and IIUC the decision was to mark all
CPUs present and the GIC must be fully initialised. But digging through
the GICv3 code, I don't see it depending on cpu_present_mask but rather
on the "always on" MADT GICR description. So I think it should be safe
as long as we don't rely on the GICC gicr_base_address. But we should
update Documentation/arch/arm64/cpu-hotplug.rst to no longer state that
all online-capable vCPUs are marked as present by the kernel.
(or maybe I misunderstood all this)
--
Catalin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox