* [PATCH v5 10/10] drm/rockchip: analogix_dp: Add support for RK3576
From: Damon Ding @ 2026-05-13 7:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
In-Reply-To: <20260513074414.2053435-1-damon.ding@rock-chips.com>
RK3576 integrates Analogix eDP 1.3 TX and Samsung combo PHY
hardware blocks that fully match the proven RK3588 design.
Add dedicated chip data table and device tree matching entry
to bring up basic eDP functionality for the RK3576 platform.
Support is limited to RGB output up to 4K@60Hz for now; audio,
PSR and other advanced eDP 1.3 features remain unvalidated.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Split out a separate patch to enable the "hclk" clock.
- Add Reviewed-by tag.
Changes in v3:
- Add Reviewed-by tag.
Changes in v4:
- Modify the commit msg.
---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d2af5eb29dbb..d4c5dd61e95b 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -522,6 +522,14 @@ static const struct rockchip_dp_chip_data rk3288_dp[] = {
{ /* sentinel */ }
};
+static const struct rockchip_dp_chip_data rk3576_edp[] = {
+ {
+ .chip_type = RK3576_EDP,
+ .reg = 0x27dc0000,
+ },
+ { /* sentinel */ }
+};
+
static const struct rockchip_dp_chip_data rk3588_edp[] = {
{
.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
@@ -539,6 +547,7 @@ static const struct rockchip_dp_chip_data rk3588_edp[] = {
static const struct of_device_id rockchip_dp_dt_ids[] = {
{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
+ {.compatible = "rockchip,rk3576-edp", .data = &rk3576_edp },
{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
{}
};
--
2.34.1
^ permalink raw reply related
* [PATCH v5 07/10] arm64: dts: rockchip: Add eDP node for RK3576
From: Damon Ding @ 2026-05-13 7:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
In-Reply-To: <20260513074414.2053435-1-damon.ding@rock-chips.com>
Add full device tree definition for the integrated eDP controller
on RK3576, following the existing RK3588 hardware layout.
Configure required register range, clocks, interrupt, phy, power
domain, reset and grf properties to fully describe the controller.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v2:
- Add Reviewed-by tag.
Changes in v4:
- Modify the commit msg.
---
arch/arm64/boot/dts/rockchip/rk3576.dtsi | 28 ++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index 28175d8200d5..733449cb88b1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1496,6 +1496,34 @@ hdmi_out: port@1 {
};
};
+ edp: edp@27dc0000 {
+ compatible = "rockchip,rk3576-edp";
+ reg = <0x0 0x27dc0000 0x0 0x1000>;
+ clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru HCLK_VO0_ROOT>;
+ clock-names = "dp", "pclk", "hclk";
+ interrupts = <GIC_SPI 365 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&hdptxphy>;
+ phy-names = "dp";
+ power-domains = <&power RK3576_PD_VO0>;
+ resets = <&cru SRST_EDP0_24M>, <&cru SRST_P_EDP0>;
+ reset-names = "dp", "apb";
+ rockchip,grf = <&vo0_grf>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ edp_in: port@0 {
+ reg = <0>;
+ };
+
+ edp_out: port@1 {
+ reg = <1>;
+ };
+ };
+ };
+
dp: dp@27e40000 {
compatible = "rockchip,rk3576-dp";
reg = <0x0 0x27e40000 0x0 0x30000>;
--
2.34.1
^ permalink raw reply related
* [PATCH v5 06/10] dt-bindings: display: rockchip: analogix-dp: Add support for RK3576
From: Damon Ding @ 2026-05-13 7:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding,
Conor Dooley
In-Reply-To: <20260513074414.2053435-1-damon.ding@rock-chips.com>
RK3576 integrates an eDP TX controller compatible with the existing
RK3588 hardware design, reuse the same binding configuration directly.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
Changes in v2:
- Split out a separate patch to add the "hclk" clock reference.
Chanegs in v4:
- Modify the commit msg.
Changes in v5:
- Add Acked-by tag.
---
.../bindings/display/rockchip/rockchip,analogix-dp.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index d679da70947d..bb75d898a5c5 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -15,6 +15,7 @@ properties:
enum:
- rockchip,rk3288-dp
- rockchip,rk3399-edp
+ - rockchip,rk3576-edp
- rockchip,rk3588-edp
clocks:
@@ -92,6 +93,7 @@ allOf:
compatible:
contains:
enum:
+ - rockchip,rk3576-edp
- rockchip,rk3588-edp
then:
properties:
--
2.34.1
^ permalink raw reply related
* [PATCH v5 03/10] arm64: dts: rockchip: Add missing hclk for RK3588 eDP0
From: Damon Ding @ 2026-05-13 7:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
In-Reply-To: <20260513074414.2053435-1-damon.ding@rock-chips.com>
Add the required HCLK_VO1 bus clock to RK3588 eDP0 node with
corresponding clock-name "hclk". This clock is necessary for the
eDP controller to access video output GRF and work properly.
Previously the clock was enabled implicitly via GRF phandle
reference. Add it explicitly now to align with updated binding.
Fixes: dc79d3d5e7c7 ("arm64: dts: rockchip: Add eDP0 node for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
Changes in v4:
- Modify the commit msg.
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 4fb8888c281c..24a5ccbac08c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1712,8 +1712,8 @@ hdmi0_out: port@1 {
edp0: edp@fdec0000 {
compatible = "rockchip,rk3588-edp";
reg = <0x0 0xfdec0000 0x0 0x1000>;
- clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>;
- clock-names = "dp", "pclk";
+ clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru HCLK_VO1>;
+ clock-names = "dp", "pclk", "hclk";
interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH 0>;
phys = <&hdptxphy0>;
phy-names = "dp";
--
2.34.1
^ permalink raw reply related
* [PATCH v5 02/10] dt-bindings: display: rockchip: analogix-dp: Add per-clock descriptions
From: Damon Ding @ 2026-05-13 7:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding,
Conor Dooley
In-Reply-To: <20260513074414.2053435-1-damon.ding@rock-chips.com>
Supplement dedicated description for each clock in the clocks
property, clarifying the function of each clock input for the
Analogix DP controller binding.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
Changes in v4:
- Modify the commit msg.
Changes in v5:
- Add Acked-by tag.
---
.../bindings/display/rockchip/rockchip,analogix-dp.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index 8001c1facf98..d679da70947d 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -19,7 +19,10 @@ properties:
clocks:
minItems: 2
- maxItems: 3
+ items:
+ - description: Reference clock
+ - description: APB bus clock
+ - description: GRF or AHB bus clock
clock-names:
minItems: 2
--
2.34.1
^ permalink raw reply related
* RE: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery
From: Hongxing Zhu @ 2026-05-13 7:41 UTC (permalink / raw)
To: Bough Chen, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, bhelgaas@google.com, Frank Li,
l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com
Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-kernel@vger.kernel.org
In-Reply-To: <GV2PR04MB122726C1150FFA1D8B0125B3890062@GV2PR04MB12272.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Bough Chen <haibo.chen@nxp.com>
> Sent: Wednesday, May 13, 2026 11:32 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; bhelgaas@google.com; Frank Li
> <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>
> Subject: RE: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link
> recovery
>
> > -----Original Message-----
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> > Sent: 2026年5月13日 10:51
> > To: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > bhelgaas@google.com; Frank Li <frank.li@nxp.com>;
> > l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> > mani@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com
> > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > devicetree@vger.kernel.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org; Hongxing Zhu <hongxing.zhu@nxp.com>
> > Subject: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link
> > recovery
> >
> > The PCIe link can go down due to various unexpected circumstances. Add
> > root port reset support to enable link recovery for the i.MX PCIe
> > controller when the optional "intr" interrupt is present.
> >
> > Reset root port to uninitialize, initialize the PCIe controller, and
> > restart the PCIe link at end when a link down event happens.
> >
> > On i.MX95 platforms, link events and PME share the same interrupt line.
> > The link event interrupt cannot use a threaded-only IRQ handler
> > because the PME driver uses request_irq() with only the IRQF_SHARED
> > flag set, which requires a primary handler.
> >
> > To handle this shared interrupt scenario, register a primary interrupt
> > handler with IRQF_SHARED for link events and manipulate the link event
> > enable bits to ensure the shared interrupt source triggers only one handler at a
> time.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 123
> > ++++++++++++++++++++++++++
> > 1 file changed, 123 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 1034ac5c5f5c..79c92c77b85b 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -34,6 +34,7 @@
> > #include <linux/pm_runtime.h>
> >
> > #include "../../pci.h"
> > +#include "../pci-host-common.h"
> > #include "pcie-designware.h"
> >
> > #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
> > @@ -78,6 +79,10 @@
> > #define IMX95_SID_MASK GENMASK(5, 0)
> > #define IMX95_MAX_LUT 32
> >
> > +#define IMX95_LINK_INT_CTRL_STS 0x1040
> > +#define IMX95_LINK_DOWN_INT_STS BIT(11)
> > +#define IMX95_LINK_DOWN_INT_EN BIT(10)
> > +
> > #define IMX95_PCIE_RST_CTRL 0x3010
> > #define IMX95_PCIE_COLD_RST BIT(0)
> >
> > @@ -125,6 +130,8 @@ enum imx_pcie_variants {
> > #define IMX_PCIE_MAX_INSTANCES 2
> >
> > struct imx_pcie;
> > +static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
> > + struct pci_dev *pdev);
> >
> > struct imx_pcie_drvdata {
> > enum imx_pcie_variants variant;
> > @@ -158,6 +165,7 @@ struct imx_pcie {
> > bool supports_clkreq;
> > bool enable_ext_refclk;
> > struct regmap *iomuxc_gpr;
> > + u32 lnk_intr;
> > u16 msi_ctrl;
> > u32 controller_id;
> > struct reset_control *pciephy_reset;
> > @@ -1301,6 +1309,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> > *pp)
> >
> > imx_setup_phy_mpll(imx_pcie);
> >
> > + /*
> > + * Callback invoked by PCI core when link down is detected and
> > + * recovery is needed.
> > + */
> > + if (pp->bridge)
> > + pp->bridge->reset_root_port = imx_pcie_reset_root_port;
> > +
> > return 0;
> >
> > err_phy_off:
> > @@ -1568,6 +1583,9 @@ static int imx_pcie_suspend_noirq(struct device
> > *dev)
> > if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
> > return 0;
> >
> > + if (imx_pcie->lnk_intr)
> > + regmap_clear_bits(imx_pcie->iomuxc_gpr,
> > IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
> > imx_pcie_msi_save_restore(imx_pcie, true);
> > if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > imx_pcie_lut_save(imx_pcie);
> > @@ -1618,6 +1636,9 @@ static int imx_pcie_resume_noirq(struct device *dev)
> > if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > imx_pcie_lut_restore(imx_pcie);
> > imx_pcie_msi_save_restore(imx_pcie, false);
> > + if (imx_pcie->lnk_intr)
> > + regmap_set_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
> >
> > return 0;
> > }
> > @@ -1627,6 +1648,84 @@ static const struct dev_pm_ops imx_pcie_pm_ops
> > = {
> > imx_pcie_resume_noirq)
> > };
> >
> > +static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv) {
> > + struct imx_pcie *imx_pcie = priv;
> > + struct dw_pcie *pci = imx_pcie->pci;
> > + struct device *dev = pci->dev;
> > + u32 val;
> > +
> > + regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
> > + if (val & IMX95_LINK_DOWN_INT_STS) {
> > + dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
> > + regmap_clear_bits(imx_pcie->iomuxc_gpr,
> > IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
> > + regmap_set_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_STS);
>
> Hi Richard
>
> Better to add comment here to point out that write the
> IMX95_LINK_DOWN_INT_STS means clear this bit, or mention this bit is W1C.
Okay, would add the comment later. Thanks.
Best Regards
Richard Zhu
>
> Regards
> Haibo Chen
^ permalink raw reply
* Re: [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support
From: Linus Walleij @ 2026-05-13 7:40 UTC (permalink / raw)
To: Yu-Chun Lin
Cc: brgl, robh, krzk+dt, conor+dt, afaerber, wbg,
mathieu.dubois-briand, mwalle, lars, Michael.Hennerich, jic23,
nuno.sa, andy, dlechner, tychang, linux-gpio, devicetree,
linux-kernel, linux-arm-kernel, linux-realtek-soc, linux-iio,
cy.huang, stanley_chang, james.tai, Linus Walleij
In-Reply-To: <20260512033317.1602537-4-eleanor.lin@realtek.com>
Hi Yu-Chun,
thanks for your patch! I really like the direction this is taking.
On Tue, May 12, 2026 at 5:33 AM Yu-Chun Lin <eleanor.lin@realtek.com> wrote:
> Extend the reg_mask_xlate callback with an operation type parameter
> (gpio_regmap_operation) to allow drivers to return different
> register/mask combinations for different GPIO operations.
>
> Also add write-enable mechanism for hardware that requires setting a
> write-enable bit before modifying GPIO control registers.
>
> Consequently, update all existing drivers utilizing the gpio-regmap
> framework (across drivers/gpio, drivers/iio, and drivers/pinctrl)
> to accommodate the new reg_mask_xlate function signature.
>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
linusw@kernel.org these days.
> Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
The rest of the comments I had Andy, Jonathan and Sashiko has
already pointed out, just hash through it and this will look really
nice in the end.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables
From: Arnaud POULIQUEN @ 2026-05-13 7:37 UTC (permalink / raw)
To: Daniel Baluta, tanmay.shah, 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
In-Reply-To: <3b1c5579-a9de-4955-ad3c-df3f8c1860f4@oss.nxp.com>
On 5/13/26 08:30, Daniel Baluta wrote:
> On 5/12/26 17:53, Shah, Tanmay wrote:
>>
>>
>> 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.
>
> Agree, this is the best way to go.
>
>
LGTM
If you keep the rproc_elf_load_rsc_table_optional() helper, I would
suggest inverting the logic for dev_dbg(). Regarding the discussion, it
seems more logical to print a message when a resource table is found.
An add-on could be to also print the address and size found.
Thanks,
Arnaud
^ permalink raw reply
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Hans Zhang @ 2026-05-13 7:34 UTC (permalink / raw)
To: Pali Rohár
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <20260513072008.vol4htgbzquly2rb@pali>
On 5/13/26 15:20, Pali Rohár wrote:
> On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
>>
>>
>> On 5/13/26 05:25, Pali Rohár wrote:
>>> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>>>> The Aardvark PCIe controller driver waits for the link to come up but
>>>> does not implement the mandatory 100 ms delay after link training
>>>> completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>>>
>>>> The driver already maintains a 'link_gen' field that holds the negotiated
>>>> link speed. Use it together with pcie_wait_after_link_train() to insert
>>>> the required delay immediately after confirming that the link is up.
>>>>
>>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>>> ---
>>>> drivers/pci/controller/pci-aardvark.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>>>> index e34bea1ff0ac..526351c21c49 100644
>>>> --- a/drivers/pci/controller/pci-aardvark.c
>>>> +++ b/drivers/pci/controller/pci-aardvark.c
>>>> @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
>>>> /* check if the link is up or not */
>>>> for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
>>>> - if (advk_pcie_link_up(pcie))
>>>> + if (advk_pcie_link_up(pcie)) {
>>>> + pcie_wait_after_link_train(pcie->link_gen);
>>>> return 0;
>>>> + }
>>>> usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>>>> }
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> Are you sure that this is correct to do? Have you checked the A3720
>>> Functional Specification which describes how to bring PCIe link up?
>>>
>>> A3720 PCIe controller is buggy and needs more timing hacks to make it
>>> behave. Playing with random sleeps can break its internal logic.
>>> I'm not sure if it could be safe without proper testing.
>>>
>>> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
>>
>>
>> Hi Pali,
>>
>> 1. This driver does not support A3720.
>>
>> static const struct of_device_id advk_pcie_of_match_table[] = {
>> { .compatible = "marvell,armada-3700-pcie", },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
>>
>> If you need support for A3720, please submit the corresponding patch so that
>> Bjorn and Mani can review it.
>
> 3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
> a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
>
>>
>> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
>> in the DT. This will not affect the functionality of this patch.
>
> Whole A37xx supports only GEN2. And in DT files for 37xx should be
> already there max-link-speed.
>
> Seems that in advk_pcie_of_match_table there is no GEN3 device
> specified.
>
Hi Pali,
However, I saw many GEN3 assignments and conditions in the code.
ret = of_pci_get_max_link_speed(dev->of_node);
if (ret <= 0 || ret > 3)
pcie->link_gen = 3;
else
pcie->link_gen = ret;
static void advk_pcie_train_link(struct advk_pcie *pcie)
{
struct device *dev = &pcie->pdev->dev;
u32 reg;
int ret;
/*
* Setup PCIe rev / gen compliance based on device tree property
* 'max-link-speed' which also forces maximal link speed.
*/
reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
reg &= ~PCIE_GEN_SEL_MSK;
if (pcie->link_gen == 3)
reg |= SPEED_GEN_3;
else if (pcie->link_gen == 2)
reg |= SPEED_GEN_2;
else
reg |= SPEED_GEN_1;
advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
/*
* Set maximal link speed value also into PCIe Link Control 2 register.
* Armada 3700 Functional Specification says that default value is based
* on SPEED_GEN but tests showed that default value is always 8.0 GT/s.
*/
reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
reg &= ~PCI_EXP_LNKCTL2_TLS;
if (pcie->link_gen == 3)
reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
else if (pcie->link_gen == 2)
reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
else
reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
....
If you are certain about the relevant information. Is it understandable
that we need to delete the code related to GEN3?
Best regards,
Hans
>> 3. This patch is a common delay requirement stipulated by the PCIe
>> specification. If it is greater than GEN2, then msleep(100) will be added;
>> otherwise, there will be no such delay.
>>
>> 4. For instance, we often come across the situation where some common APIs
>> are modified, and in many cases, their functionality does not require the
>> actual development board for verification. I believe that many other
>> developers and maintainers have modified different parts of the code. For
>> example, the recent submission:
>
> Switching one API to another is one thing. But changing code which looks
> to be critical, specially when it is known that hw has bugs, can cause
> breaking of existing boards.
>
>> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
>> Author: Nam Cao <namcao@linutronix.de>
>> Date: Thu Jun 26 16:47:53 2025 +0200
>>
>> PCI: aardvark: Switch to msi_create_parent_irq_domain()
>>
>> Switch to msi_create_parent_irq_domain() from
>> pci_msi_create_irq_domain()
>> which was using legacy MSI domain setup.
>>
>>
>> And many controller drivers have been modified.
>>
>>
>> Best regards,
>> Hans
>>
>>
^ permalink raw reply
* Re: [PATCH v7 07/20] KVM: arm64: Set up FGT for Partitioned PMU
From: Oliver Upton @ 2026-05-13 7:34 UTC (permalink / raw)
To: Colton Lewis
Cc: kvm, Alexandru Elisei, Paolo Bonzini, Jonathan Corbet,
Russell King, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mingwei Zhang, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Mark Rutland, Shuah Khan, Ganapatrao Kulkarni,
James Clark, linux-doc, linux-kernel, linux-arm-kernel, kvmarm,
linux-perf-users, linux-kselftest
In-Reply-To: <20260504211813.1804997-8-coltonlewis@google.com>
On Mon, May 04, 2026 at 09:18:00PM +0000, Colton Lewis wrote:
> +static void __compute_hdfgrtr(struct kvm_vcpu *vcpu)
> +{
> + __compute_fgt(vcpu, HDFGRTR_EL2);
> +
> + *vcpu_fgt(vcpu, HDFGRTR_EL2) |=
> + HDFGRTR_EL2_PMOVS
> + | HDFGRTR_EL2_PMCCFILTR_EL0
> + | HDFGRTR_EL2_PMEVTYPERn_EL0
> + | HDFGRTR_EL2_PMCEIDn_EL0
> + | HDFGRTR_EL2_PMMIR_EL1;
> +}
> +
I've given this feedback at least twice already...
Operators go on the preceding line in the case of line continuations.
> +
> +/**
> + * kvm_pmu_is_partitioned() - Determine if given PMU is partitioned
> + * @pmu: Pointer to arm_pmu struct
> + *
> + * Determine if given PMU is partitioned by looking at hpmn field. The
> + * PMU is partitioned if this field is less than the number of
> + * counters in the system.
> + *
> + * Return: True if the PMU is partitioned, false otherwise
> + */
> +bool kvm_pmu_is_partitioned(struct arm_pmu *pmu)
> +{
> + if (!pmu)
> + return false;
> +
> + return pmu->max_guest_counters >= 0 &&
> + pmu->max_guest_counters <= *host_data_ptr(nr_event_counters);
> +}
> +
> +/**
> + * kvm_vcpu_pmu_is_partitioned() - Determine if given VCPU has a partitioned PMU
> + * @vcpu: Pointer to kvm_vcpu struct
> + *
> + * Determine if given VCPU has a partitioned PMU by extracting that
> + * field and passing it to :c:func:`kvm_pmu_is_partitioned`
> + *
> + * Return: True if the VCPU PMU is partitioned, false otherwise
> + */
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> + return kvm_pmu_is_partitioned(vcpu->kvm->arch.arm_pmu) &&
> + false;
> +}
Ok, I'm thoroughly confused about these predicates.
Whether or not a vCPU is using a partitioned PMU is a per-VM property.
This is separate from whether or not the backing arm_pmu has a range of
available counters for the guest to use.
It is entirely possible that a VM *isn't* using the partitioned PMU
feature (i.e. backed with perf events) yet the supporting arm_pmu has a
guest counter range.
> +#if !defined(__KVM_NVHE_HYPERVISOR__)
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu);
> +bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu);
> +#else
> +static inline bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +static inline bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +#endif
> +
Don't use ifdeffery for this. Aim to have a single definition and rely
on has_vhe() to do the rest of the work.
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH v4 0/3] gpio: Add EIO GPIO support
From: Bartosz Golaszewski @ 2026-05-13 7:30 UTC (permalink / raw)
To: linux-kernel, Shubhrajyoti Datta
Cc: Bartosz Golaszewski, git, shubhrajyoti.datta, Srinivas Neeli,
Michal Simek, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-arm-kernel
In-Reply-To: <20260512060917.2096456-1-shubhrajyoti.datta@amd.com>
On Tue, 12 May 2026 11:38:46 +0530, Shubhrajyoti Datta wrote:
> Add the EIO GPIO support.
> Add the dt description and the compatible to the driver.
>
> Changes in v4:
> - Add Conor ack
> - Remove the min 52 for eio
>
> [...]
Applied, thanks!
[1/3] dt-bindings: gpio: zynq: Sort compatible strings alphabetically
https://git.kernel.org/brgl/c/3eb639ef8da2d418ae69f3c8840c4e815036adc6
[2/3] dt-bindings: gpio: Add EIO GPIO compatible to gpio-zynq
https://git.kernel.org/brgl/c/18409d06b4a002cb8550ad7c20273bedc77851df
[3/3] gpio: zynq: Add eio gpio support
https://git.kernel.org/brgl/c/eeb1d6dfd89344b17afe845d4839b79e37fdd547
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 08/10] clk: amlogic: Add A9 PLL clock controller driver
From: Jian Hu @ 2026-05-13 7:25 UTC (permalink / raw)
To: Brian Masney
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jerome Brunet, Xianwei Zhao,
Kevin Hilman, Martin Blumenstingl, linux-kernel, linux-clk,
devicetree, linux-amlogic, linux-arm-kernel
In-Reply-To: <agH3f3F0pUNOzdPB@redhat.com>
On 5/11/2026 11:36 PM, Brian Masney wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Jian,
>
> On Mon, May 11, 2026 at 08:47:30PM +0800, Jian Hu via B4 Relay wrote:
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the PLL clock controller driver for the Amlogic A9 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>> drivers/clk/meson/Kconfig | 13 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/a9-pll.c | 831 +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 845 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index cf8cf3f9e4ee..3549e67d6988 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>> controller to work.
>>
>> +config COMMON_CLK_A9_PLL
>> + tristate "Amlogic A9 SoC PLL controller support"
>> + depends on ARM64
> depends on ARM64 || COMPILE_TEST
Ok, I will add COMPILE_TEST in the next version.
>> + default ARCH_MESON
>> + select COMMON_CLK_MESON_REGMAP
>> + select COMMON_CLK_MESON_CLKC_UTILS
>> + select COMMON_CLK_MESON_PLL
>> + imply COMMON_CLK_SCMI
>> + help
>> + Support for the PLL clock controller on Amlogic A311Y3 based
>> + device, AKA A9. PLLs are required by most peripheral to operate.
>> + Say Y if you want A9 PLL clock controller to work.
>> +
>> config COMMON_CLK_C3_PLL
>> tristate "Amlogic C3 PLL clock controller"
>> depends on ARM64
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c6719694a242..77636033061f 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> +obj-$(CONFIG_COMMON_CLK_A9_PLL) += a9-pll.o
>> obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>> obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>> diff --git a/drivers/clk/meson/a9-pll.c b/drivers/clk/meson/a9-pll.c
>> new file mode 100644
>> index 000000000000..84b591c3afff
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-pll.c
>> @@ -0,0 +1,831 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
>> +#include "clk-regmap.h"
>> +#include "clk-pll.h"
>> +#include "meson-clkc-utils.h"
> Sort the headers
Ok , I will place dt-bindings header at the top.
After updated:
#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
#include <linux/clk-provider.h>
#include <linux/platform_device.h>
#include "clk-regmap.h"
#include "clk-pll.h"
#include "meson-clkc-utils.h"
If I have misunderstood, please correct me.
>> +
>> +#define GP0PLL_CTRL0 0x00
>> +#define GP0PLL_CTRL1 0x04
>> +#define GP0PLL_CTRL2 0x08
>> +#define GP0PLL_CTRL3 0x0c
>> +#define GP0PLL_CTRL4 0x10
>> +
>> +/* HIFI0 and HIFI1 share the same IP and register offset layout. */
>> +#define HIFIPLL_CTRL0 0x00
>> +#define HIFIPLL_CTRL1 0x04
>> +#define HIFIPLL_CTRL2 0x08
>> +#define HIFIPLL_CTRL3 0x0c
>> +#define HIFIPLL_CTRL4 0x10
>> +
>> +/* MCLK0 and MCLK1 share the same IP and register offset layout. */
>> +#define MCLKPLL_CTRL0 0x00
>> +#define MCLKPLL_CTRL1 0x04
>> +#define MCLKPLL_CTRL2 0x08
>> +#define MCLKPLL_CTRL3 0x0c
>> +#define MCLKPLL_CTRL4 0x10
>> +
>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>> + MESON_COMP_SEL(a9_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>> +
>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>> + MESON_COMP_DIV(a9_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>> +
>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>> + MESON_COMP_GATE(a9_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>> +
>> +/*
>> + * Compared with previous SoC PLLs, the A9 PLL input path has an inherent
>> + * 2-divider. The N pre-divider follows the same calculation rule as OD,
>> + * where the pre-divider ratio equals 2^N.
>> + *
>> + * A9 PLL is composed as follows:
>> + *
>> + * PLL
>> + * +---------------------------------+
>> + * | |
>> + * | +--+ |
>> + * in/2 >>---[ /2^N ]-->| | +-----+ |
>> + * | | |------| DCO |----->> out
>> + * | +--------->| | +--v--+ |
>> + * | | +--+ | |
>> + * | | | |
>> + * | +--[ *(M + (F/Fmax) ]<--+ |
>> + * | |
>> + * +---------------------------------+
>> + *
>> + * out = in / 2 * (m + frac / frac_max) / 2^n
>> + */
>> +
>> +static struct clk_fixed_factor a9_gp0_in_div2_div = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "gp0_in_div2_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_data = &(const struct clk_parent_data) {
>> + .fw_name = "in0",
>> + },
>> + .num_parents = 1,
>> + },
> You can use CLK_HW_INIT_FW_NAME() for the hw.init here and other places
> below.
Ok, I will use CLK_HW_INIT_FW_NAME instead in the next version.
>> +};
>> +
>> +static struct clk_regmap a9_gp0_in_div2 = {
>> + .data = &(struct clk_regmap_gate_data) {
>> + .offset = GP0PLL_CTRL0,
>> + .bit_idx = 27,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "gp0_in_div2",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &a9_gp0_in_div2_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +/* The output frequency range of the A9 PLL_DCO is 1.4 GHz to 2.8 GHz. */
>> +static const struct pll_mult_range a9_pll_mult_range = {
>> + .min = 117,
>> + .max = 233,
>> +};
>> +
>> +static const struct reg_sequence a9_gp0_pll_init_regs[] = {
>> + { .reg = GP0PLL_CTRL0, .def = 0x00010000 },
>> + { .reg = GP0PLL_CTRL1, .def = 0x11480000 },
>> + { .reg = GP0PLL_CTRL2, .def = 0x1219b010 },
>> + { .reg = GP0PLL_CTRL3, .def = 0x00008010 }
>> +};
>> +
>> +static struct clk_regmap a9_gp0_pll_dco = {
>> + .data = &(struct meson_clk_pll_data) {
>> + .en = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 0,
>> + .width = 9,
>> + },
>> + .n = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 12,
>> + .width = 3,
>> + },
>> + .frac = {
>> + .reg_off = GP0PLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .l_detect = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .range = &a9_pll_mult_range,
>> + .init_regs = a9_gp0_pll_init_regs,
>> + .init_count = ARRAY_SIZE(a9_gp0_pll_init_regs),
>> + .flags = CLK_MESON_PLL_RST_ACTIVE_LOW |
>> + CLK_MESON_PLL_N_POWER_OF_TWO |
>> + CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "gp0_pll_dco",
>> + .ops = &meson_clk_pll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &a9_gp0_in_div2.hw
>> + },
>> + .num_parents = 1,
>> + },
> You can use CLK_HW_INIT_HWS() here and other places below.
>
> Brian
>
Ok, I will use CLK_HW_INIT_HW instead for single parent case.
Best regards,
Jian
[......]
>> 2.47.1
>>
>>
^ permalink raw reply
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Pali Rohár @ 2026-05-13 7:20 UTC (permalink / raw)
To: Hans Zhang
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <581e91fb-2e57-43ed-b79d-19dbf384b955@163.com>
On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
>
>
> On 5/13/26 05:25, Pali Rohár wrote:
> > On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
> > > The Aardvark PCIe controller driver waits for the link to come up but
> > > does not implement the mandatory 100 ms delay after link training
> > > completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
> > >
> > > The driver already maintains a 'link_gen' field that holds the negotiated
> > > link speed. Use it together with pcie_wait_after_link_train() to insert
> > > the required delay immediately after confirming that the link is up.
> > >
> > > Signed-off-by: Hans Zhang <18255117159@163.com>
> > > ---
> > > drivers/pci/controller/pci-aardvark.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index e34bea1ff0ac..526351c21c49 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
> > > /* check if the link is up or not */
> > > for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> > > - if (advk_pcie_link_up(pcie))
> > > + if (advk_pcie_link_up(pcie)) {
> > > + pcie_wait_after_link_train(pcie->link_gen);
> > > return 0;
> > > + }
> > > usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
> > > }
> > > --
> > > 2.34.1
> > >
> >
> > Are you sure that this is correct to do? Have you checked the A3720
> > Functional Specification which describes how to bring PCIe link up?
> >
> > A3720 PCIe controller is buggy and needs more timing hacks to make it
> > behave. Playing with random sleeps can break its internal logic.
> > I'm not sure if it could be safe without proper testing.
> >
> > And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
>
>
> Hi Pali,
>
> 1. This driver does not support A3720.
>
> static const struct of_device_id advk_pcie_of_match_table[] = {
> { .compatible = "marvell,armada-3700-pcie", },
> {},
> };
> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
>
> If you need support for A3720, please submit the corresponding patch so that
> Bjorn and Mani can review it.
3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
>
> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
> in the DT. This will not affect the functionality of this patch.
Whole A37xx supports only GEN2. And in DT files for 37xx should be
already there max-link-speed.
Seems that in advk_pcie_of_match_table there is no GEN3 device
specified.
> 3. This patch is a common delay requirement stipulated by the PCIe
> specification. If it is greater than GEN2, then msleep(100) will be added;
> otherwise, there will be no such delay.
>
> 4. For instance, we often come across the situation where some common APIs
> are modified, and in many cases, their functionality does not require the
> actual development board for verification. I believe that many other
> developers and maintainers have modified different parts of the code. For
> example, the recent submission:
Switching one API to another is one thing. But changing code which looks
to be critical, specially when it is known that hw has bugs, can cause
breaking of existing boards.
> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
> Author: Nam Cao <namcao@linutronix.de>
> Date: Thu Jun 26 16:47:53 2025 +0200
>
> PCI: aardvark: Switch to msi_create_parent_irq_domain()
>
> Switch to msi_create_parent_irq_domain() from
> pci_msi_create_irq_domain()
> which was using legacy MSI domain setup.
>
>
> And many controller drivers have been modified.
>
>
> Best regards,
> Hans
>
>
^ permalink raw reply
* Re: [PATCH v4 2/2] coco: guest: arm64: Drop dummy RSI platform device stub
From: Greg KH @ 2026-05-13 7:11 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Catalin Marinas, linux-kernel, linux-arm-kernel, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Jonathan Cameron, Suzuki K Poulose
In-Reply-To: <yq5a7bp7u77f.fsf@kernel.org>
On Wed, May 13, 2026 at 12:28:12PM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
>
> > + Suzuki again
> >
> > On Mon, Apr 27, 2026 at 11:46:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> The SMCCC firmware driver now creates the `arm-smccc` platform device
> >> and also creates the CCA auxiliary devices once the RSI ABI is
> >> discovered. This makes the arch-specific arm64_create_dummy_rsi_dev()
> >> helper redundant. Remove the arm-cca-dev platform device registration
> >> and let the SMCCC probe manage the RSI device.
> >>
> >> systemd match on platform:arm-cca-dev for confidential vm detection [1].
> >> Losing the platform device registration can break that. Keeping this
> >> removal in its own change makes it easy to revert if that regression
> >> blocks the rollout.
> >>
> >> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
> >
> > I wouldn't merge this now given that systemd checks this file. Could we
> > have a symbolic link instead for some time until systemd eventually gets
> > updated (years?).
> >
>
> I’ll add this in the next revision.
>
> static int create_rsi_compat_link(struct device *target_dev)
> {
> struct kobject *platform_kobj;
> /*
> * target_dev is:
> * /sys/devices/platform/arm-smccc/arm_cca_guest.arm-rsi-dev.0
> * Create compat link /sys/devices/platform/arm-cca-dev
> */
> platform_kobj = target_dev->kobj.parent->parent;
What? That is crazy, you don't know that is always going to be ok.
> return sysfs_create_link(platform_kobj,
> &target_dev->kobj,
> "arm-cca-dev");
No, don't do that, if a driver calls a sysfs* function, something is
almost always wrong. Don't be making random sysfs symlinks please.
If userspace can not find the device anymore, that's fine, that's how
sysfs works, devices move around all the time. Especially platform
devices as those are almost always not supposed to be platform devices :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH] media: rc: sunxi-cir: unregister rc device on probe failure
From: 박명훈 @ 2026-05-13 7:11 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab
Cc: Myeonghun Pak, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
linux-media, linux-arm-kernel, linux-sunxi, linux-kernel, stable,
Ijae Kim
From: Myeonghun Pak <mhun512@gmail.com>
After rc_register_device() succeeds, later probe failures must undo the
registration with rc_unregister_device(). The current error path jumps to
the allocation cleanup label and only calls rc_free_device(), leaving the
rc device registration and resources created by rc_register_device()
behind.
Add a registered-device unwind label for the IRQ lookup, IRQ request, and
hardware initialization failure paths. Keep rc_free_device() for failures
before rc_register_device() succeeds.
Fixes: b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/media/rc/sunxi-cir.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 92ef4e7c6f..cc64a68dfe 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->irq = platform_get_irq(pdev, 0);
if (ir->irq < 0) {
ret = ir->irq;
- goto exit_free_dev;
+ goto exit_unregister_dev;
}
ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
if (ret) {
dev_err(dev, "failed request irq\n");
- goto exit_free_dev;
+ goto exit_unregister_dev;
}
ret = sunxi_ir_hw_init(dev);
if (ret)
- goto exit_free_dev;
+ goto exit_unregister_dev;
dev_info(dev, "initialized sunXi IR driver\n");
return 0;
+exit_unregister_dev:
+ rc_unregister_device(ir->rc);
+ return ret;
+
exit_free_dev:
rc_free_device(ir->rc);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Hans Zhang @ 2026-05-13 7:00 UTC (permalink / raw)
To: Pali Rohár
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <20260512212531.jupoocz7acv22qyg@pali>
On 5/13/26 05:25, Pali Rohár wrote:
> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>> The Aardvark PCIe controller driver waits for the link to come up but
>> does not implement the mandatory 100 ms delay after link training
>> completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>
>> The driver already maintains a 'link_gen' field that holds the negotiated
>> link speed. Use it together with pcie_wait_after_link_train() to insert
>> the required delay immediately after confirming that the link is up.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/controller/pci-aardvark.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>> index e34bea1ff0ac..526351c21c49 100644
>> --- a/drivers/pci/controller/pci-aardvark.c
>> +++ b/drivers/pci/controller/pci-aardvark.c
>> @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
>>
>> /* check if the link is up or not */
>> for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
>> - if (advk_pcie_link_up(pcie))
>> + if (advk_pcie_link_up(pcie)) {
>> + pcie_wait_after_link_train(pcie->link_gen);
>> return 0;
>> + }
>>
>> usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>> }
>> --
>> 2.34.1
>>
>
> Are you sure that this is correct to do? Have you checked the A3720
> Functional Specification which describes how to bring PCIe link up?
>
> A3720 PCIe controller is buggy and needs more timing hacks to make it
> behave. Playing with random sleeps can break its internal logic.
> I'm not sure if it could be safe without proper testing.
>
> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
Hi Pali,
1. This driver does not support A3720.
static const struct of_device_id advk_pcie_of_match_table[] = {
{ .compatible = "marvell,armada-3700-pcie", },
{},
};
MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
If you need support for A3720, please submit the corresponding patch so
that Bjorn and Mani can review it.
2. If A3720 only supports GEN2, you can configure "max-link-speed" to be
2 in the DT. This will not affect the functionality of this patch.
3. This patch is a common delay requirement stipulated by the PCIe
specification. If it is greater than GEN2, then msleep(100) will be
added; otherwise, there will be no such delay.
4. For instance, we often come across the situation where some common
APIs are modified, and in many cases, their functionality does not
require the actual development board for verification. I believe that
many other developers and maintainers have modified different parts of
the code. For example, the recent submission:
commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
Author: Nam Cao <namcao@linutronix.de>
Date: Thu Jun 26 16:47:53 2025 +0200
PCI: aardvark: Switch to msi_create_parent_irq_domain()
Switch to msi_create_parent_irq_domain() from
pci_msi_create_irq_domain()
which was using legacy MSI domain setup.
And many controller drivers have been modified.
Best regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 2/2] coco: guest: arm64: Drop dummy RSI platform device stub
From: Aneesh Kumar K.V @ 2026-05-13 6:58 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-kernel, linux-arm-kernel, Greg KH, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Jonathan Cameron, Suzuki K Poulose
In-Reply-To: <agM7VmoQ--ylUmyM@arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> + Suzuki again
>
> On Mon, Apr 27, 2026 at 11:46:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> The SMCCC firmware driver now creates the `arm-smccc` platform device
>> and also creates the CCA auxiliary devices once the RSI ABI is
>> discovered. This makes the arch-specific arm64_create_dummy_rsi_dev()
>> helper redundant. Remove the arm-cca-dev platform device registration
>> and let the SMCCC probe manage the RSI device.
>>
>> systemd match on platform:arm-cca-dev for confidential vm detection [1].
>> Losing the platform device registration can break that. Keeping this
>> removal in its own change makes it easy to revert if that regression
>> blocks the rollout.
>>
>> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
>
> I wouldn't merge this now given that systemd checks this file. Could we
> have a symbolic link instead for some time until systemd eventually gets
> updated (years?).
>
I’ll add this in the next revision.
static int create_rsi_compat_link(struct device *target_dev)
{
struct kobject *platform_kobj;
/*
* target_dev is:
* /sys/devices/platform/arm-smccc/arm_cca_guest.arm-rsi-dev.0
* Create compat link /sys/devices/platform/arm-cca-dev
*/
platform_kobj = target_dev->kobj.parent->parent;
return sysfs_create_link(platform_kobj,
&target_dev->kobj,
"arm-cca-dev");
}
-aneesh
^ permalink raw reply
* Re: [PATCH v4 1/2] firmware: smccc: coco: Manage arm-smccc platform device and CCA auxiliary drivers
From: Aneesh Kumar K.V @ 2026-05-13 6:56 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-kernel, linux-arm-kernel, Greg KH, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Suzuki K Poulose
In-Reply-To: <agM699d4KwdjnS39@arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> + Suzuki
>
> On Mon, Apr 27, 2026 at 11:46:14AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
>> index 88b50d660e85..2d2d363aaaee 100644
>> --- a/arch/arm64/include/asm/rsi.h
>> +++ b/arch/arm64/include/asm/rsi.h
>> @@ -10,7 +10,7 @@
>> #include <linux/jump_label.h>
>> #include <asm/rsi_cmds.h>
>>
>> -#define RSI_PDEV_NAME "arm-cca-dev"
>> +#define RSI_DEV_NAME "arm-rsi-dev"
> [...]
>> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
>> index bdee057db2fd..fc9b44b7c687 100644
>> --- a/drivers/firmware/smccc/smccc.c
>> +++ b/drivers/firmware/smccc/smccc.c
>> @@ -12,6 +12,8 @@
>> #include <linux/platform_device.h>
>> #include <asm/archrandom.h>
>>
>> +#include "rmm.h"
>> +
>> static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
>> static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
>>
>> @@ -85,6 +87,18 @@ static int __init smccc_devices_init(void)
>> {
>> struct platform_device *pdev;
>>
>> + pdev = platform_device_register_simple("arm-smccc",
>> + PLATFORM_DEVID_NONE, NULL, 0);
>> + if (IS_ERR(pdev)) {
>> + pr_err("arm-smccc: could not register device: %ld\n", PTR_ERR(pdev));
>> + } else {
>> + /*
>> + * Register the RMI and RSI devices only when firmware exposes
>> + * the required SMCCC function IDs at a supported revision.
>> + */
>> + register_rsi_device(pdev);
>> + }
>
> So as per the cover letter, instead of "arm-cca-dev" as a platform
> device, we get "arm-smccc" as a platform device with an auxiliary
> "arm-rsi-dev" child device. This does not get rid of the platform
> device, it just creates a synthetic platform device to represent the
> SMCCC firmware interface.
>
> Looking at the earlier discussion, I think this is what Greg/Jason were
> suggesting, except that we do not currently have an SMCCC platform
> device:
>
> https://lore.kernel.org/all/2025101534-frosty-shank-00b1@gregkh/
>
> If we go this route, shouldn't the platform device above be created only
> if !SMCCC_CONDUIT_NONE?
>
register_rsi_device() does check for
if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
return;
>
> "smccc_trng" would also fit this model (together with the driver),
> assuming we don't break any user-space (searching the Debian codebase
> did not find any use).
Will switch smccc_trng to an auxiliary device in the next revision.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 01/10] dt-bindings: display: rockchip: analogix-dp: Allow hclk as third clock
From: Damon Ding @ 2026-05-13 6:48 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <20260512-diabetic-ahead-dd36bc2d8be7@spud>
Hi Conor,
On 5/13/2026 1:12 AM, Conor Dooley wrote:
> On Tue, May 12, 2026 at 05:56:35PM +0800, Damon Ding wrote:
>> RK3588 eDP controller requires HCLK_VO1 (video output bus clock)
>> to access the VO1 GRF registers and enable the video datapath.
>>
>> Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
>> phandle reference, which allowed the eDP to work without explicitly
>> managing the hclk_vo1 clock. However, this is not safe or explicit.
>>
>> To align with other display controllers (HDMI) on RK3588 and make
>> the clock requirement explicit, expand clock-names to support either
>> "grf" (for older SoCs) or "hclk" (for RK3588) as the third clock.
>>
>> This makes the clock dependency clear and removes reliance on implicit
>> clock enablement from GRF phandle.
>>
>> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v4:
>> - Modify the commit msg.
>> ---
>> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index d99b23b88cc5..d2bc8636b626 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -26,7 +26,9 @@ properties:
>> items:
>> - const: dp
>> - const: pclk
>> - - const: grf
>> + - enum:
>> + - grf
>> + - hclk
>
> Could you also enforce the correct clock name on a per-compatible basis
> please?
>
> pw-bot: changes-requested
>
Yes, will do in v5.
>
>>
>> power-domains:
>> maxItems: 1
>> --
>> 2.34.1
>>
Best regards,
Damon
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: ti: icssg: Derive stats array lengths from ARRAY_SIZE
From: MD Danish Anwar @ 2026-05-13 6:29 UTC (permalink / raw)
To: David CARLIER
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Roger Quadros,
Andrew Lunn, Jacob Keller, Meghana Malladi, Kevin Hao,
Vadim Fedorenko, netdev, linux-doc, linux-kernel,
linux-arm-kernel, Vignesh Raghavendra
In-Reply-To: <CA+XhMqzx9CUX5H7q1UqL=heGWLFjZVfyiTx6b45VW=E9t13Fow@mail.gmail.com>
Hi David
On 12/05/26 3:33 pm, David CARLIER wrote:
> Hi Danish,
>
>
> On Tue, 12 May 2026 at 10:40, MD Danish Anwar <danishanwar@ti.com> wrote:
>>
>> Hi David,
>>
>> On 12/05/26 1:28 pm, David CARLIER wrote:
>>> Hi MD,
>>>
>>> On Tue, 12 May 2026 at 07:06, MD Danish Anwar <danishanwar@ti.com> wrote:
>>>>
>>>> Replace the manually maintained ICSSG_NUM_MIIG_STATS and
>>>> ICSSG_NUM_PA_STATS constants with ARRAY_SIZE() expressions derived
>>>> directly from the corresponding stat descriptor arrays, so that adding
>>>> new entries to icssg_all_miig_stats[] or icssg_all_pa_stats[] no longer
>>>> requires a separate update to a numeric constant.
>>>>
>>>> To make this self-contained, break the circular include dependency
>>>> between icssg_stats.h and icssg_prueth.h:
>>>>
>>>> - icssg_stats.h previously included icssg_prueth.h (transitively
>>>> pulling in icssg_switch_map.h and ETH_GSTRING_LEN). Replace that
>>>> with direct includes of <linux/ethtool.h>, <linux/kernel.h> and
>>>> "icssg_switch_map.h".
>>>>
>>>> - icssg_prueth.h now includes icssg_stats.h, giving it access to
>>>> the ARRAY_SIZE-based ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS
>>>> before they are used in the prueth_emac struct and ICSSG_NUM_STATS.
>>>>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +--
>>>> drivers/net/ethernet/ti/icssg/icssg_stats.h | 7 ++++++-
>>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> index df93d15c5b78..e2ccecb0a0dd 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> @@ -43,6 +43,7 @@
>>>>
>>>> #include "icssg_config.h"
>>>> #include "icss_iep.h"
>>>> +#include "icssg_stats.h"
>>>> #include "icssg_switch_map.h"
>>>>
>>>> #define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN)
>>>> @@ -57,8 +58,6 @@
>>>>
>>>> #define ICSSG_MAX_RFLOWS 8 /* per slice */
>>>>
>>>> -#define ICSSG_NUM_PA_STATS 32
>>>> -#define ICSSG_NUM_MIIG_STATS 60
>>>> /* Number of ICSSG related stats */
>>>> #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
>>>> #define ICSSG_NUM_STANDARD_STATS 31
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> index 5ec0b38e0c67..b854eb587c1e 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> @@ -8,10 +8,15 @@
>>>> #ifndef __NET_TI_ICSSG_STATS_H
>>>> #define __NET_TI_ICSSG_STATS_H
>>>>
>>>> -#include "icssg_prueth.h"
>>>> +#include <linux/ethtool.h>
>>>> +#include <linux/kernel.h>
>>>> +#include "icssg_switch_map.h"
>>>>
>>>> #define STATS_TIME_LIMIT_1G_MS 25000 /* 25 seconds @ 1G */
>>>>
>>>> +#define ICSSG_NUM_MIIG_STATS ARRAY_SIZE(icssg_all_miig_stats)
>>>> +#define ICSSG_NUM_PA_STATS ARRAY_SIZE(icssg_all_pa_stats)
>>>> +
>>>> struct miig_stats_regs {
>>>> /* Rx */
>>>> u32 rx_packets;
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> One thing that caught my eye: icssg_all_miig_stats[] and
>>> icssg_all_pa_stats[] are 'static const' arrays in icssg_stats.h with
>>> ETH_GSTRING_LEN name buffers per entry. Right now only icssg_stats.c
>>> and icssg_ethtool.c pull them in. After this patch icssg_prueth.h
>>> includes icssg_stats.h, so every .c in the driver (classifier,
>>> common, config, mii_cfg, queues, switchdev, ...) ends up with its own
>>> static-const copy of both tables.
>>>
>>> Would a static_assert() work for what you're after? Something like:
>>>
>>
>> While adding more stats manually, The ARRAY_SIZE() approach was
>> explicitly requested by maintainer [1]:
>>
>> This patch is a direct response to that feedback. static_assert() would
>> still require updating the numeric constant on every array change. The
>> goal here is to eliminate the need of manually incrementing stats count
>> whenever new stats are added
>>
>> Your concern about multiple copies of table is noted and valid. Could
>> you advise on the preferred way to reconcile these two requirements? I
>> am happy to restructure if there is an approach that satisfies both.
>>
>> [1]
>> https://lore.kernel.org/all/20260112181436.4s5ceywwembn674r@skbuf/#:~:text=Can%27t%20this%20be%20expressed%20as%20ARRAY_SIZE(icssg_all_pa_stats)%3F%20It%20is%20very%0Afragile%20to%20have%20to%20count%20and%20update%20this%20manually.
>>
>>
>>> static const struct icssg_miig_stats icssg_all_miig_stats[] = {
>>> ...
>>> };
>>> static_assert(ARRAY_SIZE(icssg_all_miig_stats) == ICSSG_NUM_MIIG_STATS);
>>>
>>> next to each array, keeping the numeric #defines as-is. Then 2/2 fails
>>> to build the moment a new entry is added without bumping the count,
>>> which is the case you're guarding against — without touching the
>>> include graph.
>>>
>>> What do you think ?
>>>
>>> Cheers.
>>
>> --
>> Thanks and Regards,
>> Danish
>>
>
>
> Thanks for digging up the context — fair point, I'd missed Vladimir's
> earlier ask. Reading it again though, what he calls fragile is the
> silent miscount, not the keystroke of typing a number. A static_assert
> turns "forgot to bump" into a build error, which I think gets you
> there.
>
Thank you for the suggestion. I think your previous suggestion fits
better. I believe keeping the arrays in icssg_stats.h is preferable to
moving them to icssg_stats.c. Here is my reasoning:
Your binary-bloat concern was about icssg_prueth.h including
icssg_stats.h, which would drag the static const tables into every .c
that includes icssg_prueth.h (~11 translation units). That concern is
valid, but it is specific to the include direction of the previous
patch. If we simply revert to the original include graph —
icssg_stats.h includes icssg_prueth.h, not the other way around —
only the two files that have always included icssg_stats.h directly
(icssg_stats.c and icssg_ethtool.c) get a copy of the arrays. No
regression in binary size compared to the baseline.
> What about moving the two arrays into icssg_stats.c, declaring them
> extern in the header, and dropping a static_assert next to each
> definition? Numeric #defines stay where they are, icssg_prueth.h
> doesn't need to know about icssg_stats.h, and the tables live in one
> TU instead of every .o in the driver. If the count and the array
> disagree, you get a compile error on the spot.
>
Moving the arrays to icssg_stats.c (approach #2) adds extern
declarations, splits the definition from the static_assert, and is a
larger restructuring for the same safety guarantee. Keeping the arrays
in the header with a static_assert immediately after each one is a
2-line diff and leaves the code easy to read in one place.
Please let me know if this sounds okay to you. I will send out a v2 soon
if this approach is fine with you.
> Probably worth keeping Vladimir on Cc for v2 in case he had something
> else in mind.
>
I will CC Vladimir in v2.
--
Thanks and Regards,
Danish
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables
From: Daniel Baluta @ 2026-05-13 6:30 UTC (permalink / raw)
To: tanmay.shah, 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
In-Reply-To: <9e3a88d9-1679-43ae-a96b-62a29bd45d9d@amd.com>
On 5/12/26 17:53, Shah, Tanmay wrote:
>
>
> 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.
Agree, this is the best way to go.
^ permalink raw reply
* [PATCH v2 3/3] clk: nuvoton: ma35d1: fix ma35d1_clk_pll_determine_rate logic
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
ma35d1_clk_pll_determine_rate() called ma35d1_pll_find_closest()
unconditionally before the switch statement, and then every case
branch overwrote pll_freq by reading the current hardware registers.
For CAPLL and DDRPLL this means find_closest() ran unnecessarily
(and incorrectly, since those PLLs are read-only) and its result
was silently discarded.
Fix by moving the find_closest() call inside the APLL/EPLL/VPLL
branch where it belongs. Group CAPLL and DDRPLL together as
read-only PLLs that simply report their current rate; handle them
with an explicit if/else to keep the CAPLL (SMIC design) and DDRPLL
(standard design) paths distinct.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index 7e6b30d20c01..314b81e7727c 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -255,32 +255,32 @@ static int ma35d1_clk_pll_determine_rate(struct clk_hw *hw,
if (req->best_parent_rate < PLL_FREF_MIN_FREQ || req->best_parent_rate > PLL_FREF_MAX_FREQ)
return -EINVAL;
- ret = ma35d1_pll_find_closest(pll, req->rate, req->best_parent_rate,
- reg_ctl, &pll_freq);
- if (ret < 0)
- return ret;
-
switch (pll->id) {
case CAPLL:
+ case DDRPLL:
+ /* Read-only PLLs: return current rate */
reg_ctl[0] = readl_relaxed(pll->ctl0_base);
- pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], req->best_parent_rate);
+ if (pll->id == CAPLL) {
+ pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], req->best_parent_rate);
+ } else {
+ reg_ctl[1] = readl_relaxed(pll->ctl1_base);
+ pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, req->best_parent_rate);
+ }
req->rate = pll_freq;
-
return 0;
- case DDRPLL:
case APLL:
case EPLL:
case VPLL:
- reg_ctl[0] = readl_relaxed(pll->ctl0_base);
- reg_ctl[1] = readl_relaxed(pll->ctl1_base);
- pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, req->best_parent_rate);
+ /* Configurable PLLs: find closest achievable rate */
+ ret = ma35d1_pll_find_closest(pll, req->rate, req->best_parent_rate,
+ reg_ctl, &pll_freq);
+ if (ret < 0)
+ return ret;
req->rate = pll_freq;
-
return 0;
}
req->rate = 0;
-
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] clk: nuvoton: ma35d1: fix PLL_CTL1_FRAC bit field width and fractional calc
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
PLL_CTL1_FRAC was defined as GENMASK(31, 24), covering only 8 bits.
The hardware fractional field occupies bits [31:8] (24 bits), so the
mask must be GENMASK(31, 8).
The previous fractional-mode calculation used FIELD_MAX(PLL_CTL1_FRAC)
as the denominator to obtain 2 decimal places. With the corrected 24-bit
mask the old divisor is wrong; replace the arithmetic with a proper
24-bit fixed-point rounding to 3 decimal places:
n_frac = n * 1000 + (x * 1000 + 500) >> 24
The +500 term provides round-to-nearest before the right shift.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index bfedd45bd04b..7e6b30d20c01 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -48,7 +48,7 @@
#define PLL_CTL1_PD BIT(0)
#define PLL_CTL1_BP BIT(1)
#define PLL_CTL1_OUTDIV GENMASK(6, 4)
-#define PLL_CTL1_FRAC GENMASK(31, 24)
+#define PLL_CTL1_FRAC GENMASK(31, 8)
#define PLL_CTL2_SLOPE GENMASK(23, 0)
#define INDIV_MIN 1
@@ -113,9 +113,9 @@ static unsigned long ma35d1_calc_pll_freq(u8 mode, u32 *reg_ctl, unsigned long p
pll_freq = div_u64(pll_freq, m * p);
} else {
x = FIELD_GET(PLL_CTL1_FRAC, reg_ctl[1]);
- /* 2 decimal places floating to integer (ex. 1.23 to 123) */
- n = n * 100 + ((x * 100) / FIELD_MAX(PLL_CTL1_FRAC));
- pll_freq = div_u64(parent_rate * n, 100 * m * p);
+ /* x is 24-bit fractional part, convert to 3 decimal digits */
+ n = n * 1000 + (u32)(((u64)x * 1000 + 500) >> 24);
+ pll_freq = div_u64((u64)parent_rate * n, 1000 * m * p);
}
return pll_freq;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] clk: nuvoton: ma35d1: fix ignored div_u64 return values in PLL freq calculation
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
div_u64() does not modify its argument in place; the return value must
be assigned. Both ma35d1_calc_smic_pll_freq() and ma35d1_calc_pll_freq()
called div_u64() and discarded the result, leaving pll_freq holding the
undivided product and thus returning a frequency orders of magnitude too
high.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index 4620acfe47e8..bfedd45bd04b 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -92,7 +92,7 @@ static unsigned long ma35d1_calc_smic_pll_freq(u32 pll0_ctl0,
p = FIELD_GET(SPLL0_CTL0_OUTDIV, pll0_ctl0);
outdiv = 1 << p;
pll_freq = (u64)parent_rate * n;
- div_u64(pll_freq, m * outdiv);
+ pll_freq = div_u64(pll_freq, m * outdiv);
return pll_freq;
}
@@ -110,7 +110,7 @@ static unsigned long ma35d1_calc_pll_freq(u8 mode, u32 *reg_ctl, unsigned long p
if (mode == PLL_MODE_INT) {
pll_freq = (u64)parent_rate * n;
- div_u64(pll_freq, m * p);
+ pll_freq = div_u64(pll_freq, m * p);
} else {
x = FIELD_GET(PLL_CTL1_FRAC, reg_ctl[1]);
/* 2 decimal places floating to integer (ex. 1.23 to 123) */
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] clk: nuvoton: ma35d1: fix PLL frequency calculation
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
Fix four bugs in the MA35D1 PLL clock driver that cause incorrect
frequency values returned from recalc_rate() and determine_rate().
v1 combined all fixes into a single commit. At reviewer request,
split into one patch per logical fix:
1/3 - fix div_u64 return value being discarded (affects both
ma35d1_calc_smic_pll_freq and ma35d1_calc_pll_freq INT mode)
2/3 - fix PLL_CTL1_FRAC mask width (8-bit -> 24-bit) and update
the fractional-mode arithmetic accordingly
3/3 - fix ma35d1_clk_pll_determine_rate: move find_closest() into
the configurable-PLL branch; unify read-only PLL handling
Joey Lu (3):
clk: nuvoton: ma35d1: fix ignored div_u64 return values in PLL freq
calculation
clk: nuvoton: ma35d1: fix PLL_CTL1_FRAC bit field width and fractional
calc
clk: nuvoton: ma35d1: fix ma35d1_clk_pll_determine_rate logic
drivers/clk/nuvoton/clk-ma35d1-pll.c | 38 ++++++++++++++--------------
1 file changed, 19 insertions(+), 19 deletions(-)
--
2.43.0
^ 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