Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] arm64: dts: qcom: kaanapali: Add SoCCP for Kaanapali SoC
From: Jingyi Wang @ 2026-03-30  2:53 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang, linux-arm-msm,
	devicetree, linux-kernel, 20260310-knp-soccp-v4-0-0a91575e0e7e
In-Reply-To: <ac75a339-ca20-4264-9295-d7290e218bc5@oss.qualcomm.com>



On 3/27/2026 5:52 PM, Konrad Dybcio wrote:
> On 3/27/26 4:20 AM, Jingyi Wang wrote:
>> Add remoteproc PAS loader for SoCCP with its SMP2P. On Kaanapali, it
>> is brought up by bootloader, so set the status "okay".
> 
> Simply remove it, "okay" is the default if the property is absent
> 
> [...]
> 

well noted

>> +		remoteproc_soccp: remoteproc-soccp@d00000 {
> 
> -> remoteproc@

well noted

> 
> Konrad

Thanks,
Jingyi

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64: dts: qcom: kaanapali: Add USB support for Kaanapali SoC
From: Jingyi Wang @ 2026-03-30  2:52 UTC (permalink / raw)
  To: Krishna Kurapati, Konrad Dybcio, Bjorn Andersson, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Ronak Raheja
In-Reply-To: <20260329175249.2946508-2-krishna.kurapati@oss.qualcomm.com>



On 3/30/2026 1:52 AM, Krishna Kurapati wrote:
> From: Ronak Raheja <ronak.raheja@oss.qualcomm.com>
> 
> Add the base USB devicetree definitions for Kaanapali platform. The overall
> chipset contains a single DWC3 USB3 controller (rev. 200a), SS QMP PHY
> (rev. v8) and M31 eUSB2 PHY.
> 
> Signed-off-by: Ronak Raheja <ronak.raheja@oss.qualcomm.com>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>   arch/arm64/boot/dts/qcom/kaanapali.dtsi | 154 ++++++++++++++++++++++++
>   1 file changed, 154 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index ac6a6c789902..08d7c1a1d829 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> @@ -6026,6 +6026,160 @@ pdp_tx: scp-sram-section@100 {
>   				reg = <0x100 0x80>;
>   			};
>   		};
> +
> +		usb_hsphy: phy@88e3000 {
> +			compatible = "qcom,kaanapali-m31-eusb2-phy",
> +				     "qcom,sm8750-m31-eusb2-phy";
> +			reg = <0x0 0x88e3000 0x0 0x29c>;
> +
> +			clocks = <&tcsr TCSR_USB2_CLKREF_EN>;
> +			clock-names = "ref";
> +
> +			resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_dp_qmpphy: phy@88e8000 {
> +			compatible = "qcom,kaanapali-qmp-usb3-dp-phy",
> +				     "qcom,sm8750-qmp-usb3-dp-phy";
> +			reg = <0x0 0x088e8000 0x0 0x4000>;
> +
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> +				 <&tcsr TCSR_USB3_CLKREF_EN>,
> +				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +			clock-names = "aux",
> +				      "ref",
> +				      "com_aux",
> +				      "usb3_pipe";
> +
> +			resets = <&gcc GCC_USB3_PHY_PRIM_BCR>,
> +				 <&gcc GCC_USB3_DP_PHY_PRIM_BCR>;
> +			reset-names = "phy",
> +				      "common";
> +
> +			power-domains = <&gcc GCC_USB3_PHY_GDSC>;
> +
> +			#clock-cells = <1>;
> +			#phy-cells = <1>;
> +
> +			orientation-switch;
> +
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					usb_dp_qmpphy_out: endpoint {
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					usb_dp_qmpphy_usb_ss_in: endpoint {
> +						remote-endpoint = <&usb_dwc3_ss>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +
> +					usb_dp_qmpphy_dp_in: endpoint {
> +					};
> +				};
> +			};
> +		};
> +
> +		usb: usb@a600000 {
> +			compatible = "qcom,kaanapali-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x0a600000 0x0 0xfc100>;
> +
> +			clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
> +				 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
> +				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
> +				 <&gcc GCC_USB30_PRIM_SLEEP_CLK>,
> +				 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>;
> +			clock-names = "cfg_noc",
> +				      "core",
> +				      "iface",
> +				      "sleep",
> +				      "mock_utmi";
> +
> +			assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
> +					  <&gcc GCC_USB30_PRIM_MASTER_CLK>;
> +			assigned-clock-rates = <19200000>, <200000000>;
> +
> +			interrupts-extended = <&intc GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
> +					      <&pdc 15 IRQ_TYPE_EDGE_BOTH>,
> +					      <&pdc 17 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "dwc_usb3",
> +					  "pwr_event",
> +					  "hs_phy_irq",
> +					  "dp_hs_phy_irq",
> +					  "dm_hs_phy_irq",
> +					  "ss_phy_irq";
> +
> +			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
> +			required-opps = <&rpmhpd_opp_nom>;
> +
> +			resets = <&gcc GCC_USB30_PRIM_BCR>;
> +
> +			interconnects = <&aggre_noc MASTER_USB3 QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +					 &config_noc SLAVE_USB3 QCOM_ICC_TAG_ACTIVE_ONLY>;
> +			interconnect-names = "usb-ddr", "apps-usb";
> +			iommus = <&apps_smmu 0x40 0x0>;
> +
> +			phys = <&usb_hsphy>, <&usb_dp_qmpphy QMP_USB43DP_USB3_PHY>;
> +			phy-names = "usb2-phy", "usb3-phy";
> +
> +			snps,hird-threshold = /bits/ 8 <0x0>;
> +			snps,usb2-gadget-lpm-disable;
> +			snps,dis_u2_susphy_quirk;
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u1-entry-quirk;
> +			snps,dis-u2-entry-quirk;
> +			snps,is-utmi-l1-suspend;
> +			snps,usb3_lpm_capable;
> +			snps,usb2-lpm-disable;
> +			snps,has-lpm-erratum;
> +			tx-fifo-resize;
> +			dma-coherent;
> +
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					usb_dwc3_hs: endpoint {
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					usb_dwc3_ss: endpoint {
> +						remote-endpoint = <&usb_dp_qmpphy_usb_ss_in>;
> +					};
> +				};
> +			};
> +		};

please make sure the nodes are sorted in address order.

Thanks,
Jingyi

>   	};
>   
>   	thermal-zones {


^ permalink raw reply

* RE: [PATCH v5 01/10] dt-bindings: clock: Add Realtek RTD1625 Clock & Reset Controller
From: Yu-Chun Lin [林祐君] @ 2026-03-30  2:48 UTC (permalink / raw)
  To: Stephen Boyd, afaerber@suse.com, conor+dt@kernel.org,
	Edgar Lee [李承諭],
	Jyan Chou [周芷安], krzk+dt@kernel.org,
	mturquette@baylibre.com, p.zabel@pengutronix.de, robh@kernel.org
  Cc: devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-realtek-soc@lists.infradead.org,
	James Tai [戴志峰],
	CY_Huang[黃鉦晏],
	Stanley Chang[昌育德]
In-Reply-To: <177440394165.5403.17868576455504268400@localhost.localdomain>

> Quoting Yu-Chun Lin (2026-03-23 19:53:22)
> > diff --git
> > a/Documentation/devicetree/bindings/clock/realtek,rtd1625-clk.yaml
> > b/Documentation/devicetree/bindings/clock/realtek,rtd1625-clk.yaml
> > new file mode 100644
> > index 000000000000..6fabc2da3975
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/realtek,rtd1625-clk.yaml
> > @@ -0,0 +1,52 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/realtek,rtd1625-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Realtek RTD1625 Clock & Reset Controller
> > +
> > +maintainers:
> > +  - Yu-Chun Lin <eleanor.lin@realtek.com>
> > +
> > +description: |
> > +  The Realtek RTD1625 Clock Controller manages and distributes clock
> > +  signals to various controllers and implements a Reset Controller
> > +for the
> > +  SoC peripherals.
> > +
> > +  Clocks and resets are referenced by unique identifiers, which are
> > + defined as  preprocessor macros in
> > + include/dt-bindings/clock/realtek,rtd1625-clk.h and
> include/dt-bindings/reset/realtek,rtd1625.h.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - realtek,rtd1625-crt-clk
> > +      - realtek,rtd1625-iso-clk
> > +      - realtek,rtd1625-iso-s-clk
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  "#reset-cells":
> > +    const: 1
> 
> Are there any input clks for the clk tree?
> 

We don't dynamically calculate frequencies based on an input clock.
Since all of our current SoCs use a fixed 27MHz oscillator, we use
predefined lookup tables in the driver for the target frequencies instead.

Nevertheless, to properly describe the hardware layout, I will add the clock
properties in the bindings and DTS in v6.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    clock-controller@98000000 {
> > +      compatible = "realtek,rtd1625-crt-clk";
> > +      reg = <98000000 0x1000>;
> 
> Should be 0x98000000 to match the unit address.
> 

Ack.

Best Regards,
Yu-Chun

> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: Add RK3562 serial aliases
From: 谢致邦 (XIE Zhibang) @ 2026-03-30  2:46 UTC (permalink / raw)
  To: krzk
  Cc: Yeking, conor+dt, devicetree, finley.xiao, heiko, kever.yang,
	krzk+dt, linux-arm-kernel, linux-kernel, linux-rockchip, robh
In-Reply-To: <9b3ee9e9-d44d-49b1-81ac-9c3806dc0efb@kernel.org>

On Sat, Mar 28, 2026 at 04:08:57PM +0100, Krzysztof Kozlowski wrote:
> On 28/03/2026 14:05, 谢致邦 (XIE Zhibang) wrote:
> > This fixes the stdout-path in rk3562-evb2-v10.dts.
> > 
> > Fixes: ceb6ef1ea900 ("arm64: dts: rockchip: Add RK3562 evb2 devicetree")
> > Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3562.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3562.dtsi b/arch/arm64/boot/dts/rockchip/rk3562.dtsi
> > index e4816aa3dae0..14e74e8ac7df 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3562.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3562.dtsi
> > @@ -26,6 +26,16 @@ aliases {
> >  		gpio2 = &gpio2;
> >  		gpio3 = &gpio3;
> >  		gpio4 = &gpio4;
> > +		serial0 = &uart0;
> > +		serial1 = &uart1;
> > +		serial2 = &uart2;
> > +		serial3 = &uart3;
> > +		serial4 = &uart4;
> > +		serial5 = &uart5;
> > +		serial6 = &uart6;
> > +		serial7 = &uart7;
> > +		serial8 = &uart8;
> > +		serial9 = &uart9;
> 
> UART aliases are properties of the boards, not SoC.
> 
> Best regards,
> Krzysztof

So are you saying that we need to remove the serial aliases from files
like rk3308.dtsi, rk3328.dtsi, rk3368.dtsi, rk3399-base.dtsi,
rk356x-base.dtsi, rk3576.dtsi, rk3588-base.dtsi, and so on?

Kind regards,
XIE Zhibang


^ permalink raw reply

* Re: [PATCH v2 4/7] pwm: tegra: Parametrize enable register offset
From: Mikko Perttunen @ 2026-03-30  2:24 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mikko Perttunen, Thierry Reding, Uwe Kleine-König,
	Jonathan Hunter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-pwm, linux-tegra, linux-kernel, devicetree, Yi-Wei Wang
In-Reply-To: <acT_nz0TRM4yXwkb@orome>

On 2026-03-26 10:47 +0100, Thierry Reding wrote:
> On Wed, Mar 25, 2026 at 07:17:02PM +0900, Mikko Perttunen wrote:
> > On Tegra264, the PWM enablement bit is not located at the base address
> > of the PWM controller. Hence, introduce an enablement offset field in
> > the tegra_pwm_soc structure to describe the offset of the register.
> > 
> > Co-developed-by: Yi-Wei Wang <yiweiw@nvidia.com>
> > Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >  drivers/pwm/pwm-tegra.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > index cf54f75d92a5..22d709986e8c 100644
> > --- a/drivers/pwm/pwm-tegra.c
> > +++ b/drivers/pwm/pwm-tegra.c
> > @@ -61,6 +61,7 @@
> >  
> >  struct tegra_pwm_soc {
> >  	unsigned int num_channels;
> > +	unsigned int enable_reg;
> >  };
> >  
> >  struct tegra_pwm_chip {
> > @@ -197,8 +198,9 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  		err = pm_runtime_resume_and_get(pwmchip_parent(chip));
> >  		if (err)
> >  			return err;
> > -	} else
> > +	} else if (pc->soc->enable_reg == PWM_CSR_0) {
> >  		val |= PWM_ENABLE;
> > +	}
> 
> This looks incomplete for the Tegra264 case where
> 
> 	pc->soc->enable_reg == PWM_CSR_1
> 
> >  
> >  	pwm_writel(pwm, PWM_CSR_0, val);
> 
> I think we need another write for PWM_CSR_1 here to properly toggle the
> PWM_ENABLE bit on Tegra264.
> 
> Or am I missing something?

This check is here just so we don't change the value of PWM_ENABLE when
writing the CSR_0 register. The function doesn't write to CSR_1 so
nothing needs to be done on Tegra264.

I agree it's not the clearest, but it'll get cleaned up when adding
support for configurable depth, as at that point we will need to write
both registers on Tegra264.

> 
> Thierry



^ permalink raw reply

* Re: [PATCH v3 5/6] PCI: tegra: Add Tegra264 support
From: Mikko Perttunen @ 2026-03-30  2:17 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jon Hunter, Mikko Perttunen, linux-pci, devicetree,
	linux-tegra
In-Reply-To: <20260326135855.2795149-6-thierry.reding@kernel.org>

On Thu, 26 Mar 2026 14:58:52 +0100, Thierry Reding <thierry.reding@kernel.org> wrote:
> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> new file mode 100644
> index 000000000000..21872797e41a
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-tegra264.c
> @@ -0,0 +1,522 @@
> [ ... skip 137 lines ... ]
> +
> +	value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
> +	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
> +
> +	bw = width * (PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]));

Nit: this now has unnecessary double parentheses.

-- 


^ permalink raw reply

* Re: [PATCH v3 0/6] PCI: tegra: Add Tegra264 support
From: Mikko Perttunen @ 2026-03-30  2:17 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jon Hunter, Mikko Perttunen, linux-pci, devicetree,
	linux-tegra
In-Reply-To: <20260326135855.2795149-1-thierry.reding@kernel.org>

On Thu, 26 Mar 2026 14:58:47 +0100, Thierry Reding <thierry.reding@kernel.org> wrote:
> [...]
>  16 files changed, 4629 insertions(+), 955 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
>  create mode 100644 drivers/pci/controller/pcie-tegra264.c
> 
> --
> 2.52.0

Thank you! Works nicely.

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com>

-- 


^ permalink raw reply

* [PATCH v2 1/1] arm64: dts: qcom: hamoa: Fix incomplete Root Port property migration
From: Ziyue Zhang @ 2026-03-30  2:09 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, ziyue.zhang,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas, johan+linaro,
	vkoul, kishon, neil.armstrong, abel.vesa, kw
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-phy,
	qiang.yu, quic_krichai, quic_vbadigan

Historically, the Qualcomm PCIe controller node (Host bridge) described
all Root Port properties, such as PHY, PERST#, and WAKE#. But to provide
a more accurate hardware description and to support future multi-Root Port
controllers, these properties were moved to the Root Port node in the
devicetree bindings.

Commit 960609b22be5 ("arm64: dts: qcom: hamoa: Move PHY, PERST, and Wake
GPIOs to PCIe port nodes and add port Nodes for all PCIe ports")
initiated this transition for the Hamoa platform by moving the PHY
property to the Root Port node in hamoa.dtsi. However, it only updated
some platform specific DTS files for PERST# and WAKE#, leaving others in
a "mixed" binding state.

While the PCIe controller driver supports both legacy and Root Port
bindings, It cannot correctly handle a mix of both. In these cases, the
driver parses the PHY from the Root Port node, but fails to find the
PERST# property (which it then assumes is not present, as it is optional).
Consequently, the controller probe succeeds, but PERST# remains
uncontrolled, preventing PCIe endpoints from functioning.

So, fix the incomplete migration by moving the PERST# and WAKE# properties
from the controller node to the Root Port node in all remaining Hamoa
platform DTS files.

Fixes: 960609b22be5 ("arm64: dts: qcom: hamoa: Move PHY, PERST, and Wake GPIOs to PCIe port nodes and add port Nodes for all PCIe ports")
Signed-off-by: Ziyue Zhang <ziyue.zhang@oss.qualcomm.com>
---
 .../boot/dts/qcom/x1-asus-zenbook-a14.dtsi    | 16 ++++++++-----
 arch/arm64/boot/dts/qcom/x1-crd.dtsi          | 24 ++++++++++++-------
 arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi   | 14 ++++++-----
 .../boot/dts/qcom/x1-hp-omnibook-x14.dtsi     | 14 ++++++-----
 .../boot/dts/qcom/x1-microsoft-denali.dtsi    |  8 ++++---
 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts  |  6 ++---
 .../qcom/x1e80100-medion-sprchrgd-14-s1.dts   | 14 +++++------
 .../dts/qcom/x1p42100-lenovo-thinkbook-16.dts | 14 ++++++-----
 8 files changed, 64 insertions(+), 46 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi b/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
index cd062f844b2d..66d566808f58 100644
--- a/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
@@ -1079,9 +1079,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1095,10 +1092,12 @@ &pcie4_phy {
 	status = "okay";
 };
 
-&pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+&pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+};
 
+&pcie6a {
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -1114,6 +1113,11 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pm8550_gpios {
 	rtmr0_default: rtmr0-reset-n-active-state {
 		pins = "gpio10";
diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
index 485dcd946757..a9c5c523575e 100644
--- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
@@ -1248,15 +1248,17 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
 	status = "okay";
 };
 
+&pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+};
+
 &pcie4_phy {
 	vdda-phy-supply = <&vreg_l3i_0p8>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
@@ -1265,9 +1267,6 @@ &pcie4_phy {
 };
 
 &pcie5 {
-	perst-gpios = <&tlmm 149 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_wwan>;
 
 	pinctrl-0 = <&pcie5_default>;
@@ -1283,10 +1282,12 @@ &pcie5_phy {
 	status = "okay";
 };
 
-&pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+&pcie5_port0 {
+	reset-gpios = <&tlmm 149 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
+};
 
+&pcie6a {
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-names = "default";
@@ -1302,6 +1303,11 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pm8550_gpios {
 	kypd_vol_up_n: kypd-vol-up-n-state {
 		pins = "gpio6";
diff --git a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
index 343844cc62f2..0d9a324cc6cc 100644
--- a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
@@ -1081,9 +1081,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1098,6 +1095,9 @@ &pcie4_phy {
 };
 
 &pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
@@ -1115,9 +1115,6 @@ wifi@0 {
 };
 
 &pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -1126,6 +1123,11 @@ &pcie6a {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pcie6a_phy {
 	vdda-phy-supply = <&vreg_l1d_0p8>;
 	vdda-pll-supply = <&vreg_l2j_1p2>;
diff --git a/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi b/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
index 16437139d336..b773a4976d1b 100644
--- a/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
@@ -1065,9 +1065,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1082,6 +1079,9 @@ &pcie4_phy {
 };
 
 &pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
@@ -1099,9 +1099,6 @@ wifi@0 {
 };
 
 &pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -1110,6 +1107,11 @@ &pcie6a {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pcie6a_phy {
 	vdda-phy-supply = <&vreg_l1d_0p8>;
 	vdda-pll-supply = <&vreg_l2j_1p2>;
diff --git a/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi b/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
index 6ab595b6ea30..dd2de1f723b0 100644
--- a/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
@@ -964,9 +964,6 @@ wifi@0 {
 };
 
 &pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -982,6 +979,11 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pm8550_gpios {
 	rtmr0_default: rtmr0-reset-n-active-state {
 		pins = "gpio10";
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
index bd0e3009fb41..beb1475d7fa0 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
@@ -1126,9 +1126,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1143,6 +1140,9 @@ &pcie4_phy {
 };
 
 &pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts b/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
index 763efb9e070d..23a298248a29 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
@@ -1033,9 +1033,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1050,6 +1047,8 @@ &pcie4_phy {
 };
 
 &pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
@@ -1067,10 +1066,6 @@ wifi@0 {
 };
 
 &pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -1086,6 +1081,11 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pm8550_gpios {
 	rtmr0_default: rtmr0-reset-n-active-state {
 		pins = "gpio10";
diff --git a/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts b/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
index ab309d547ed5..500809772097 100644
--- a/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
+++ b/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
@@ -1131,9 +1131,6 @@ &mdss_dp3_phy {
 };
 
 &pcie4 {
-	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
-
 	pinctrl-0 = <&pcie4_default>;
 	pinctrl-names = "default";
 
@@ -1148,6 +1145,9 @@ &pcie4_phy {
 };
 
 &pcie4_port0 {
+	reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
+
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
@@ -1165,9 +1165,6 @@ wifi@0 {
 };
 
 &pcie6a {
-	perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
-
 	vddpe-3v3-supply = <&vreg_nvme>;
 
 	pinctrl-0 = <&pcie6a_default>;
@@ -1183,6 +1180,11 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pcie6a_port0 {
+	reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
+};
+
 &pm8550_pwm {
 	status = "okay";
 };
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
From: Liu Ying @ 2026-03-30  2:05 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel
In-Reply-To: <crqzju5cqhvmy5msxvuquydmnpb2ft2t3gsyr6qsre6ccqjvzz@46gfcrelczsr>

On Sun, Mar 29, 2026 at 07:42:23PM +0200, Marco Felsch wrote:
> Hi Liu,

Hi Marco,

> 
> sorry for not writting back earlier, the last weeks were quite busy.
> 
> On 26-03-29, Liu Ying wrote:
>> LDB's parent device could be a syscon which doesn't allow a reg property
>> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
>> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
>> without a reg property(LDB is also a child device of the Media blk-ctrl).
>> To make the LDB schema be able to describe LDBs without the reg property
>> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> 
> NACK, we want to describe the HW and from HW PoV the LDB is and was
> always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.

The reality is that i.MX6SX and i.MX8MP LDB DT nodes are already in-tree.
People may take them as ABI(not only for Linux, but also for other
potential projects which use the LDB schema and/or the DT nodes).

> 
>> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> 
> Therefore I would just revert this patch completely.

IMHO, it doesn't make too much difference between my patch and reverting
this offending patch, because of the ABI, i.e., the reg properties in
i.MX6SX and i.MX8MP LDB DT nodes are supposed to be stable.

I feel that what you are asking for is even more than simply reverting
this offending patch, that is to say, completely disallowing the reg and
reg-names properties for LDBs across all SoCs.  But again, that would
break the ABI.

> 
> Regards,
>   Marco
> 
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> index 7f380879fffd..5f6dc2b11d7b 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> @@ -28,6 +28,7 @@ properties:
>>      const: ldb
>>  
>>    reg:
>> +    minItems: 1
>>      maxItems: 2
>>  
>>    reg-names:
>> @@ -68,7 +69,6 @@ required:
>>    - compatible
>>    - clocks
>>    - ports
>> -  - reg
>>  
>>  allOf:
>>    - if:
>> @@ -83,12 +83,23 @@ allOf:
>>          ports:
>>            properties:
>>              port@2: false
>> +
>>    - if:
>> -      not:
>> -        properties:
>> -          compatible:
>> -            contains:
>> -              const: fsl,imx6sx-ldb
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - fsl,imx6sx-ldb
>> +              - fsl,imx8mp-ldb
>> +    then:
>> +      required:
>> +        - reg
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: fsl,imx8mp-ldb
>>      then:
>>        required:
>>          - reg-names
>>
>> ---
>> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
>> change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3
>>
>> Best regards,
>> -- 
>> Liu Ying <victor.liu@nxp.com>
>>
>>
> 

-- 
Regards,
Liu Ying

^ permalink raw reply

* Re: [PATCH 00/10] Synopsys DisplayPort Controller improvements for Rockchip platforms
From: Chaoyi Chen @ 2026-03-30  1:34 UTC (permalink / raw)
  To: Sebastian Reichel, Sandy Huang, Heiko Stübner, Andy Yan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Damon Ding, Dmitry Baryshkov, Alexey Charkov, dri-devel,
	linux-rockchip, linux-kernel, devicetree, kernel
In-Reply-To: <20260326-synopsys-dw-dp-improvements-v1-0-501849162290@collabora.com>

Hello Sebastian, 

On 3/27/2026 1:31 AM, Sebastian Reichel wrote:
> This patch series updates the Synopsys Designware DisplayPort bridge
> together with the only existing user: The Rockchip RK3576/RK3588:
> 
>  1. follow-up bridges (PHY, USB-C connector)
>     this is needed to get USB-C DP AltMode working; I've followed the
>     Qualcomm driver as reference
> 
>  2. runtime PM
>     the initial driver has been upstreamed without RPM; add it to
>     avoid wasting power when nothing is plugged
> 
>  3. audio
>     the initial driver has been upstreamed without audio support;
>     this adds all missing bits for audio with single stream transport
> 
> The series is based on drm-misc-next with Cristian's cleanup series
> applied as I expect that to land first:
> 
> https://lore.kernel.org/linux-rockchip/20260310-drm-rk-fixes-v2-0-645ecfb43f49@collabora.com/
> 
> To properly make use of the bridge code the following USBDP PHY series
> is also needed:
> 
> https://lore.kernel.org/linux-rockchip/20260313-rockchip-usbdp-cleanup-v3-0-3e8fe89a35b5@collabora.com/
> 
> There are two parts, which possibly need some discussion:
> 
>  1. I added a dedicated bridge callback for out-of-band hotplug events,
>     which is separate from the hotplug_notify. I have a feeling, that
>     there might be a better solution, but haven't found it.
> 

Could you explain what an out-of-band hotplug event is?

Can't the drivers/usb/typec/altmodes/displayport.c respond to these
hot-plug events? Thank you.


>  2. The DT binding for audio support - explicitly marked as RFC - works
>     perfectly fine, but is not ready for MST. I don't intend to
>     implement that right now, but the binding should obviously take it
>     into consideration to avoid breaking it in the future. I've put
>     some points for discussion into the relevant patch.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> Sebastian Reichel (10):
>       drm/bridge: synopsys: dw-dp: Simplify driver data setting
>       drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED
>       drm/bridge: synopsys: dw-dp: Add follow-up bridge support
>       drm/bridge: Add out-of-band HPD notify handler
>       drm/bridge: synopsys: dw-dp: Support software triggered OOB HPD
>       drm/rockchip: dw_dp: Implement out-of-band HPD handling
>       drm/bridge: synopsys: dw-dp: Add Runtime PM support
>       drm/rockchip: dw_dp: Add runtime PM support
>       [RFC] dt-bindings: display: rockchip: dw-dp: fix sound DAI cells
>       drm/bridge: synopsys: dw-dp: Add audio support
> 
>  .../bindings/display/rockchip/rockchip,dw-dp.yaml  |   5 +-
>  drivers/gpu/drm/bridge/synopsys/dw-dp.c            | 284 ++++++++++++++++++++-
>  drivers/gpu/drm/display/drm_bridge_connector.c     |   6 +
>  drivers/gpu/drm/rockchip/dw_dp-rockchip.c          | 167 +++++++++++-
>  include/drm/bridge/dw_dp.h                         |   6 +
>  include/drm/drm_bridge.h                           |  14 +
>  6 files changed, 469 insertions(+), 13 deletions(-)
> ---
> base-commit: 0660ee19141e5e90b422b7daa0d8518a8d0d898b
> change-id: 20260325-synopsys-dw-dp-improvements-7da2e98df1dd
> 
> Best regards,

-- 
Best, 
Chaoyi


^ permalink raw reply

* Re: [PATCH 4/8] drm/bridge: dw-hdmi: document the output_port field
From: Damon Ding @ 2026-03-30  1:13 UTC (permalink / raw)
  To: Luca Ceresoli, Liu Ying, Marek Vasut, Stefan Agner,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Rob Herring,
	Saravana Kannan
  Cc: Kory Maincent (TI.com), Hervé Codina, Hui Pu, Ian Ray,
	Thomas Petazzoni, dri-devel, imx, linux-arm-kernel, linux-kernel,
	devicetree, Adam Ford, Alexander Stein, Anson Huang,
	Christopher Obbard, Daniel Scally, Emanuele Ghidoli,
	Fabio Estevam, Francesco Dolcini, Frieder Schrempf, Gilles Talis,
	Goran Rađenović, Heiko Schocher, Joao Paulo Goncalves,
	Josua Mayer, Kieran Bingham, Marco Felsch, Martyn Welch,
	Oleksij Rempel, Peng Fan, Philippe Schenker, Richard Hu,
	Shengjiu Wang, Stefan Eichenberger, Vitor Soares
In-Reply-To: <DHDIOSVSDG9W.B7BW87297KT3@bootlin.com>

On 3/27/2026 7:10 PM, Luca Ceresoli wrote:
> Hello Damon,
> 
> On Thu Mar 26, 2026 at 10:15 AM CET, Damon Ding wrote:
>> On 3/26/2026 3:25 PM, Liu Ying wrote:
>>> Hi Luca,
>>>
>>> On Fri, Mar 20, 2026 at 11:46:15AM +0100, Luca Ceresoli wrote:
>>>> The meaning of this flag may not be obvious at first sight.
>>>>
>>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>>> ---
>>>>    include/drm/bridge/dw_hdmi.h | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>
>> First of all, these changes related to the DW HDMI controller work well
>> when tested on RK3399 HDMI.
> 
> Great!
> 
> You'd be welcome to send your Tested-by: tag if you tested the series on
> hardware, that would be useful.
> 
> However at this point I suggest to wait for v2, which I'm sending soon, and
> test that. I added you in Cc for it.
> 
>>>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>>>> index 336f062e1f9d..45f6ba1a8ee1 100644
>>>> --- a/include/drm/bridge/dw_hdmi.h
>>>> +++ b/include/drm/bridge/dw_hdmi.h
>>>> @@ -126,6 +126,11 @@ struct dw_hdmi_phy_ops {
>>>>    struct dw_hdmi_plat_data {
>>>>    	struct regmap *regm;
>>>>
>>>> +	/*
>>>> +	 * The HDMI output port number (which must be 1) if it is described
>>>
>>> I'd rephrase:
>>> The HDMI output port number must be 1 ...
>>>
>>
>> Yes, the output port number should be 1, but I found that the output
>> port number in the Rockchip-side dw-hdmi driver remains 0.
> 
> Really? I checked all the bindings in
> Documentation/devicetree/bindings/display/rockchip/*hdmi* and all mention
> port@1 as the output port number. Can you point to code using port@0 as the
> output port?
> 
> Should it be true, that would be unfortunate because the output_port
> variable does not handle this case. It's used as a sort of bool-or-int
> variable:
> 
>   * as a bool [0] to find out whether the DT is supposed to describe the
>     output port
>   * as an integer to tell the port number to parse in DT [1]
> 
> So saying "please parse port 0" is impossible.
> 
> [0] https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L3310
> [1] https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L3315
> 

Aha, my description might be a little misleading. The 
&dw_hdmi_plat_data.output_port is 0 on the Rockchip side, so the next 
bridge will not be parsed for it.

Then I think the &dw_hdmi_plat_data.output_port should be 1, as this 
helps support the hdmi-connector and other bridge chips.

>> Therefore, it may be better to adapt the dw-hdmi drivers across all
>> platforms to the bridge-connector architecture simultaneously.
>>
>> This would allow removing &dw_hdmi_plat_data.output_port and unify the
>> setting of DRM_BRIDGE_ATTACH_NO_CONNECTOR during the attach stage.
>> (Just as the Analogix DP driver does [0])
>>
>> [0]
>> https://lore.kernel.org/all/20260319071452.1961274-1-damon.ding@rock-chips.com/
> 
> I agree converting all users is a good goal, but I disagree it should be
> done simultaneously. There are various users of dw-hdmi, and converting one
> having the hardware to test it was painful enough for me. Converting all of
> them without testing on hardware would be a hell.
> 
> However I might be wrong. Having a precise list of all users, which ones
> need to be converted, and whether they have any special detail to be taken
> care of would be good to estimate the work to convert all users. Without
> that I'd rather let users convert one by one and hopefully get rid of
> legacy code eventually.
> 

Yes, I've noticed that there are quite a few drivers associated with the 
DW HDMI controller. It would be a better idea to let users handle this 
themselves.

BTW: The Rockchip side dw-hdmi patches for bridge connector support will 
be updated as a follow-up to your patch series. :-)

Best regards,
Damon


^ permalink raw reply

* Re: [PATCH v3 2/3] riscv: dts: spacemit: Define the P1 PMIC regulators for OrangePi RV2
From: Yixun Lan @ 2026-03-30  0:31 UTC (permalink / raw)
  To: Han Gao
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, devicetree,
	linux-riscv, spacemit, linux-kernel, Han Gao
In-Reply-To: <492e0bea0fb84f49f4bead3dc3d563b13d98df11.1774803532.git.gaohan@iscas.ac.cn>

Hi Han,

On 01:05 Mon 30 Mar     , Han Gao wrote:
> Define the DC power input and the 4v power as fixed regulator supplies.
> 
> Define the SpacemiT P1 PMIC voltage regulators and their constraints.
> 
..
> The power management hardware design on the OrangePi RV2 is identical to
> the Banana Pi BPI-F3, so the DT Nodes were taken from k1-bananapi-f3.dts.
I'd suggest to drop above comment, it's vague and even not a direct reason..

> 
> Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
> ---
>  .../boot/dts/spacemit/k1-orangepi-rv2.dts     | 137 ++++++++++++++++++
>  1 file changed, 137 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts b/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
> index 93880ba7bdfe..1b1b27bc95d8 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts
> @@ -23,6 +23,25 @@ chosen {
>  		stdout-path = "serial0";
>  	};
>  
..
> +	reg_dc_in: regulator-vcc-in-5v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "dc_in_5v";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
This regulator can be dropped, it's not used by *device*, take a look at Krzysztof's comment

https://lore.kernel.org/all/6530526f-59ca-4753-a068-46c62a1a1fed@kernel.org

-- 
Yixun Lan (dlan)

^ permalink raw reply

* Re: [PATCH v4 0/5] Add initial Milk-V Duo S board support
From: Michael Opdenacker @ 2026-03-29 21:14 UTC (permalink / raw)
  To: Joshua Milas, tglx, robh, krzk+dt, conor+dt, pjw, samuel.holland,
	unicorn_wang, inochiama, daniel.lezcano, palmer, aou, alex,
	liujingqi, alexander.sverdlin, rabenda.cn, dlan, chao.wei, anup
  Cc: michael.opdenacker, linux-kernel, devicetree, linux-riscv, sophgo,
	hanguidong02
In-Reply-To: <20260328173450.219664-1-josh.milas@gmail.com>

Hi Joshua

On 3/28/26 6:34 PM, Joshua Milas wrote:
> This adds an initial device tree for the Milk-V Duo S board
> with support for reading from the SD card and network over
> Ethernet. This is continued work from Michael Opdenacker's
> v6 series [1] on the ARM64 and RISCV side. It has been tested
> with ARM64 and RISCV64 to boot from an SD card, have networking,
> and read I2C slave devices over i2c4.

I could test successfully on RISC-V, booting from MMC on a command line 
shell, and testing i2c4. However, I have an issue with Ethernet. When I 
add "ip=172.24.0.2" (for example) to the kernel command line, I'm 
getting this:

[    2.586984] stmmaceth 4070000.ethernet eth0: Register 
MEM_TYPE_PAGE_POOL RxQ-0
[    2.596428] stmmaceth 4070000.ethernet eth0: cannot attach to PHY 
(error: -ENODEV)
[    2.627351] IP-Config: Failed to open eth0
[    2.631859] IP-Config: No network devices available

However, "eth0" appears in the list of networking devices.
Does this work on your side?
Cheers
Michael.

-- 
Root Commit
Embedded Linux Training and Consulting
https://rootcommit.com


^ permalink raw reply

* Re: [PATCH net-next v5 00/14] macb usrio/tsu patches
From: patchwork-bot+netdevbpf @ 2026-03-29 21:40 UTC (permalink / raw)
  To: Conor Dooley
  Cc: netdev, conor.dooley, Valentina.FernandezAlanis, andrew+netdev,
	davem, edumazet, kuba, pabeni, robh, krzk+dt, conor+dt,
	daire.mcnamara, pjw, palmer, aou, alex, nicolas.ferre,
	claudiu.beznea, richardcochran, samuel.holland, devicetree,
	linux-kernel, linux-riscv, dave.stevenson, sean.anderson,
	vineeth.karumanchi, abin.joseph, theo.lebrun, Ryan.Wanner,
	haokexin
In-Reply-To: <20260325-unsterile-flail-4c7729750dc4@spud>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 25 Mar 2026 16:28:04 +0000 you wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Hey folks,
> 
> At the very least, it'd be good of the soc vendor folks could check
> their platforms and see if their usrio stuff actually lines up with what
> the driver currently calls "macb_default_usrio". Ours didn't and it was
> a nasty surprise.
> 
> [...]

Here is the summary with links:
  - [net-next,v5,01/14] Revert "net: macb: Clean up the .usrio settings in macb_config instances"
    https://git.kernel.org/netdev/net-next/c/8ccf062c6770
  - [net-next,v5,02/14] net: macb: rename macb_default_usrio to at91_default_usrio as not all platforms have mii mode control in usrio
    https://git.kernel.org/netdev/net-next/c/a17871778ee2
  - [net-next,v5,03/14] net: macb: split USRIO_HAS_CLKEN capability in two
    https://git.kernel.org/netdev/net-next/c/039f185a0060
  - [net-next,v5,04/14] dt-bindings: net: cdns,macb: replace cdns,refclk-ext with cdns,refclk-source
    https://git.kernel.org/netdev/net-next/c/dfa36d7e860c
  - [net-next,v5,05/14] net: macb: rework usrio refclk selection code
    https://git.kernel.org/netdev/net-next/c/6c5b565d7d41
  - [net-next,v5,06/14] net: macb: np4 doesn't need a usrio pointer
    https://git.kernel.org/netdev/net-next/c/826cbe636e10
  - [net-next,v5,07/14] net: macb: add mpfs specific usrio configuration
    https://git.kernel.org/netdev/net-next/c/c711311d6ba3
  - [net-next,v5,08/14] net: macb: warn on pclk use as a tsu_clk fallback
    https://git.kernel.org/netdev/net-next/c/3fe13d858f83
  - [net-next,v5,09/14] net: macb: clean up tsu clk rate acquisition
    https://git.kernel.org/netdev/net-next/c/b698a1e397ab
  - [net-next,v5,10/14] dt-bindings: net: macb: add property indicating timer adjust mode
    https://git.kernel.org/netdev/net-next/c/09a6164a4f1d
  - [net-next,v5,11/14] net: macb: timer adjust mode is not supported
    https://git.kernel.org/netdev/net-next/c/41adda8764fd
  - [net-next,v5,12/14] net: macb: runtime detect MACB_CAPS_USRIO_DISABLED
    https://git.kernel.org/netdev/net-next/c/47c86c463612
  - [net-next,v5,13/14] net: macb: set MACB_CAPS_USRIO_DISABLED if no usrio config is provided
    https://git.kernel.org/netdev/net-next/c/32fc6a9f6e75
  - [net-next,v5,14/14] net: macb: drop usrio pointer on EyeQ5 config
    https://git.kernel.org/netdev/net-next/c/cd1082a96f9a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH v4 2/5] arm64: dts: sophgo: add initial Milk-V Duo S board support
From: Michael Opdenacker @ 2026-03-29 20:21 UTC (permalink / raw)
  To: Joshua Milas, tglx, robh, krzk+dt, conor+dt, pjw, samuel.holland,
	unicorn_wang, inochiama, daniel.lezcano, palmer, aou, alex,
	liujingqi, alexander.sverdlin, rabenda.cn, dlan, chao.wei, anup
  Cc: michael.opdenacker, linux-kernel, devicetree, linux-riscv, sophgo,
	hanguidong02
In-Reply-To: <20260328173450.219664-3-josh.milas@gmail.com>

Hi Joshua

Thanks a lot for this new update!

On 3/28/26 6:34 PM, Joshua Milas wrote:
> Adds initial arm64 support for the Milk-V Duo S board
> [1] making it possible to boot Linux to the command line.
>
> Link: https://milkv.io/duo-s [1]
>
> Signed-off-by: Joshua Milas <josh.milas@gmail.com>
...
> diff --git a/arch/arm64/boot/dts/sophgo/sg2000-milkv-duo-s.dts b/arch/arm64/boot/dts/sophgo/sg2000-milkv-duo-s.dts
> new file mode 100644
> index 0000000000000..4ae44b40f9edb
> --- /dev/null
> +++ b/arch/arm64/boot/dts/sophgo/sg2000-milkv-duo-s.dts
...
> +
> +&usb {
> +       dr_mode = "host";
> +       status = "okay";
> +};


You have a small checkpatch.pl issue here:

./scripts/checkpatch.pl *.patch
...
WARNING: please, no spaces at the start of a line
#120: FILE: arch/riscv/boot/dts/sophgo/sg2000-milkv-duo-s.dts:82:
+       dr_mode = "host";$

WARNING: please, no spaces at the start of a line
#121: FILE: arch/riscv/boot/dts/sophgo/sg2000-milkv-duo-s.dts:83:
+       status = "okay";$

Replace the spaces by a tab as in the other entries in the DTS and you 
should be fine.
Thanks again
Cheers
Michael.

-- 
Root Commit
Embedded Linux Training and Consulting
https://rootcommit.com


^ permalink raw reply

* [PATCH v2 2/2] ARM: dts: gemini: Rename power controller node to gemini-poweroff
From: Khushal Chitturi @ 2026-03-29 20:51 UTC (permalink / raw)
  To: sre, robh, krzk+dt, conor+dt, ulli.kroll, linusw
  Cc: daniel.baluta, simona.toaca, d-gole, m-chawdhry, linux-pm,
	devicetree, linux-arm-kernel, linux-kernel, Khushal Chitturi
In-Reply-To: <20260329205151.15161-1-khushalchitturi@gmail.com>

Update the node name for the Cortina Gemini power controller from
power-controller to gemini-poweroff since node "power controller" is
reserved for power domain controller.

Signed-off-by: Khushal Chitturi <khushalchitturi@gmail.com>
---
 arch/arm/boot/dts/gemini/gemini.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/gemini/gemini.dtsi b/arch/arm/boot/dts/gemini/gemini.dtsi
index befe322bd7de..c524adadcf81 100644
--- a/arch/arm/boot/dts/gemini/gemini.dtsi
+++ b/arch/arm/boot/dts/gemini/gemini.dtsi
@@ -228,7 +228,7 @@ intcon: interrupt-controller@48000000 {
 			#interrupt-cells = <2>;
 		};
 
-		power-controller@4b000000 {
+		gemini-poweroff@4b000000 {
 			compatible = "cortina,gemini-power-controller";
 			reg = <0x4b000000 0x100>;
 			interrupts = <26 IRQ_TYPE_EDGE_RISING>;
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: power: reset: cortina,gemini-power-controller: convert to DT schema
From: Khushal Chitturi @ 2026-03-29 20:51 UTC (permalink / raw)
  To: sre, robh, krzk+dt, conor+dt, ulli.kroll, linusw
  Cc: daniel.baluta, simona.toaca, d-gole, m-chawdhry, linux-pm,
	devicetree, linux-arm-kernel, linux-kernel, Khushal Chitturi
In-Reply-To: <20260329205151.15161-1-khushalchitturi@gmail.com>

Convert the Cortina Systems Gemini Poweroff Controller bindings to
DT schema.

Signed-off-by: Khushal Chitturi <khushalchitturi@gmail.com>
---
Changelog:
v1 -> v2:
- Renamed the node from power-controller to gemini-poweroff to resolve dtschema warnings.

Note:
* This patch series is part of the GSoC2026 application process for device tree bindings conversions
* https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings

 .../cortina,gemini-power-controller.yaml      | 42 +++++++++++++++++++
 .../bindings/power/reset/gemini-poweroff.txt  | 17 --------
 2 files changed, 42 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/cortina,gemini-power-controller.yaml
 delete mode 100644 Documentation/devicetree/bindings/power/reset/gemini-poweroff.txt

diff --git a/Documentation/devicetree/bindings/power/reset/cortina,gemini-power-controller.yaml b/Documentation/devicetree/bindings/power/reset/cortina,gemini-power-controller.yaml
new file mode 100644
index 000000000000..8fbe7e952b25
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/cortina,gemini-power-controller.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/cortina,gemini-power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cortina Systems Gemini Poweroff Controller
+
+maintainers:
+  - Linus Walleij <linusw@kernel.org>
+
+description: |
+  The Gemini power controller is a dedicated IP block in the Cortina Gemini SoC that
+  controls system power-down operations.
+
+properties:
+  compatible:
+    const: cortina,gemini-power-controller
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    gemini-poweroff@4b000000 {
+      compatible = "cortina,gemini-power-controller";
+      reg = <0x4b000000 0x100>;
+      interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/power/reset/gemini-poweroff.txt b/Documentation/devicetree/bindings/power/reset/gemini-poweroff.txt
deleted file mode 100644
index 7fec3e100214..000000000000
--- a/Documentation/devicetree/bindings/power/reset/gemini-poweroff.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-* Device-Tree bindings for Cortina Systems Gemini Poweroff
-
-This is a special IP block in the Cortina Gemini SoC that only
-deals with different ways to power the system down.
-
-Required properties:
-- compatible: should be "cortina,gemini-power-controller"
-- reg: should contain the physical memory base and size
-- interrupts: should contain the power management interrupt
-
-Example:
-
-power-controller@4b000000 {
-	compatible = "cortina,gemini-power-controller";
-	reg = <0x4b000000 0x100>;
-	interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
-};
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 0/2] dt-bindings: power: reset: cortina: Convert to DT schema and rename node
From: Khushal Chitturi @ 2026-03-29 20:51 UTC (permalink / raw)
  To: sre, robh, krzk+dt, conor+dt, ulli.kroll, linusw
  Cc: daniel.baluta, simona.toaca, d-gole, m-chawdhry, linux-pm,
	devicetree, linux-arm-kernel, linux-kernel, Khushal Chitturi

Convert the Cortina Systems Gemini Poweroff Controller bindings to
DT schema and update corresponding dtsi file with new node name

---
Khushal Chitturi (2):
  dt-bindings: power: reset: cortina,gemini-power-controller: convert to
    DT schema
  ARM: dts: gemini: Rename power controller node to gemini-poweroff

 .../cortina,gemini-power-controller.yaml      | 42 +++++++++++++++++++
 .../bindings/power/reset/gemini-poweroff.txt  | 17 --------
 arch/arm/boot/dts/gemini/gemini.dtsi          |  2 +-
 3 files changed, 43 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/cortina,gemini-power-controller.yaml
 delete mode 100644 Documentation/devicetree/bindings/power/reset/gemini-poweroff.txt

-- 
2.53.0


^ permalink raw reply

* [PATCH] arm64: dts: qcom: msm8939-asus-z00t: add regulators for ambient light and proximity sensor
From: Erikas Bitovtas @ 2026-03-29 23:37 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel,
	~postmarketos/upstreaming, phone-devel, Erikas Bitovtas

VCNL4000 includes support for regulators. Add regulators listed in the
downstream device tree so they can be powered in during initialization.
VLED supply is missing downstream, so it will be powered on by a dummy.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
arm64: dts: qcom: msm8939-asus-z00t: add regulators for ambient light
and proximity sensor.

This patch series describes regulators needed for the ambient light and
proximity sensor.

In the patch series which added the ambient light and proximity sensor
for this device, regulators were missing, as they were not yet supported
by the sensor's device tree binding. Missing regulators for the ambient
light and proximity sensor resulted in it not powering on when a
regulator supplied only the ambient light and proximity sensor. In Asus
ZenFone 2 Laser case, the regulators were shared with the panel and
magnetometer, so it was not affected by this bug. However, for the sake
of completeness of the binding, we add the neccessary regulators anyway.

The mainline binding for the sensor also includes vled-supply, however,
in downstream, it is missing, and we have no datasheet for the device,
so for now, we'll have it being supplied by a dummy.
---
 arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts b/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
index 1a340fa92755..90e966242720 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
@@ -154,6 +154,9 @@ light-sensor@60 {
 		interrupts-extended = <&tlmm 113 IRQ_TYPE_EDGE_FALLING>;
 		proximity-near-level = <30>;
 
+		vdd-supply = <&pm8916_l8>;
+		vio-supply = <&pm8916_l6>;
+
 		pinctrl-0 = <&light_int_default>;
 		pinctrl-names = "default";
 	};

---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260325-z00t-cm36686-regulators-8f138d47090c

Best regards,
--  
Erikas Bitovtas <xerikasxx@gmail.com>


^ permalink raw reply related

* Re: [PATCH net-next v2 9/9] net: dsa: lan9645x: add port statistics
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-9-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add port statistics
    
Add statistics support for the port counters. Chip registers are 32 bit,
so this unit is responsible maintaining a 64bit software cache, and
updating it frequently to handle overflows in hardware.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 24a60f40f6b8..9a46c08dad59 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -262,6 +264,12 @@ static int lan9645x_setup(struct dsa_switch *ds)
>  	if (!lan9645x->owq)
>  		return -ENOMEM;
>  
> +	err = lan9645x_stats_init(lan9645x);
> +	if (err) {
> +		dev_err(dev, "Failed to init stats.\n");
> +		goto owq_destroy;
> +	}
> +
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
> index 000000000000..3e82a859f173
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
[ ... ]
> +static const struct lan9645x_ethtool_stat lan9645x_port_ethtool_stats[] = {
> +	{ "rx_uc",              SCNT_RX_UC },
> +	{ "rx_cat_drop",        SCNT_RX_CAT_DROP },
[ ... ]
> +	{ "tx_uc",              SCNT_TX_UC },
> +	{ "tx_drop",            SCNT_TX_DROP },
[ ... ]
> +};

Is it necessary to duplicate standard networking statistics in the driver
private ethtool output?

The networking subsystem guidelines state that stats possessing a standard UAPI
representation, such as tx_dropped and rx_packets in struct rtnl_link_stats64,
should not be duplicated in the ethtool -S output.

Could these redundant strings be removed since the driver natively supports
them via lan9645x_stats_get_stats64?

[ ... ]
> +static const struct lan9645x_view_stats lan9645x_view_stat_cfgs[] = {
> +	[LAN9645X_STAT_PORTS] = {
> +		.name = "ports",
> +		.type = LAN9645X_STAT_PORTS,
> +		.layout = lan9645x_port_stats_layout,
> +		.num_cnts = ARRAY_SIZE(lan9645x_port_stats_layout),
> +		.num_indexes = NUM_PHYS_PORTS,
> +	},
> +};
[ ... ]
> +static u64 *lan9645x_stats_view_idx_update(struct lan9645x *lan9645x,
> +					   enum lan9645x_view_stat_type vtype,
> +					   int idx)
> +{
> +	struct lan9645x_stats *s = lan9645x->stats;
> +
> +	mutex_lock(&s->hw_lock);
> +	__lan9645x_stats_view_idx_update(lan9645x, vtype, idx);
> +	mutex_unlock(&s->hw_lock);
> +
> +	return STAT_COUNTERS(lan9645x, vtype, idx);
> +}

Does this code overflow the allocated buffer when called for the CPU port?

The stats buffer cnts is allocated using num_indexes, which is NUM_PHYS_PORTS
(9). This covers ports 0 to 8. However, the CPU port is defined as index 9
(CPU_PORT = 9).

When the DSA core invokes stat callbacks like get_ethtool_stats or get_stats64
for the CPU port, the idx argument is 9. While the helper
__lan9645x_stats_view_idx_update skips the hardware read for out-of-bounds
indices, lan9645x_stats_view_idx_update ignores that failure and returns the
result of STAT_COUNTERS(lan9645x, vtype, idx).

This computes a pointer past the end of the cnts array, which the callers then
blindly dereference, potentially leaking kernel memory or causing a crash. Can
bounds checking be added to the callers, or should the buffer be sized to
include the CPU port?

[ ... ]
> +void lan9645x_stats_get_ethtool_stats(struct lan9645x *lan9645x, int port,
> +				      u64 *data)
> +{
> +	struct lan9645x_stats *stats = lan9645x->stats;
> +	u64 *c;
> +	int i;
> +
> +	c = lan9645x_stats_view_idx_update(lan9645x, LAN9645X_STAT_PORTS, port);
> +
> +	spin_lock(&stats->sw_lock);
> +	for (i = 0; i < ARRAY_SIZE(lan9645x_port_ethtool_stats); i++)
> +		*data++ = c[lan9645x_port_ethtool_stats[i].idx];
> +	spin_unlock(&stats->sw_lock);
> +}
[ ... ]
> +/* Called in atomic context */
> +void lan9645x_stats_get_stats64(struct lan9645x *lan9645x, int port,
> +				struct rtnl_link_stats64 *stats)
> +{
> +	struct lan9645x_stats *s = lan9645x->stats;
> +	u64 *c;
> +
> +	c = STAT_COUNTERS(lan9645x, LAN9645X_STAT_PORTS, port);
> +
> +	spin_lock(&s->sw_lock);

Can this code deadlock when a softirq interrupts a process holding the lock?

The sw_lock spinlock protects the 64-bit software stats counters. In
lan9645x_stats_get_stats64, which acts as the ndo_get_stats64 callback, this
lock is acquired using a plain spin_lock. The code comment notes this function
is called in atomic context (such as from NET_RX_SOFTIRQ via dev_get_stats).

Concurrently, process context paths like lan9645x_stats_get_ethtool_stats and
lan9645x_check_stats_work also acquire sw_lock using plain spin_lock. Because
plain spin_lock does not disable softirqs, if a softirq preempts the execution
on a CPU while process context holds sw_lock, the softirq will spin
indefinitely waiting for the lock.

Should the process-context acquisitions use spin_lock_bh instead?

[ ... ]
> +int lan9645x_stats_init(struct lan9645x *lan9645x)
> +{
> +	const struct lan9645x_view_stats *vs;
> +	struct lan9645x_stats *stats;
> +	int err, i;
> +
> +	lan9645x->stats = devm_kzalloc(lan9645x->dev, sizeof(*stats),
> +				       GFP_KERNEL);

Will this memory be leaked when the DSA switch unbinds and rebinds?

The lan9645x_stats_init function allocates lan9645x->stats, vstat->cnts, and
vstat->buf using devm_kzalloc and devm_kcalloc. This function is called from
lan9645x_setup, which is the DSA .setup callback.

The DSA tree lifecycle is independent of the platform device lifecycle. If the
master netdevice is unbound and bound again, the DSA switch's .teardown and
.setup callbacks will be invoked multiple times. Because .teardown cannot free
devres-managed memory, the memory tied to the platform device will be leaked
on every setup cycle until the switch's platform device itself is destroyed.

Could this use standard kzalloc allocations paired with kfree in teardown?

^ permalink raw reply

* Re: [PATCH net-next v2 8/9] net: dsa: lan9645x: add mdb management
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-8-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: dsa: lan9645x: add mdb management

Add support for dsa mdb callbacks. L2 multicast and IP multicast is
handled differently. IP multicast stores the port group mask inline in
the mac table. L2 multicast points to a PGID index, which encodes the
port group mask.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c
> new file mode 100644
> index 000000000000..4cf92887a4eb
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c
> @@ -0,0 +1,391 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2026 Microchip Technology Inc.
> + */
> +
> +#include "lan9645x_main.h"

[ ... ]

> +static void lan9645x_mdb_encode_mac(unsigned char *mac,
> +				    struct lan9645x_mdb_entry *mdb_entry,
> +				    enum macaccess_entry_type type)
> +{
> +	ether_addr_copy(mac, mdb_entry->mac);
> +
> +	/* The HW encodes the portmask in the high bits of the mac for ip
> +	 * multicast entries, to save on the limited PGID resources.
> +	 *
> +	 * IPv4 Multicast DMAC: 0x01005Exxxxxx
> +	 * IPv6 Multicast DMAC: 0x3333xxxxxxxx
> +	 *
> +	 * which gives us 24 or 16 bits to encode the portmask.
> +	 */
> +	if (type == ENTRYTYPE_MACV4) {
> +		mac[0] = 0;
> +		mac[1] = mdb_entry->ports >> 8;
> +		mac[2] = mdb_entry->ports & 0xff;
> +	} else if (type == ENTRYTYPE_MACV6) {
> +		mac[0] = mdb_entry->ports >> 8;
> +		mac[1] = mdb_entry->ports & 0xff;
> +	}
> +}

Does this code need to mask out BIT(CPU_PORT) from mdb_entry->ports?

For L2 multicast, the CPU port is explicitly stripped from the PGID portmask
to avoid conflicting instructions, since CPU_PORT forwarding is handled by the
cpu_copy flag. Here, the IP multicast portmask is encoded directly into the
MAC address using the full mdb_entry->ports. Could this cause hardware to
receive conflicting instructions (forwarding to port 9 and copying to the CPU
extraction queue), leading to duplicate multicast packets for the CPU?

[ ... ]

> +static struct lan9645x_pgid_entry *
> +lan9645x_mdb_pgid_entry_create(struct lan9645x *lan9645x, u16 ports)
> +{
> +	struct lan9645x_pgid_entry *pgid_entry = NULL;
> +	int index;
> +
> +	for (index = PGID_GP_START; index < PGID_GP_END; index++) {

Is it safe to start the search at PGID_GP_START (defined as CPU_PORT)?

Hardware uses ANA_PGID(DEST_IDX) to forward unicast traffic, and MAC table
entries for the CPU have DEST_IDX = 9. When the first L2 multicast group is
added, it appears this loop will allocate PGID index 9, overwriting the
unicast portmask for the CPU port. Could this cause all unicast traffic
destined for the CPU to be flooded to the external physical ports in the
multicast group instead?

> +		bool used = false;
> +
> +		list_for_each_entry(pgid_entry, &lan9645x->pgid_entries, list) {
> +			if (pgid_entry->index == index) {
> +				used = true;
> +				break;
> +			}
> +		}
> +
> +		if (!used)
> +			return lan9645x_pgid_entry_alloc(lan9645x, index,
> +							 ports);
> +	}
> +
> +	return ERR_PTR(-ENOSPC);
> +}

[ ... ]

> +static int __lan9645x_mdb_add(struct lan9645x *lan9645x, int chip_port,
> +			      const unsigned char addr[ETH_ALEN], u16 vid,
> +			      enum macaccess_entry_type type)
> +{
> +	struct lan9645x_pgid_entry *old_pgid, *new_pgid;
> +	struct lan9645x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +	int err, pgid_index;
> +	bool cpu_copy;
> +
> +	mdb_entry = lan9645x_mdb_entry_lookup(lan9645x, addr, vid);
> +	if (!mdb_entry) {
> +		mdb_entry = lan9645x_mdb_entry_alloc(lan9645x, addr, vid);
> +		if (IS_ERR(mdb_entry))
> +			return PTR_ERR(mdb_entry);
> +	}
> +
> +	if (mdb_entry->ports & BIT(chip_port))
> +		return 0;
> +
> +	mdb_entry->ports |= BIT(chip_port);
> +
> +	/* Encode mac for IP mc */
> +	lan9645x_mdb_encode_mac(mac, mdb_entry, type);
> +
> +	/* Update PGID ptr for non-IP entries (L2 multicast) */
> +	old_pgid = mdb_entry->pgid;
> +	new_pgid = lan9645x_mdb_pgid_entry_get(lan9645x, mdb_entry, type);
> +	if (IS_ERR(new_pgid)) {
> +		/* Out of PGIDs or mem. Continue forwarding to old port
> +		 * group, or remove if fresh mdb_entry.
> +		 */
> +		mdb_entry->ports &= ~BIT(chip_port);
> +		if (!mdb_entry->ports)
> +			lan9645x_mdb_entry_dealloc(lan9645x, mdb_entry);
> +
> +		return PTR_ERR(new_pgid);
> +	}
> +	mdb_entry->pgid = new_pgid;
> +
> +	cpu_copy = !!(mdb_entry->ports & BIT(CPU_PORT));
> +	pgid_index = lan9645x_mdb_pgid_index(mdb_entry, type);
> +
> +	/* Make sure to write on top of existing entry, so we do not disrupt
> +	 * flowing traffic.
> +	 */
> +	err = lan9645x_mact_learn_cpu_copy(lan9645x, pgid_index, mac,
> +					   mdb_entry->vid, type, cpu_copy);
> +	lan9645x_pgid_entry_put(lan9645x, old_pgid);
> +	return err;
> +}

What happens if lan9645x_mact_learn_cpu_copy() fails and returns an error?

The software MDB state (mdb_entry->ports and mdb_entry->pgid) is updated and
the old PGID reference is dropped via lan9645x_pgid_entry_put() before
verifying if the hardware MAC table update succeeds.

If the hardware update fails, the software state isn't rolled back, leaving
them out of sync. Furthermore, the dropped old_pgid reference could allow the
index to be reallocated to a new multicast group while the hardware still
points to the old PGID index, potentially causing silent traffic cross-talk.

> +
> +static int __lan9645x_mdb_del(struct lan9645x *lan9645x, int chip_port,
> +			      const unsigned char addr[ETH_ALEN], u16 vid,
> +			      enum macaccess_entry_type type)
> +{
> +	struct lan9645x_pgid_entry *old_pgid, *new_pgid;
> +	struct lan9645x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +	int err, pgid_index;
> +	bool cpu_copy;
> +
> +	mdb_entry = lan9645x_mdb_entry_lookup(lan9645x, addr, vid);
> +	if (!mdb_entry)
> +		return -ENOENT;
> +
> +	if (!(mdb_entry->ports & BIT(chip_port)))
> +		return 0;
> +
> +	mdb_entry->ports &= ~BIT(chip_port);
> +
> +	/* Encode mac for IP mc */
> +	lan9645x_mdb_encode_mac(mac, mdb_entry, type);
> +
> +	/* Update PGID ptr for non-IP entries (L2 multicast) */
> +	old_pgid = mdb_entry->pgid;
> +	new_pgid = lan9645x_mdb_pgid_entry_get(lan9645x, mdb_entry, type);
> +	if (IS_ERR(new_pgid)) {
> +		/* Continue forwarding to old port group. */
> +		mdb_entry->ports |= BIT(chip_port);
> +		return PTR_ERR(new_pgid);
> +	}
> +	mdb_entry->pgid = new_pgid;
> +
> +	if (!mdb_entry->ports) {
> +		lan9645x_mact_forget(lan9645x, mac, mdb_entry->vid, type);
> +		lan9645x_pgid_entry_put(lan9645x, old_pgid);
> +		lan9645x_mdb_entry_dealloc(lan9645x, mdb_entry);
> +		return 0;
> +	}

Similar to the addition path above, if lan9645x_mact_forget() fails, its
return value is ignored, and the software MDB entry and PGID are freed
anyway. This could also lead to hardware/software state desynchronization.

> +
> +	cpu_copy = !!(mdb_entry->ports & BIT(CPU_PORT));
> +	pgid_index = lan9645x_mdb_pgid_index(mdb_entry, type);
> +
> +	err = lan9645x_mact_learn_cpu_copy(lan9645x, pgid_index, mac,
> +					   mdb_entry->vid, type, cpu_copy);
> +	lan9645x_pgid_entry_put(lan9645x, old_pgid);
> +	return err;
> +}

^ permalink raw reply

* Re: [PATCH net-next v2 7/9] net: dsa: lan9645x: add mac table integration
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-7-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add mac table integration

Add MAC table support, and dsa fdb callback integration. The mactable is
keyed on (vid,mac) and each bucket has 4 slots. A mac table entry typically
points to a PGID index, the first 9 of which represent a front port.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
> index 000000000000..6335714dca21
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
[ ... ]
> +int lan9645x_mact_entry_add(struct lan9645x *lan9645x, int pgid,
> +			    const unsigned char *mac, u16 vid)
> +{
> +	struct lan9645x_mact_entry *entry;
> +	int err;
> +
> +	mutex_lock(&lan9645x->mact_lock);
> +
> +	/* Users can not move (vid,mac) to a different port, without removing
> +	 * the original entry first. But we overwrite entry in HW, and update
> +	 * software pgid for good measure.
> +	 */
> +	entry = lan9645x_mact_entry_find(lan9645x, mac, vid);
> +	if (entry) {
> +		entry->common.pgid = pgid;
> +		goto mac_learn;
> +	}
> +
> +	entry = lan9645x_mact_entry_alloc(lan9645x, mac, vid, pgid,
> +					  ENTRYTYPE_LOCKED);
> +	if (!entry) {
> +		mutex_unlock(&lan9645x->mact_lock);
> +		return -ENOMEM;
> +	}
> +
> +	list_add_tail(&entry->list, &lan9645x->mac_entries);
> +
> +mac_learn:
> +	err = __lan9645x_mact_learn(lan9645x, pgid, mac, vid, ENTRYTYPE_LOCKED);
> +	if (err)
> +		lan9645x_mact_entry_dealloc(lan9645x, entry);

If a pre-existing entry is updated but the hardware learning command fails,
will this inadvertently delete the valid software entry and cause the hardware
and software tracking to become desynchronized?

> +
> +	mutex_unlock(&lan9645x->mact_lock);
> +	return err;
> +}
> +
> +int lan9645x_mact_entry_del(struct lan9645x *lan9645x, int pgid,
> +			    const unsigned char *mac, u16 vid)
> +{
> +	struct lan9645x_mact_entry *entry;
> +	int err;
> +
> +	mutex_lock(&lan9645x->mact_lock);
> +
> +	entry = lan9645x_mact_entry_find(lan9645x, mac, vid);
> +	if (!entry) {
> +		mutex_unlock(&lan9645x->mact_lock);
> +		return -ENOENT;
> +	}
> +
> +	WARN_ON(entry->common.pgid != pgid);
> +	lan9645x_mact_entry_dealloc(lan9645x, entry);
> +	err = __lan9645x_mact_forget(lan9645x, mac, vid, ENTRYTYPE_LOCKED);

Does this permanently remove the software entry even if the hardware
command to forget the entry fails?

> +
> +	mutex_unlock(&lan9645x->mact_lock);
> +	return err;
> +}
[ ... ]
> +void lan9645x_mac_deinit(struct lan9645x *lan9645x)
> +{
> +	mutex_destroy(&lan9645x->mact_lock);
> +}

Are the static MAC entries in the mac_entries list leaked here during driver
teardown, or are they freed elsewhere?

> +
> +int lan9645x_mact_dsa_dump(struct lan9645x *lan9645x, int port,
> +			   dsa_fdb_dump_cb_t *cb, void *data)
> +{
> +	struct lan9645x_mact_entry entry = {};
> +	u32 mach, macl, maca;
> +	int err = 0;
> +	u32 autoage;
[ ... ]
> +	while (1) {
> +		/* NOTE: we rely on mach, macl and type being set correctly in
> +		 * the registers from previous round, vis a vis the GET_NEXT
> +		 * semantics, so locking entire loop is important.
> +		 */
> +		lan_wr(ANA_MACACCESS_MAC_TABLE_CMD_SET(CMD_GET_NEXT) |
> +		       ANA_MACACCESS_ENTRYTYPE_SET(entry.common.type),
> +		       lan9645x, ANA_MACACCESS);
> +
> +		if (lan9645x_mac_wait_for_completion(lan9645x, &maca))
> +			break;

If the hardware completion times out, the loop breaks out and returns the
local err variable which is initialized to 0. Will this suppress the hardware
timeout error code and incorrectly report a successful dump?

> +
> +		if (ANA_MACACCESS_VALID_GET(maca) == 0)
> +			break;
> +
> +		mach = lan_rd(lan9645x, ANA_MACHDATA);
> +		macl = lan_rd(lan9645x, ANA_MACLDATA);
> +
> +		lan9645x_mact_parse(mach, macl, maca, &entry.common);
> +
> +		if (ANA_MACACCESS_DEST_IDX_GET(maca) == port &&
> +		    entry.common.type == ENTRYTYPE_NORMAL) {

Since static FDB entries are programmed with ENTRYTYPE_LOCKED, will this
condition exclude them from the FDB dump, preventing them from showing up
during bridge FDB queries?

> +			if (entry.common.key.vid > VLAN_MAX)
> +				entry.common.key.vid = 0;
> +
> +			err = cb(entry.common.key.mac, entry.common.key.vid,
> +				 false, data);
> +			if (err)
> +				break;
> +		}
> +	}
> +
> +	/* Remove aging filters and restore aging */
> +	lan_wr(0, lan9645x, ANA_ANAGEFIL);
> +	lan_rmw(ANA_AUTOAGE_AGE_PERIOD_SET(ANA_AUTOAGE_AGE_PERIOD_GET(autoage)),
> +		ANA_AUTOAGE_AGE_PERIOD,
> +		lan9645x, ANA_AUTOAGE);

Does restoring the ANA_AUTOAGE register using the autoage value saved at the
start of the function race with other operations setting the aging time?

If the aging time is updated concurrently without taking mact_lock, its newly
configured aging period might be overwritten here.

> +
> +	mutex_unlock(&lan9645x->mact_lock);
> +
> +	return err;
> +}
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 046c95a72242..32c0301030a4 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> +static int lan9645x_fdb_del(struct dsa_switch *ds, int port,
> +			    const unsigned char *addr, u16 vid,
> +			    struct dsa_db db)
> +{
> +	struct net_device *br = lan9645x_db2bridge(db);
> +	struct dsa_port *dp = dsa_to_port(ds, port);
> +	struct lan9645x *lan9645x = ds->priv;
> +	int err;
> +
> +	if (IS_ERR(br))
> +		return PTR_ERR(br);
> +
> +	if (dsa_port_is_cpu(dp) && !br &&
> +	    dsa_fdb_present_in_other_db(ds, port, addr, vid, db))
> +		return 0;
> +
> +	if (!vid)
> +		vid = lan9645x_vlan_unaware_pvid(!!br);
> +
> +	if (dsa_port_is_cpu(dp))
> +		return lan9645x_mact_forget(lan9645x, addr, vid,
> +					    ENTRYTYPE_LOCKED);
> +
> +	err = lan9645x_mact_entry_del(lan9645x, port, addr, vid);
> +	if (err == -ENOENT) {
> +		dev_dbg(lan9645x->dev,
> +			"fdb not found port=%d addr=%pM vid=%u\n", port, addr,
> +			vid);
> +		return 0;
> +	}
> +
> +	return err;
> +}

Since lan9645x_mact_entry_del returns -ENOENT when a MAC address is not found
in the software tracking list, and the software list only contains statically
added entries, won't this intercept attempts to manually delete dynamically
learned MAC addresses and return success without actually removing them from
the hardware table?

^ permalink raw reply

* Re: [PATCH net-next v2 6/9] net: dsa: lan9645x: add vlan support
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-6-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add vlan support

Add support for vlanaware bridge. We reserve vid 4095 for standalone
mode, to implement fdb-isolation. A vlan-unaware bridge uses vid 0.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 599e589c4ec3..046c95a72242 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -535,11 +536,54 @@ static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
[ ... ]
> +static int lan9645x_port_vlan_filtering(struct dsa_switch *ds, int port,
> +					bool enabled,
> +					struct netlink_ext_ack *extack)
> +{
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct lan9645x_port *p;
> +
> +	p = lan9645x_to_port(lan9645x, port);
> +	p->vlan_aware = enabled;
> +	lan9645x_vlan_port_apply(p);

In other places, the driver avoids calling egress functions on the npi
port because it conflicts with cpu port header extraction. Since this
unconditionally applies the vlan configuration to both ingress and egress,
could this overwrite the npi port's configuration when the state is
synchronized?

> +
> +	return 0;
> +}
> +
> +static int lan9645x_port_vlan_add(struct dsa_switch *ds, int port,
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> new file mode 100644
> index 000000000000..0459932a626c
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
[ ... ]
> +static void lan9645x_vlan_hw_wr(struct lan9645x *lan9645x, u16 vid)
> +{
> +	struct lan9645x_vlan *v = &lan9645x->vlans[vid];
> +	bool cpu_dis = !(v->portmask & BIT(CPU_PORT));
> +	u32 val;
> +
> +	val = ANA_VLANTIDX_VLAN_PGID_CPU_DIS_SET(cpu_dis) |
> +	      ANA_VLANTIDX_V_INDEX_SET(vid) |
> +	      ANA_VLANTIDX_VLAN_SEC_FWD_ENA_SET(v->s_fwd_ena) |
> +	      ANA_VLANTIDX_VLAN_FLOOD_DIS_SET(v->fld_dis) |
> +	      ANA_VLANTIDX_VLAN_PRIV_VLAN_SET(v->prv_vlan) |
> +	      ANA_VLANTIDX_VLAN_LEARN_DISABLED_SET(v->lrn_dis) |
> +	      ANA_VLANTIDX_VLAN_MIRROR_SET(v->mir) |
> +	      ANA_VLANTIDX_VLAN_SRC_CHK_SET(v->src_chk);
> +
> +	lan_wr(val, lan9645x, ANA_VLANTIDX);
> +	lan_wr(v->portmask, lan9645x, ANA_VLAN_PORT_MASK);
> +	lan_wr(VLANACCESS_CMD_WRITE, lan9645x, ANA_VLANACCESS);

Are these registers written with raw values rather than using their respective
set macros? This circumvents the register abstraction and might risk failures
if the hardware layout changes.

> +
> +	if (lan9645x_vlan_wait_for_completion(lan9645x))
> +		dev_err(lan9645x->dev, "Vlan set mask failed\n");
> +}
[ ... ]
> +static void
> +lan9645x_vlan_port_apply_egress(struct lan9645x_port *p,
> +				struct lan9645x_vlan_port_info *info)
> +{
[ ... ]
> +	lan_rmw(REW_TAG_CFG_TAG_TPID_CFG_SET(3) |
> +		REW_TAG_CFG_TAG_CFG_SET(tag_cfg),
> +		REW_TAG_CFG_TAG_TPID_CFG |
> +		REW_TAG_CFG_TAG_CFG,
> +		lan9645x, REW_TAG_CFG(p->chip_port));
> +
> +	lan_rmw(REW_PORT_VLAN_CFG_PORT_TPID_SET(ETH_P_8021AD) |
> +		REW_PORT_VLAN_CFG_PORT_VID_SET(port_vid),
> +		REW_PORT_VLAN_CFG_PORT_TPID |
> +		REW_PORT_VLAN_CFG_PORT_VID,
> +		lan9645x, REW_PORT_VLAN_CFG(p->chip_port));
> +}

Setting the port tpid to 0x88a8 causes untagged ingress frames to egress trunk
ports with 802.1ad tags. Could this break interoperability with standard
networks that expect 0x8100 tags?

> +static void lan9645x_vlan_port_apply_ingress(struct lan9645x_port *p)
> +{
[ ... ]
> +	/* Drop frames with multicast source address */
> +	val = ANA_DROP_CFG_DROP_MC_SMAC_ENA_SET(1);
> +	if (p->vlan_aware && !pvid)
> +		/* If port is vlan-aware and tagged, drop untagged and priority
> +		 * tagged frames.
> +		 */
> +		val |= ANA_DROP_CFG_DROP_UNTAGGED_ENA_SET(1) |
> +		       ANA_DROP_CFG_DROP_PRIO_S_TAGGED_ENA_SET(1) |
> +		       ANA_DROP_CFG_DROP_PRIO_C_TAGGED_ENA_SET(1);
> +
> +	lan_wr(val, lan9645x, ANA_DROP_CFG(p->chip_port));
> +}

Does this overwrite the entire 32-bit register and silently clear other
initialized bitfields? Would it be safer to use a read-modify-write here
instead?

[ ... ]
> +static int lan9645x_vlan_cpu_add(struct lan9645x_port *p, u16 vid, bool pvid,
> +				 bool untagged)
> +{
> +	struct lan9645x_vlan *v;
> +
> +	v = lan9645x_vlan_port_modify(p, vid, pvid, untagged);

Is it intentional that this bypasses the bounds check on the vlan id?

> +	v->portmask |= BIT(CPU_PORT) | BIT(p->chip_port);
> +	lan9645x_vlan_hw_wr(p->lan9645x, vid);
> +	lan9645x_vlan_port_apply_ingress(p);
> +
> +	return 0;
> +}
> +
> +int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
> +				bool untagged, struct netlink_ext_ack *extack)
> +{
[ ... ]
> +void lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid)
> +{
> +	struct lan9645x *lan9645x = p->lan9645x;
> +	struct lan9645x_vlan *v;
> +
> +	if (!vid)
> +		return;
> +
> +	v = lan9645x_vlan_port_modify(p, vid, false, false);

There is a check for the maximum vlan id when adding a vlan, but no bounds
check when deleting one. Could userspace use this to delete the reserved
vid 4095 and break standalone forwarding?

> +	v->portmask &= ~BIT(p->chip_port);
> +
> +	if (p->chip_port == lan9645x->npi)
> +		v->portmask &= ~BIT(CPU_PORT);
> +
> +	lan9645x_vlan_hw_wr(lan9645x, vid);
[ ... ]
> +void lan9645x_vlan_init(struct lan9645x *lan9645x)
> +{
[ ... ]
> +	for (vid = 1; vid < VLAN_N_VID; vid++)
> +		lan9645x_vlan_hw_wr(lan9645x, vid);

If the indirect table access times out, the write function prints an error
but does not return a status code. Will this cause the initialization loop
to stall for the full timeout duration 4095 consecutive times if the
hardware is unresponsive?

> +
> +	/* Set all the ports + cpu to be part of HOST_PVID and UNAWARE_PVID */
> +	lan9645x->vlans[HOST_PVID].portmask = all_ports;

^ permalink raw reply

* Re: [PATCH net-next v2 5/9] net: dsa: lan9645x: add bridge support
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-5-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add bridge support

This commit adds support for hardware offloading of the bridge in the
lan9645x DSA driver. It enables a single bridge device to be used with
the switch.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index a0908cbf89c7..599e589c4ec3 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -145,6 +155,8 @@ static int lan9645x_setup(struct dsa_switch *ds)
>  		return err;
>  	}
>  
> +	mutex_init(&lan9645x->fwd_domain_lock);
> +
>  	/* Link Aggregation Mode: NETDEV_LAG_HASH_L2 */
>  	lan_wr(ANA_AGGR_CFG_AC_SMAC_ENA |
>  	       ANA_AGGR_CFG_AC_DMAC_ENA,
[ ... ]
> @@ -240,6 +252,11 @@ static int lan9645x_setup(struct dsa_switch *ds)
>  
>  	lan9645x_port_set_tail_drop_wm(lan9645x);
>  
> +	lan9645x->owq = alloc_ordered_workqueue("%s-owq", 0,
> +						dev_name(lan9645x->dev));
> +	if (!lan9645x->owq)
> +		return -ENOMEM;
> +

Is it possible that the hardware remains initialized if the call to
alloc_ordered_workqueue fails? Because the setup callback returns an error,
the DSA core will skip the teardown callback. This would bypass
lan9645x_npi_port_deinit, potentially leaving the conduit netdev in a
corrupted state with hardware tagging enabled.

Also in lan9645x_setup, the initialization loop for PGID_ENTRIES iterates
through index 88:

for (int i = 0; i < PGID_ENTRIES; ++i)
        lan_wr(ANA_PGID_CFG_OBEY_VLAN_SET(1),
               lan9645x, ANA_PGID_CFG(i));

Could this leave the CPU port's source configuration in an undefined state?
The CPU port is port 9, which has its source analysis register at PGID_SRC
(80) + 9 = 89, but the loop terminates before reaching it.

> @@ -258,6 +275,271 @@ static void lan9645x_port_phylink_get_caps(struct dsa_switch *ds, int port,
>  	lan9645x_phylink_get_caps(ds->priv, port, config);
>  }
>  
> +static int lan9645x_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> +{
> +	u32 age_secs = max(1, msecs / MSEC_PER_SEC / 2);
> +	struct lan9645x *lan9645x = ds->priv;
> +
> +	/* Entry is must suffer two aging scans before it is removed, so it is
> +	 * aged after 2*AGE_PERIOD, and the unit is in seconds.
> +	 * An age period of 0 disables automatic aging.
> +	 */
> +	lan_rmw(ANA_AUTOAGE_AGE_PERIOD_SET(age_secs),
> +		ANA_AUTOAGE_AGE_PERIOD,
> +		lan9645x, ANA_AUTOAGE);
> +	return 0;
> +}

Does this function properly disable automatic aging when requested? When the
Linux bridge passes msecs = 0 to disable aging, the max macro evaluates to 1.
Since writing 1 configures an aggressive 2-second aging period, it appears
this might not fulfill the user's intent.

[ ... ]
> +static void __lan9645x_port_set_host_flood(struct lan9645x *lan9645x, int port,
> +					   bool uc, bool mc)
> +{
> +	bool mc_ena, uc_ena;
> +
> +	lockdep_assert_held(&lan9645x->fwd_domain_lock);
> +
> +	/* We want promiscuous and all_multi to affect standalone ports, for
> +	 * debug and test purposes.
> +	 *
> +	 * However, the linux bridge is incredibly eager to put bridged ports in
> +	 * promiscuous mode.
> +
> +	 * This is unfortunate since lan9645x flood masks are global and not per
> +	 * ingress port. When some port triggers unknown uc/mc to the CPU, the
> +	 * traffic from any port is forwarded to the CPU.
> +	 *
> +	 * If the host CPU is weak, this can cause tremendous stress. Therefore,
> +	 * we compromise by ignoring this host flood request for bridged ports.
> +	 */
> +	if (lan9645x_port_is_bridged(lan9645x_to_port(lan9645x, port)))
> +		return;

Will ignoring the host flood request for bridged ports break IPv6 Neighbor
Discovery? IPv6 ND relies on multicast, and the PGID_MC and PGID_MCIPV6 masks
do not include the CPU port by default. By ignoring the bridge's request to
enable multicast flooding, the CPU port may fail to receive ICMPv6 ND packets.

[ ... ]
> +/* Called in atomic context */
> +static void lan9645x_port_set_host_flood(struct dsa_switch *ds, int port,
> +					 bool uc, bool mc)
> +{
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct lan9645x_host_flood_work *w;
> +
> +	w = kzalloc_obj(*w, GFP_ATOMIC);

Is kzalloc_obj an available kernel function? Using it might cause a build
error, as the standard API would be kzalloc(sizeof(*w), GFP_ATOMIC).

> +	if (!w)
> +		return;

Are there consequences to returning silently when the allocation fails?
If this allocation fails under memory pressure, the network stack will still
assume the configuration was successfully applied, leaving the hardware state
out of sync.

[ ... ]
> +static int lan9645x_port_bridge_join(struct dsa_switch *ds, int port,
> +				     struct dsa_bridge bridge,
> +				     bool *tx_fwd_offload,
> +				     struct netlink_ext_ack *extack)
> +{
[ ... ]
> +	/* The bridge puts ports in IFF_ALLMULTI before calling
> +	 * port_bridge_join, so clean up before the port is marked as bridged.
> +	 */
> +	__lan9645x_port_set_host_flood(lan9645x, port, false, false);
> +	lan9645x->bridge_mask |= BIT(p->chip_port);
> +
> +	mutex_unlock(&lan9645x->fwd_domain_lock);
> +
> +	/* Later: stp_state_set updates forwarding */
> +
> +	return 0;
> +}

Could clearing the host flood masks here lead to lost hardware state when the
port leaves the bridge? If a user previously enabled IFF_PROMISC or
IFF_ALLMULTI on the standalone port, the software netdev flags remain set.
When lan9645x_port_bridge_leave is called, it does not restore the hardware's
promiscuous or multicast state to match the unchanged software flags.

> +static void lan9645x_port_bridge_stp_state_set(struct dsa_switch *ds, int port,
> +					       u8 state)
> +{
> +	struct lan9645x *lan9645x;
> +	struct lan9645x_port *p;
> +	bool learn_ena;
> +
> +	lan9645x = ds->priv;
> +	p = lan9645x_to_port(lan9645x, port);
> +
> +	mutex_lock(&lan9645x->fwd_domain_lock);
> +
> +	p->stp_state = state;
> +
> +	if (state == BR_STATE_FORWARDING)
> +		lan9645x->bridge_fwd_mask |= BIT(p->chip_port);
> +	else
> +		lan9645x->bridge_fwd_mask &= ~BIT(p->chip_port);
> +
> +	learn_ena = (state == BR_STATE_LEARNING ||
> +		     state == BR_STATE_FORWARDING) && p->learn_ena;
> +
> +	lan_rmw(ANA_PORT_CFG_LEARN_ENA_SET(learn_ena),
> +		ANA_PORT_CFG_LEARN_ENA, lan9645x,
> +		ANA_PORT_CFG(p->chip_port));
> +
> +	lan9645x_update_fwd_mask(lan9645x);
> +	mutex_unlock(&lan9645x->fwd_domain_lock);
> +}

Are ingress data frames correctly dropped for ports in non-forwarding STP
states? When the port is removed from bridge_fwd_mask,
lan9645x_update_fwd_mask calculates the source PGID mask as
bridge_mask & bridge_fwd_mask & ~BIT(dp->index).

This evaluates to the set of all other forwarding ports in the bridge. Since
ingress forwarding is not explicitly disabled on the port (for example, by
setting ANA_PORT_CFG_PORTID_VAL to NONE), data frames received on a blocking
or learning port can enter the analyzer and be forwarded.

> +static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
> +				       struct dsa_bridge bridge)
> +{
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct lan9645x_port *p;
> +
> +	p = lan9645x_to_port(lan9645x, port);
> +
> +	mutex_lock(&lan9645x->fwd_domain_lock);
> +
> +	lan9645x->bridge_mask &= ~BIT(p->chip_port);
> +
> +	/* Last port leaving clears bridge dev */
> +	if (!lan9645x->bridge_mask)
> +		lan9645x->bridge = NULL;
> +
> +	lan9645x_update_fwd_mask(lan9645x);
> +
> +	mutex_unlock(&lan9645x->fwd_domain_lock);
> +}

Should p->learn_ena be reset when a port leaves the bridge?

During port_bridge_flags, p->learn_ena is set to true. When leaving the
bridge, this flag remains true. The DSA core will transition the leaving
port to BR_STATE_FORWARDING, which calls lan9645x_port_bridge_stp_state_set
and leaves hardware learning enabled. This can pollute the shared FDB with
MAC addresses from standalone ports, leading to silent packet drops if
bridged ports attempt to forward traffic to them.

^ permalink raw reply

* Re: [PATCH net-next v2 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X
From: Jakub Kicinski @ 2026-03-29 19:56 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
	pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
	Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260324-dsa_lan9645x_switch_driver_base-v2-4-f7504e3b0681@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: dsa: lan9645x: add basic dsa driver for LAN9645X

This commit adds a basic DSA driver for the Microchip LAN9645X switch. It
includes initialization, port module setup for NPI, CPU, and front ports,
and phylink integration for MAC side configuration.

diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_npi.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_npi.c
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_npi.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_npi.c
[ ... ]
> +static void disable_conduit_ipv6(struct lan9645x *lan9645x,
> +				 struct net_device *conduit)
> +{
> +	struct inet6_dev *dev_v6;
> +
> +	if (!conduit)
> +		return;
> +
> +	/* IPv6 on the conduit will generate frames bypassing our tag driver, so
> +	 * they lack an IFH. This will be garbage in garbage out and we want to
> +	 * avoid this.
> +	 */
> +	rtnl_lock();
> +	dev_v6 = __in6_dev_get(conduit);
> +	if (dev_v6) {
> +		WRITE_ONCE(dev_v6->cnf.disable_ipv6, 1);
> +		dev_warn(lan9645x->dev, "Disabled IPv6 on conduit device: %s\n",
> +			 netdev_name(conduit));
> +	}
> +	rtnl_lock();
> +}

Does directly modifying dev_v6->cnf.disable_ipv6 bypass the proper sysctl
teardown path (addrconf_disable_ipv6) and leave the IPv6 stack in an
inconsistent state on the conduit device by failing to clean up routes,
addresses, and MLD memberships?

Is it a layering violation for a switch driver to alter the networking
configuration of an external upper device, especially since this change
isn't reverted during driver unbind?

[ ... ]
> +void lan9645x_npi_port_init(struct lan9645x *lan9645x,
> +			    struct dsa_port *cpu_port)
> +{
> +	int port = cpu_port->index;
> +	struct lan9645x_port *p;
> +
> +	p = lan9645x_to_port(lan9645x, port);
> +	lan9645x->npi = port;

In a cascaded multi-switch DSA topology, cpu_port->index may belong to a
different switch and be greater than 8. If lan9645x_to_port returns NULL,
will this cause a panic when dereferencing p->chip_port below?

[ ... ]
> +	/* Configure IFH prefix mode for NPI port. */
> +	lan_rmw(SYS_PORT_MODE_INCL_XTR_HDR_SET(LAN9645X_TAG_PREFIX_LONG) |
> +		SYS_PORT_MODE_INCL_INJ_HDR_SET(LAN9645X_TAG_PREFIX_NONE),
> +		SYS_PORT_MODE_INCL_XTR_HDR |
> +		SYS_PORT_MODE_INCL_INJ_HDR,
> +		lan9645x,
> +		SYS_PORT_MODE(p->chip_port));

Setting LAN9645X_TAG_PREFIX_NONE means the hardware interprets the first
bytes of any frame received from the host as an IFH command structure.

Does this allow a local user to transmit crafted frames (for example, via
raw sockets or standard protocols) whose MAC address bytes map to malicious
IFH commands, thereby bypassing VLAN isolation or flooding the switch?

Should the hardware be configured to require a strict prefix, such as
LAN9645X_TAG_PREFIX_LONG, for injected frames to prevent this?

diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> +static int lan9645x_setup(struct dsa_switch *ds)
> +{
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct device *dev = lan9645x->dev;
> +	u32 all_phys_ports;
> +	int err;
> +
> +	lan9645x->num_phys_ports = ds->num_ports;
> +	all_phys_ports = GENMASK(lan9645x->num_phys_ports - 1, 0);
> +
> +	err = lan9645x_reset_switch(lan9645x);
> +	if (err)
> +		return err;
> +
> +	lan9645x->ports = devm_kcalloc(lan9645x->dev, lan9645x->num_phys_ports,
> +				       sizeof(struct lan9645x_port *),
> +				       GFP_KERNEL);

Since the DSA .setup callback can be invoked multiple times during the
platform device's lifetime (for example, when the DSA master interface is
unbound and bound again), does using devm_kcalloc here cause a memory leak?
Should these allocations be moved to .probe or managed with kcalloc and
freed in .teardown?

[ ... ]
> +	/* Set all the entries to obey VLAN. */
> +	for (int i = 0; i < PGID_ENTRIES; ++i)
> +		lan_wr(ANA_PGID_CFG_OBEY_VLAN_SET(1),
> +		       lan9645x, ANA_PGID_CFG(i));

PGID_ENTRIES is defined as 89, so this loop initializes indices 0 through
88. Since the CPU port is index 9, its source PGID is PGID_SRC + CPU_PORT
(80 + 9 = 89).

Is index 89 left uninitialized, breaking the OBEY_VLAN rule and allowing
CPU-injected frames to leak across VLAN boundaries?

[ ... ]
> +	/* Multicast to all front ports */
> +	lan_wr(all_phys_ports, lan9645x, ANA_PGID(PGID_MC));
> +
> +	/* IP multicast to all front ports */
> +	lan_wr(all_phys_ports, lan9645x, ANA_PGID(PGID_MCIPV4));
> +	lan_wr(all_phys_ports, lan9645x, ANA_PGID(PGID_MCIPV6));
> +
> +	/* Unicast to all front ports */
> +	lan_wr(all_phys_ports, lan9645x, ANA_PGID(PGID_UC));
> +
> +	/* Broadcast to all ports */
> +	lan_wr(BIT(CPU_PORT) | all_phys_ports, lan9645x, ANA_PGID(PGID_BC));

PGID_BC includes BIT(CPU_PORT) and all_phys_ports (which includes the NPI
port). Will this forward broadcast frames to both the CPU extraction queue
and the NPI port's normal egress queue, causing duplicate frames for the host?

Conversely, the multicast masks and PGID_UC exclude BIT(CPU_PORT). Does
this cause them to bypass the CPU extraction queue entirely, thereby
lacking the LONG extraction prefix and breaking the host's DSA tagger parsing?

diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c
[ ... ]
> +static int lan9645x_phylink_mac_prepare(struct phylink_config *config,
> +					unsigned int mode,
> +					phy_interface_t iface)
> +{
> +	struct lan9645x_port *p = lan9645x_phylink_config_to_port(config);
> +	struct lan9645x *lan9645x = p->lan9645x;
> +	int port = p->chip_port;
> +	u32 mask;
> +
> +	if (port == 5 || port == 6 || port > 8)
> +		return -EINVAL;
> +
> +	mask = HSIO_HW_CFG_GMII_ENA_SET(BIT(port));
> +	lan_rmw(mask, mask, lan9645x, HSIO_HW_CFG);
> +
> +	if (port == 4 && phy_interface_mode_is_rgmii(iface))
> +		lan_rmw(HSIO_HW_CFG_RGMII_0_CFG_SET(1),
> +			HSIO_HW_CFG_RGMII_0_CFG,
> +			lan9645x, HSIO_HW_CFG);

The lan9645x_phylink_get_caps function advertises RGMII support for ports
4, 7, and 8. Does restricting the hardware multiplexer configuration here to
port 4 break connectivity for ports 7 and 8 when used in RGMII mode?

diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
[ ... ]
> +int lan9645x_port_set_maxlen(struct lan9645x *lan9645x, int port, size_t sdu)
> +{
> +	struct lan9645x_port *p = lan9645x_to_port(lan9645x, port);
> +	int maxlen = sdu + ETH_HLEN + ETH_FCS_LEN;

Does this maximum frame length calculation drop standard 1500-byte MTU
frames that are 802.1Q VLAN tagged (1522 bytes total), since it does not
account for VLAN_HLEN?

[ ... ]
> +int lan9645x_port_setup(struct dsa_switch *ds, int port)
> +{
> +	struct dsa_port *dp = dsa_to_port(ds, port);
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct lan9645x_port *p;
> +
> +	p = lan9645x_to_port(lan9645x, port);
> +
> +	if (dp->dn) {
> +		p->rx_internal_delay =
> +			of_property_present(dp->dn, "rx-internal-delay-ps");
> +		p->tx_internal_delay =
> +			of_property_present(dp->dn, "tx-internal-delay-ps");
> +	}

These are standard integer properties specifying delays in picoseconds. If
a user explicitly disables the delay via devicetree using a value of 0,
will of_property_present evaluate to true and enable the hardware delay
anyway? Should of_property_read_u32 be used instead to check the value?

^ permalink raw reply


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