Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v9 1/5] PCI: Add pcie_get_link_speed() helper for safe array access
From: Bjorn Helgaas @ 2026-03-26 18:09 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, jingoohan1, mani, kwilczynski, bhelgaas,
	florian.fainelli, jim2101024, robh, ilpo.jarvinen, linux-arm-msm,
	linux-arm-kernel, linux-renesas-soc, claudiu.beznea.uj,
	linux-mediatek, linux-tegra, linux-omap, bcm-kernel-feedback-list,
	linux-pci, linux-kernel, shawn.lin
In-Reply-To: <20260313165522.123518-2-18255117159@163.com>

On Sat, Mar 14, 2026 at 12:55:18AM +0800, Hans Zhang wrote:
> The pcie_link_speed[] array is indexed by PCIe generation numbers
> (1 = 2.5 GT/s, 2 = 5 GT/s, ...).  Several drivers use it directly,
> which can lead to out-of-bounds accesses if an invalid generation
> number is used.
> 
> Introduce a helper function pcie_get_link_speed() that returns the
> corresponding enum pci_bus_speed value for a given generation number,
> or PCI_SPEED_UNKNOWN if the generation is out of range.  This will
> allow us to safely handle invalid values after the range check is
> removed from of_pci_get_max_link_speed().
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pci.h   |  2 ++
>  drivers/pci/probe.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 13d998fbacce..409aca7d737a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -108,6 +108,8 @@ struct pcie_tlp_log;
>  				 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
>  
>  extern const unsigned char pcie_link_speed[];
> +unsigned char pcie_get_link_speed(unsigned int speed);
> +
>  extern bool pci_early_dump;
>  
>  extern struct mutex pci_rescan_remove_lock;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..d6592898330c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -783,6 +783,22 @@ const unsigned char pcie_link_speed[] = {
>  };
>  EXPORT_SYMBOL_GPL(pcie_link_speed);
>  
> +/**
> + * pcie_link_speed_value - Get speed value from PCIe generation number
> + * @speed: PCIe speed (1-based: 1 = 2.5GT, 2 = 5GT, ...)
> + *
> + * Returns the speed value (e.g., PCIE_SPEED_2_5GT) if @speed is valid,
> + * otherwise returns PCI_SPEED_UNKNOWN.
> + */
> +unsigned char pcie_get_link_speed(unsigned int speed)
> +{
> +	if (speed >= ARRAY_SIZE(pcie_link_speed))
> +		return PCI_SPEED_UNKNOWN;
> +
> +	return pcie_link_speed[speed];
> +}
> +EXPORT_SYMBOL_GPL(pcie_get_link_speed);
> +
>  const char *pci_speed_string(enum pci_bus_speed speed)
>  {
>  	/* Indexed by the pci_bus_speed enum */
> -- 
> 2.34.1
> 


^ permalink raw reply

* Re: [PATCH net-next 03/15] net: stmmac: qcom-ethqos: eliminate configure_func
From: Simon Horman @ 2026-03-26 18:04 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-arm-msm,
	linux-stm32, Mohd Ayaan Anwar, netdev, Paolo Abeni
In-Reply-To: <E1w51Xs-0000000DwVV-2bnh@rmk-PC.armlinux.org.uk>

On Tue, Mar 24, 2026 at 01:11:44PM +0000, Russell King (Oracle) wrote:
> Since ethqos_fix_mac_speed() is called via a function pointer, and only
> indirects via the configure_func function pointer, eliminate this
> unnecessary indirection.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

...

> @@ -623,14 +627,6 @@ static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos,
>  	ethqos_pcs_set_inband(ethqos, interface == PHY_INTERFACE_MODE_SGMII);
>  }
>  
> -static void ethqos_fix_mac_speed(void *priv, phy_interface_t interface,
> -				 int speed, unsigned int mode)
> -{
> -	struct qcom_ethqos *ethqos = priv;
> -
> -	ethqos->configure_func(ethqos, interface, speed);
> -}
> -
>  static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
>  {
>  	struct qcom_ethqos *ethqos = priv;

Hi Russell,

FYI, AI generated review reports that the comment in ethqos_clks_config()
that references ethqos_fix_mac_speed() should also be updated.

...


^ permalink raw reply

* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Ville Syrjälä @ 2026-03-26 17:58 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Nicolas Frattaroli, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Thomas Zimmermann, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Sandy Huang, Heiko Stübner, Andy Yan,
	Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	Dmitry Baryshkov, Sascha Hauer, Rob Herring, Jonathan Corbet,
	Shuah Khan, kernel, amd-gfx, dri-devel, linux-kernel,
	linux-arm-kernel, linux-rockchip, intel-gfx, intel-xe, linux-doc,
	Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <20260326-pumpkin-goshawk-of-stamina-0ccb84@houat>

On Thu, Mar 26, 2026 at 06:02:47PM +0100, Maxime Ripard wrote:
> On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > +enum drm_connector_color_format {
> > > > > > > > > +	/**
> > > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > > +	 * helpers should pick a suitable color format. All implementations of a
> > > > > > > > > +	 * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > > +	 * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > > +	 * semantics.
> > > > > > > > > +	 *
> > > > > > > > > +	 * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > > +	 * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > > +	 * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > > +	 * YCbCr 4:2:0.
> > > > > > > > > +	 *
> > > > > > > > > +	 * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > > +	 * format selection picks the first chain of bridge formats that works,
> > > > > > > > > +	 * as has already been the case before the introduction of the "color
> > > > > > > > > +	 * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > > +	 * bus output formats by preference, or agree on a unified auto format
> > > > > > > > > +	 * selection logic that's implemented in a common state helper (like
> > > > > > > > > +	 * how HDMI does it).
> > > > > > > > > +	 */
> > > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > > +
> > > > > > > > > +	/**
> > > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > > +	 */
> > > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > > +
> > > > > > > > > +	/**
> > > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > > +	 * not subsampled)
> > > > > > > > > +	 */
> > > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > > +
> > > > > > > > > +	/**
> > > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > > +	 * with horizontal subsampling)
> > > > > > > > > +	 */
> > > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > > +
> > > > > > > > > +	/**
> > > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > > +	 * with horizontal and vertical subsampling)
> > > > > > > > > +	 */
> > > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > > 
> > > > > > > > Seems like this should document what the quantization range
> > > > > > > > should be for each format.
> > > > > > > > 
> > > > > > > 
> > > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > > enum is more abstract than that, and is there to communicate
> > > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > > by other properties.
> > > > > > > 
> > > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > > be added; no digital display protocol standard supports it to my
> > > > > > > knowledge, and hopefully none ever will.
> > > > > > 
> > > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > > 
> > > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > > RGB can either be full or limited range depending on the 
> > > > > > "Broadcast RGB" property and other related factors.
> > > > > 
> > > > > So far the HDMI state has both the format and quantization range as
> > > > > different fields. I'm not sure we need to document the range in the
> > > > > format field, maybe only mention it's not part of the format but has a
> > > > > field of its own?
> > > > 
> > > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > > I think the assumption is limited range everywhere.
> > > > 
> > > > But I'm not really concerned about documenting struct members.
> > > > What I'm talking about is the *uapi* docs. Surely userspace
> > > > will want to know what the new property actually does so the
> > > > uapi needs to be documented properly. And down the line some
> > > > new driver might also implement the wrong behaviour if there
> > > > is no clear specification.
> > > 
> > > Ack
> > > 
> > > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > > - YCbCr limited range 
> > > > - RGB full range if "Broadcast RGB" property is not present
> > > 
> > > Isn't it much more complicated than that for HDMI though? My
> > > recollection was that any VIC but VIC1 would be limited range, and
> > > anything else full range?
> > 
> > Do we have some driver that implements the CTA-861 CE vs. IT mode
> > logic but doesn't expose the "Broadcast RGB" property? I was hoping
> > those would always go hand in hand now.
> 
> I'm not sure. i915 and the HDMI state helpers handle it properly (I
> think?) but it looks like only vc4 registers the Broadcast RGB property
> and uses the HDMI state helpers.
> 
> And it looks like amdgpu registers Broadcast RGB but doesn't use
> drm_default_rgb_quant_range() which seems suspicious?

If they want just manual full vs. limited then they should
limit the property to not expose the "auto" option at all.

amdgpu also ties this in with the "colorspace" property, which
originally in i915 only controlled the infoframes/etc. But on
amdgpu it now controls various aspects of output color
transformation. The end result is that the property is a complete
mess with most of the values making no sense. And for whatever
reason everyone involved refused to remove/deprecate the
nonsensical values :/

Looks like this series should make sure the documentation for
the "colorspace" property is in sync with the new property
as well. Currently now it's giving conflicting information.

-- 
Ville Syrjälä
Intel


^ permalink raw reply

* Re: [PATCH net-next 1/2] net: stmmac: remove axi_kbbe, axi_mb and axi_rb members
From: Russell King (Oracle) @ 2026-03-26 17:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, Conor Dooley,
	David S. Miller, devicetree, Eric Dumazet, Giuseppe Cavallaro,
	Jakub Kicinski, Jose Abreu, Krzysztof Kozlowski, linux-arm-kernel,
	linux-stm32, netdev, Paolo Abeni, Rob Herring, Yao Zi
In-Reply-To: <20260326172943.GR111839@horms.kernel.org>

On Thu, Mar 26, 2026 at 05:29:43PM +0000, Simon Horman wrote:
> On Tue, Mar 24, 2026 at 10:05:40AM +0000, Russell King (Oracle) wrote:
> > axi_kbbe, axi_mb and axi_rb are all written, but nothing ever reads
> > their values. Remove the code that sets these and the struct members.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> 
> Hi Russell,
> 
> FYI, AI review suggests that these fields should also be removed from
> Documentation/networking/device_drivers/ethernet/stmicro/stmmac.rst

I noticed. I've prepared an update if netdev folk want that to happen
as I've noticed that that documentation is fairly out of date now.

Do we think it's still useful, or should we consider deleting or
trimming it down? Would it be better to move the struct definitions
into the header file and making the header file part of the docs so
that the documentation is local to the structs?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply

* Re: [PATCH net-next v2 4/4] net: phy: Introduce Airoha AN8801/R Gigabit Ethernet PHY driver
From: Russell King (Oracle) @ 2026-03-26 17:44 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Andrew Lunn, Andrew Lunn, Louis-Alexis Eyraud, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, AngeloGioacchino Del Regno,
	Heiner Kallweit, kevin-kw.huang, macpaul.lin, matthias.bgg,
	kernel, netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel
In-Reply-To: <385d2137-0d15-4dc0-8994-9cd48e4150b4@bootlin.com>

On Thu, Mar 26, 2026 at 06:25:20PM +0100, Maxime Chevallier wrote:
> On 26/03/2026 17:56, Andrew Lunn wrote:
> >> What is the timing requirements for a system going into suspend vs a WoL
> >> packet being sent? Should a WoL packet abort entry into suspend? If yes,
> >> then we need to program the MAC before the PHY is suspended, because
> >> suspend could already be in progress.
> > 
> > We would then need to hook into the NETDEV_CHANGEADDR notifier, and
> > call into the PHY driver to let it reprogram the MAC address.
> 
> We would also probably have to re-do that MAC addr programming at phy
> attach time ? If the PHY isn't attached (e.g. link admin-down on some
> boards), we can't use that notifier as we don't know what netdev to
> listen to. Or I'm missing something ?

Another point to consider in this area:

Should a detached PHY remain with WoL enabled?

If e.g. the network driver is unbound from its device, which certainly
disconnects the PHY from the network interface, should that PHY still
be able to wake up the system when there is no way to configure,
check its status, or handle its interrupts?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply

* Re: [PATCH v9 1/5] PCI: Add pcie_get_link_speed() helper for safe array access
From: Manivannan Sadhasivam @ 2026-03-26 17:40 UTC (permalink / raw)
  To: Hans Zhang, bhelgaas, helgaas
  Cc: lpieralisi, jingoohan1, kwilczynski, florian.fainelli, jim2101024,
	robh, ilpo.jarvinen, linux-arm-msm, linux-arm-kernel,
	linux-renesas-soc, claudiu.beznea.uj, linux-mediatek, linux-tegra,
	linux-omap, bcm-kernel-feedback-list, linux-pci, linux-kernel,
	shawn.lin
In-Reply-To: <20260313165522.123518-2-18255117159@163.com>

On Sat, Mar 14, 2026 at 12:55:18AM +0800, Hans Zhang wrote:
> The pcie_link_speed[] array is indexed by PCIe generation numbers
> (1 = 2.5 GT/s, 2 = 5 GT/s, ...).  Several drivers use it directly,
> which can lead to out-of-bounds accesses if an invalid generation
> number is used.
> 
> Introduce a helper function pcie_get_link_speed() that returns the
> corresponding enum pci_bus_speed value for a given generation number,
> or PCI_SPEED_UNKNOWN if the generation is out of range.  This will
> allow us to safely handle invalid values after the range check is
> removed from of_pci_get_max_link_speed().
> 

Bjorn: Could you please take a look at this patch and ack if looks good? Rest of
the patches look good to me (I might squash patch 5 with 4 while applying).

- Mani

> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/pci.h   |  2 ++
>  drivers/pci/probe.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 13d998fbacce..409aca7d737a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -108,6 +108,8 @@ struct pcie_tlp_log;
>  				 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
>  
>  extern const unsigned char pcie_link_speed[];
> +unsigned char pcie_get_link_speed(unsigned int speed);
> +
>  extern bool pci_early_dump;
>  
>  extern struct mutex pci_rescan_remove_lock;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..d6592898330c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -783,6 +783,22 @@ const unsigned char pcie_link_speed[] = {
>  };
>  EXPORT_SYMBOL_GPL(pcie_link_speed);
>  
> +/**
> + * pcie_link_speed_value - Get speed value from PCIe generation number
> + * @speed: PCIe speed (1-based: 1 = 2.5GT, 2 = 5GT, ...)
> + *
> + * Returns the speed value (e.g., PCIE_SPEED_2_5GT) if @speed is valid,
> + * otherwise returns PCI_SPEED_UNKNOWN.
> + */
> +unsigned char pcie_get_link_speed(unsigned int speed)
> +{
> +	if (speed >= ARRAY_SIZE(pcie_link_speed))
> +		return PCI_SPEED_UNKNOWN;
> +
> +	return pcie_link_speed[speed];
> +}
> +EXPORT_SYMBOL_GPL(pcie_get_link_speed);
> +
>  const char *pci_speed_string(enum pci_bus_speed speed)
>  {
>  	/* Indexed by the pci_bus_speed enum */
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்


^ permalink raw reply

* Re: [PATCH 00/12] treewide: Convert buses to use generic driver_override
From: Danilo Krummrich @ 2026-03-26 17:38 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Russell King, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ioana Ciornei, Nipun Gupta, Nikhil Agarwal, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Bjorn Helgaas,
	Armin Wolf, Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
	Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	Harald Freudenberger, Holger Dengler, Mark Brown, Jason Wang,
	Xuan Zhuo, Eugenio Pérez, Alex Williamson, Juergen Gross,
	Stefano Stabellini, Oleksandr Tyshchenko,
	Christophe Leroy (CS GROUP), linux-kernel, driver-core,
	linuxppc-dev, linux-hyperv, linux-pci, platform-driver-x86,
	linux-arm-msm, linux-remoteproc, linux-s390, linux-spi,
	virtualization, kvm, xen-devel, linux-arm-kernel
In-Reply-To: <20260325052919-mutt-send-email-mst@kernel.org>

On Wed Mar 25, 2026 at 10:29 AM CET, Michael S. Tsirkin wrote:
> vdpa bits:
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> I assume it'll all be merged together?

I can take it through the driver-core tree if you prefer, but you can also pick
it up yourself.


^ permalink raw reply

* Re: [PATCH net-next v4 1/2] net: stmmac: provide flag to disable EEE
From: Kieran Bingham @ 2026-03-26 17:31 UTC (permalink / raw)
  To: Laurent Pinchart, imx, netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Fabio Estevam,
	Francesco Dolcini, Frank Li, Jakub Kicinski, Joy Zou,
	Marco Felsch, Paolo Abeni, Pengutronix Kernel Team,
	Russell King (Oracle), Stefan Klug, linux-arm-kernel
In-Reply-To: <20260325210003.2752013-2-laurent.pinchart@ideasonboard.com>

Quoting Laurent Pinchart (2026-03-25 21:00:02)
> From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
> 
> Some platforms have problems when EEE is enabled, and thus need a way
> to disable stmmac EEE support. Add a flag before the other LPI related
> flags which tells stmmac to avoid populating the phylink LPI
> capabilities, which causes phylink to call phy_disable_eee() for any
> PHY that is attached to the affected phylink instance.
> 
> iMX8MP is an example - the lpi_intr_o signal is wired to an OR gate
> along with the main dwmac interrupts. Since lpi_intr_o is synchronous
> to the receive clock domain, and takes four clock cycles to clear, this
> leads to interrupt storms as the interrupt remains asserted for some
> time after the LPI control and status register is read.
> 
> This problem becomes worse when the receive clock from the PHY stops
> when the receive path enters LPI state - which means that lpi_intr_o
> can not deassert until the clock restarts. Since the LPI state of the
> receive path depends on the link partner, this is out of our control.
> We could disable RX clock stop at the PHY, but that doesn't get around
> the slow-to-deassert lpi_intr_o mentioned in the above paragraph.
> 
> Previously, iMX8MP worked around this by disabling gigabit EEE, but
> this is insufficient - the problem is also visible at 100M speeds,
> where the receive clock is slower.
> 
> There is extensive discussion and investigation in the thread linked
> below, the result of which is summarised in this commit message.

This one has been a roller coaster, but I'm glad we got to the bottom of
it.

I hope someone from synopsis takes note and and documents this for
future silicon integrations.

Can't wait to see where else this pops up.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Closes: https://lore.kernel.org/r/20251026122905.29028-1-laurent.pinchart@ideasonboard.com
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Tested-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  7 ++++++-
>  include/linux/stmmac.h                            | 13 +++++++------
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9b6b49331639..ce51b9c22129 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1438,7 +1438,12 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
>                                  config->supported_interfaces,
>                                  pcs->supported_interfaces);
>  
> -       if (priv->dma_cap.eee) {
> +       /* Some platforms, e.g. iMX8MP, wire lpi_intr_o to the same interrupt
> +        * used for stmmac's main interrupts, which leads to interrupt storms.
> +        * STMMAC_FLAG_EEE_DISABLE allows EEE to be disabled on such platforms.
> +        */
> +       if (priv->dma_cap.eee &&
> +           !(priv->plat->flags & STMMAC_FLAG_EEE_DISABLE)) {
>                 /* The GMAC 3.74a databook states that EEE is only supported
>                  * in MII, GMII, and RGMII interfaces.
>                  */
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 5b2bece81448..e62d21afd56d 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -207,12 +207,13 @@ enum dwmac_core_type {
>  #define STMMAC_FLAG_MULTI_MSI_EN               BIT(7)
>  #define STMMAC_FLAG_EXT_SNAPSHOT_EN            BIT(8)
>  #define STMMAC_FLAG_INT_SNAPSHOT_EN            BIT(9)
> -#define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI         BIT(10)
> -#define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING      BIT(11)
> -#define STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP      BIT(12)
> -#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY   BIT(13)
> -#define STMMAC_FLAG_KEEP_PREAMBLE_BEFORE_SFD   BIT(14)
> -#define STMMAC_FLAG_SERDES_SUPPORTS_2500M      BIT(15)
> +#define STMMAC_FLAG_EEE_DISABLE                        BIT(10)
> +#define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI         BIT(11)
> +#define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING      BIT(12)
> +#define STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP      BIT(13)
> +#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY   BIT(14)
> +#define STMMAC_FLAG_KEEP_PREAMBLE_BEFORE_SFD   BIT(15)
> +#define STMMAC_FLAG_SERDES_SUPPORTS_2500M      BIT(16)
>  
>  struct mac_device_info;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>


^ permalink raw reply

* Re: [PATCH v2 0/3] Inline helpers into Rust without full LTO
From: Miguel Ojeda @ 2026-03-26 17:31 UTC (permalink / raw)
  To: Christian Schrefl
  Cc: Alice Ryhl, Ard Biesheuvel, Jamie Cunliffe, Will Deacon,
	Catalin Marinas, Russell King (Oracle), Miguel Ojeda, a.hindborg,
	acourbot, akpm, anton.ivanov, bjorn3_gh, boqun.feng, dakr, david,
	gary, johannes, justinstitt, linux-arm-kernel, linux-kbuild,
	linux-kernel, linux-mm, linux-um, llvm, lossin, mark.rutland,
	mmaurer, morbo, nathan, nick.desaulniers+lkml, nicolas.schier,
	nsc, peterz, richard, rust-for-linux, tmgross, urezki
In-Reply-To: <9cf5a94c-0f37-446c-b63d-ddac5674d220@gmail.com>

On Thu, Mar 26, 2026 at 3:31 PM Christian Schrefl
<chrisi.schrefl@gmail.com> wrote:
>
> It should probably be fine to use armv7a-none-eabi. I've mostly used
> arm-unknown-linux-gnueabi since I though it needed to match the
> bindgen-target (which is -linux-gnu for all architectures) and
> because from what I understand clang also uses arm-linux-gnueabi [1].
> Also when I selected the target I thought that we would also support
> armv6, but since I had no v6 hardware to test on I disabled it.

I see, thanks for the quick reply!

My current thinking is to enable this only for x86_64 and arm64, so if
you/arm decide to switch the target (and/or enable this experimental
feature), then that can be done without a rush, independently.

(Please take a look at Russell's reply as well about the short enums thing.)

Cheers,
Miguel


^ permalink raw reply

* Re: [PATCH v2 0/3] Inline helpers into Rust without full LTO
From: Miguel Ojeda @ 2026-03-26 17:30 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Christian Schrefl, Alice Ryhl, Ard Biesheuvel, Jamie Cunliffe,
	Will Deacon, Catalin Marinas, Miguel Ojeda, a.hindborg, acourbot,
	akpm, anton.ivanov, bjorn3_gh, boqun.feng, dakr, david, gary,
	johannes, justinstitt, linux-arm-kernel, linux-kbuild,
	linux-kernel, linux-mm, linux-um, llvm, lossin, mark.rutland,
	mmaurer, morbo, nathan, nick.desaulniers+lkml, nicolas.schier,
	nsc, peterz, richard, rust-for-linux, tmgross, urezki
In-Reply-To: <acVOL5Psz6kHlhq2@shell.armlinux.org.uk>

On Thu, Mar 26, 2026 at 4:18 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> I'm not sure if this is still true, but I believe it used to be the case
> that the -linux-gnueabi target has one behaviour for enums (fixed size)
> whereas -none-eabi, the size of the type depends on the range of values
> included in the enum.
>
> Certianly, when Arm Ltd were proposing EABI, EABI had the latter
> behaviour, and I think there were cases where Linux used "enum" in
> its UAPI.

Short enums? I see `c-enum-min-bits` in the armv7a-none-eabi built-in
`rustc` target, and indeed:

    #![no_std]

    #[repr(C)]
    enum T {
        A,
        B,
    }

    pub static S: usize = core::mem::size_of::<T>();

is 1 for that one, and 4 for the other.

Thanks!

Cheers,
Miguel


^ permalink raw reply

* Re: [PATCH net-next 1/2] net: stmmac: remove axi_kbbe, axi_mb and axi_rb members
From: Simon Horman @ 2026-03-26 17:29 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, Conor Dooley,
	David S. Miller, devicetree, Eric Dumazet, Giuseppe Cavallaro,
	Jakub Kicinski, Jose Abreu, Krzysztof Kozlowski, linux-arm-kernel,
	linux-stm32, netdev, Paolo Abeni, Rob Herring, Yao Zi
In-Reply-To: <E1w4ydo-0000000Dlpb-34jd@rmk-PC.armlinux.org.uk>

On Tue, Mar 24, 2026 at 10:05:40AM +0000, Russell King (Oracle) wrote:
> axi_kbbe, axi_mb and axi_rb are all written, but nothing ever reads
> their values. Remove the code that sets these and the struct members.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Hi Russell,

FYI, AI review suggests that these fields should also be removed from
Documentation/networking/device_drivers/ethernet/stmicro/stmmac.rst


^ permalink raw reply

* Re: [PATCH net-next v2 4/4] net: phy: Introduce Airoha AN8801/R Gigabit Ethernet PHY driver
From: Maxime Chevallier @ 2026-03-26 17:25 UTC (permalink / raw)
  To: Andrew Lunn, Russell King (Oracle)
  Cc: Andrew Lunn, Louis-Alexis Eyraud, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, AngeloGioacchino Del Regno, Heiner Kallweit,
	kevin-kw.huang, macpaul.lin, matthias.bgg, kernel, netdev,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <425217fd-f74c-455c-9aa7-5b6682d887fa@lunn.ch>



On 26/03/2026 17:56, Andrew Lunn wrote:
>> What is the timing requirements for a system going into suspend vs a WoL
>> packet being sent? Should a WoL packet abort entry into suspend? If yes,
>> then we need to program the MAC before the PHY is suspended, because
>> suspend could already be in progress.
> 
> We would then need to hook into the NETDEV_CHANGEADDR notifier, and
> call into the PHY driver to let it reprogram the MAC address.

We would also probably have to re-do that MAC addr programming at phy
attach time ? If the PHY isn't attached (e.g. link admin-down on some
boards), we can't use that notifier as we don't know what netdev to
listen to. Or I'm missing something ?

Maxime

> 
>      Andrew
> 



^ permalink raw reply

* Re: Re: [PATCH net-next v5 3/3] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller
From: Simon Horman @ 2026-03-26 17:21 UTC (permalink / raw)
  To: 李志
  Cc: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, pjw, palmer, aou, alex, linux-riscv, linux-stm32,
	linux-arm-kernel, linux-kernel, maxime.chevallier, ningyu, linmin,
	pinkesh.vaghela, pritesh.patel, weishangjuan
In-Reply-To: <2a12c839.5e64.19d28232537.Coremail.lizhi2@eswincomputing.com>

On Thu, Mar 26, 2026 at 11:14:45AM +0800, 李志 wrote:

...

> Hi Simon,
> 
> Thanks for your review.
> 
> You're right, this build failure is due to an invalid clock reference
> ("clk") in the Ethernet node, which does not correspond to an existing
> clock provider label in the current DTS.
> 
> For context, this was discussed during an earlier revision:
> https://lore.kernel.org/lkml/5dea8ce0.4435.19c471231f5.Coremail.lizhi2@eswincomputing.com/
> 
> The EIC7700 clock controller support has since been applied, so I will
> update the DTS to reference the correct clock provider and ensure the
> build passes cleanly.
> 
> I will fix this in the next revision (v6).

Thanks.

Please be aware that if the patch is routed via net-next,
then the dependency will need to be present in net-next.


^ permalink raw reply

* Re: [PATCH net] net: ethernet: mtk_ppe: avoid NULL deref when gmac0 is disabled
From: Simon Horman @ 2026-03-26 17:15 UTC (permalink / raw)
  To: Sven Eckelmann (Plasma Cloud)
  Cc: Felix Fietkau, Lorenzo Bianconi, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno, Elad Yifee, netdev, linux-kernel,
	linux-arm-kernel, linux-mediatek, stable
In-Reply-To: <20260324-wed-crash-gmac0-disabled-v1-1-3bc388aee565@simonwunderlich.de>

On Tue, Mar 24, 2026 at 09:36:01AM +0100, Sven Eckelmann (Plasma Cloud) wrote:
> If the gmac0 is disabled, the precheck for a valid ingress device will
> cause a NULL pointer deref and crash the system. This happens because
> eth->netdev[0] will be NULL but the code will directly try to access
> netdev_ops.
> 
> Instead of just checking for the first net_device, it must be checked if
> any of the mtk_eth net_devices is matching the netdev_ops of the ingress
> device.
> 
> Cc: stable@vger.kernel.org
> Fixes: 73cfd947dbdb ("net: ethernet: mtk_eth_soc: ppe: prevent ppe update for non-mtk devices")
> Signed-off-by: Sven Eckelmann (Plasma Cloud) <se@simonwunderlich.de>

Reviewed-by: Simon Horman <horms@kernel.org>



^ permalink raw reply

* Re: [PATCH v2 0/3] Inline helpers into Rust without full LTO
From: Miguel Ojeda @ 2026-03-26 17:13 UTC (permalink / raw)
  To: Nathan Chancellor, Arnd Bergmann, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini, Linux ARM, soc,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	linux-kernel
  Cc: Miguel Ojeda, Nicolas Schier, Nick Desaulniers, Bill Wendling,
	Justin Stitt, David Gow, Russell King, Richard Weinberger,
	Anton Ivanov, Johannes Berg, aliceryhl, linux-um, llvm,
	linux-kbuild, a.hindborg, acourbot, akpm, bjorn3_gh, boqun.feng,
	dakr, gary, linux-mm, lossin, mark.rutland, mmaurer,
	nicolas.schier, peterz, rust-for-linux, tmgross, urezki, will
In-Reply-To: <20260326024226.GB2302780@ax162>

On Thu, Mar 26, 2026 at 3:42 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> I do agree with some of the concerns that adding an architecure
> dimension to this is a little complicated. I would rather try to flush
> out those build problems with patches and keep it enabled for all
> architectures. At the same time though, I understand that enabling it
> for the "tier 1" architectures is a low barrier of entry for getting the
> feature upstream, validated, and distributed to the majority of people
> that would actually use and depend on it, so I ultimately leave that
> call up to you.

Thanks! I agree that it would be ideal to get it clean everywhere, but
given it is experimental and that arch maintainers should likely known
about this, I think it is best to start simple first.

In fact, let me Cc the x86 and arm64 maintainers so that they are aware.

My current thinking is that I will add:

  depends on ARM64 || X86_64

and try to land it this cycle.

My understanding is that this will be used at least by Google, mostly
for Android (and mostly arm64, but possibly x86_64 too), so I think at
least arm64 will see some actual users on an ongoing basis, i.e. apart
from the "synthetic" testing I was doing.

> No real concern on that front but .gitignore has a command to run when
> modifying it, which will require a !timeconst.bc in a
> kernel/time/.gitignore file.

Yeah, that is the exception I mentioned. Initially I thought about
putting it in a local `.gitignore`, because local is best. But another
option, with a different kind of locality, is keeping the exception
close to the rule, i.e. in the global one, which has the advantage of
showing us all the exceptions easily (and being able to write a
comment for all of them at once).

I am not sure what is best clearer, but I am happy to do either:

    diff --git a/.gitignore b/.gitignore
    index 3a7241c941f5..3044b9590f05 100644
    --- a/.gitignore
    +++ b/.gitignore
    @@ -13,6 +13,7 @@
     .*
     *.a
     *.asn1.[ch]
    +*.bc
     *.bin
     *.bz2
     *.c.[012]*.*
    @@ -184,3 +185,6 @@ sphinx_*/

     # Rust analyzer configuration
     /rust-project.json
    +
    +# bc language scripts (not LLVM bitcode)
    +!kernel/time/timeconst.bc

Thanks again!

Cheers,
Miguel


^ permalink raw reply

* Re: [PATCH] PCI: imx6: Separate PERST# assertion from core reset functions
From: Manivannan Sadhasivam @ 2026-03-26 17:13 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, Frank.Li, bhelgaas, lpieralisi,
	kwilczynski, robh, s.hauer, festevam, Sherry Sun
  Cc: imx, kernel, linux-pci, linux-arm-kernel, linux-kernel
In-Reply-To: <20260306030456.1032815-1-sherry.sun@nxp.com>


On Fri, 06 Mar 2026 11:04:56 +0800, Sherry Sun wrote:
> The imx_pcie_assert_core_reset() and imx_pcie_deassert_core_reset()
> functions are primarily intended to reset the RC controller itself, not
> the remote PCIe endpoint devices. However, the PERST# GPIO control was
> previously embedded within these functions, which conflates two distinct
> reset operations.
> 
> Move the PERST# GPIO handling into a dedicated function
> imx_pcie_assert_perst(). This makes the code more maintainable and
> prepares for parsing the reset-gpios property according to the new
> Root Port DT binding in subsequent patches.
> 
> [...]

Applied, thanks!

[1/1] PCI: imx6: Separate PERST# assertion from core reset functions
      commit: 180ea823bb45eb71dd5ed0dc0b78633accd21096

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>



^ permalink raw reply

* Re: [PATCH V2] PCI: imx6: Change imx_pcie_deassert_core_reset() to return void
From: Manivannan Sadhasivam @ 2026-03-26 17:10 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, Frank.Li, bhelgaas, lpieralisi,
	kwilczynski, robh, krzk+dt, s.hauer, festevam, Sherry Sun
  Cc: imx, kernel, linux-pci, linux-arm-kernel, linux-kernel
In-Reply-To: <20260306021247.991976-1-sherry.sun@nxp.com>


On Fri, 06 Mar 2026 10:12:47 +0800, Sherry Sun wrote:
> The function imx_pcie_deassert_core_reset() always returns 0 and the
> return value is not used meaningfully by its callers.
> 
> Change the return type from int to void to simplify the code and
> remove unnecessary error handling paths. No functional change intended.
> 
> 
> [...]

Applied, thanks!

[1/1] PCI: imx6: Change imx_pcie_deassert_core_reset() to return void
      commit: 192b8a1bf81c17852b6cf540c95b0a3bcc9c58c4

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>



^ permalink raw reply

* Re: [PATCH v2] thermal/drivers/mediatek/lvts: add missing fields for mt7987
From: Frank Wunderlich @ 2026-03-26 17:07 UTC (permalink / raw)
  To: Frank Wunderlich, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: linux-pm, linux-kernel, Daniel Golle, Laura Nao, linux-mediatek,
	linux-arm-kernel
In-Reply-To: <20260312202433.39092-1-linux@fw-web.de>

Hi,

just a friedly reminder

@angelo do my mails get tagged as spam or similar?


Am 12. März 2026 um 21:24 schrieb "Frank Wunderlich" <linux@fw-web.de mailto:linux@fw-web.de?to=%22Frank%20Wunderlich%22%20%3Clinux%40fw-web.de%3E >:

> 
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Commit a4c40559499f ("thermal/drivers/mediatek/lvts: Add platform ops to
> support alternative conversion logic") added new ops field which causes
> crash on mt7987.
> 
> [ 1.518540] Internal error: Oops: 0000000096000005 [#1] SMP
> ...
> [ 1.564481] pc : lvts_get_temp+0x84/0xc4
> [ 1.564492] lr : lvts_get_temp+0x60/0xc4
> ...
> [ 1.620900] Call trace:
> [ 1.631753] lvts_get_temp+0x84/0xc4 (P)
> [ 1.645471] __thermal_zone_get_temp+0x24/0x11c
> [ 1.656502] __thermal_zone_device_update+0x9c/0x52c
> 
> Add the new ops member added in 7.0-rc1 for mt7987 too.
> 
> Commit 6931d597c5ef ("thermal/drivers/mediatek/lvts: Make number of
> calibration offsets configurable") introduced field num_cal_offsets
> in lvts_data. Add this for mt7987 too.
> 
> Fixes: 78c24e67d6f8 ("thermal/drivers/mediatek/lvts_thermal: Add mt7987 support")
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  drivers/thermal/mediatek/lvts_thermal.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index a9617d5e0077..1224ce0c1507 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -2026,6 +2026,8 @@ static const struct lvts_data mt7987_lvts_ap_data = {
>  .temp_offset = LVTS_COEFF_B_MT7987,
>  .gt_calib_bit_offset = 32,
>  .def_calibration = 19380,
> + .num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT7988,
> + .ops = &lvts_platform_ops_mt7988,
>  };
>  
>  static const struct lvts_data mt7988_lvts_ap_data = {
> -- 
> 2.43.0
> 

regards Frank


^ permalink raw reply

* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Maxime Ripard @ 2026-03-26 17:02 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Nicolas Frattaroli, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Thomas Zimmermann, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Sandy Huang, Heiko Stübner, Andy Yan,
	Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	Dmitry Baryshkov, Sascha Hauer, Rob Herring, Jonathan Corbet,
	Shuah Khan, kernel, amd-gfx, dri-devel, linux-kernel,
	linux-arm-kernel, linux-rockchip, intel-gfx, intel-xe, linux-doc,
	Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <acQsw3Wi_xVlBZ8d@intel.com>

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

On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > +enum drm_connector_color_format {
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > +	 * helpers should pick a suitable color format. All implementations of a
> > > > > > > > +	 * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > +	 * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > +	 * semantics.
> > > > > > > > +	 *
> > > > > > > > +	 * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > +	 * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > +	 * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > +	 * YCbCr 4:2:0.
> > > > > > > > +	 *
> > > > > > > > +	 * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > +	 * format selection picks the first chain of bridge formats that works,
> > > > > > > > +	 * as has already been the case before the introduction of the "color
> > > > > > > > +	 * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > +	 * bus output formats by preference, or agree on a unified auto format
> > > > > > > > +	 * selection logic that's implemented in a common state helper (like
> > > > > > > > +	 * how HDMI does it).
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > +	 * not subsampled)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > +	 * with horizontal subsampling)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > +	 * with horizontal and vertical subsampling)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > 
> > > > > > > Seems like this should document what the quantization range
> > > > > > > should be for each format.
> > > > > > > 
> > > > > > 
> > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > enum is more abstract than that, and is there to communicate
> > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > by other properties.
> > > > > > 
> > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > be added; no digital display protocol standard supports it to my
> > > > > > knowledge, and hopefully none ever will.
> > > > > 
> > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > 
> > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > RGB can either be full or limited range depending on the 
> > > > > "Broadcast RGB" property and other related factors.
> > > > 
> > > > So far the HDMI state has both the format and quantization range as
> > > > different fields. I'm not sure we need to document the range in the
> > > > format field, maybe only mention it's not part of the format but has a
> > > > field of its own?
> > > 
> > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > I think the assumption is limited range everywhere.
> > > 
> > > But I'm not really concerned about documenting struct members.
> > > What I'm talking about is the *uapi* docs. Surely userspace
> > > will want to know what the new property actually does so the
> > > uapi needs to be documented properly. And down the line some
> > > new driver might also implement the wrong behaviour if there
> > > is no clear specification.
> > 
> > Ack
> > 
> > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > - YCbCr limited range 
> > > - RGB full range if "Broadcast RGB" property is not present
> > 
> > Isn't it much more complicated than that for HDMI though? My
> > recollection was that any VIC but VIC1 would be limited range, and
> > anything else full range?
> 
> Do we have some driver that implements the CTA-861 CE vs. IT mode
> logic but doesn't expose the "Broadcast RGB" property? I was hoping
> those would always go hand in hand now.

I'm not sure. i915 and the HDMI state helpers handle it properly (I
think?) but it looks like only vc4 registers the Broadcast RGB property
and uses the HDMI state helpers.

And it looks like amdgpu registers Broadcast RGB but doesn't use
drm_default_rgb_quant_range() which seems suspicious?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v8 5/6] Fix error handling in probe function.
From: Russell King (Oracle) @ 2026-03-26 16:57 UTC (permalink / raw)
  To: Jitendra Vegiraju
  Cc: netdev, alexandre.torgue, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
	daniel, hawk, john.fastabend, rohan.g.thomas, linux-kernel,
	linux-stm32, linux-arm-kernel, bpf, andrew+netdev, horms, sdf, me,
	siyanteng, prabhakar.mahadev-lad.rj, weishangjuan, wens,
	vladimir.oltean, lizhi2, boon.khai.ng, maxime.chevallier,
	chenchuangyu, yangtiezhu, ovidiu.panait.rb, chenhuacai,
	florian.fainelli, quic_abchauha
In-Reply-To: <20260320211921.1202058-6-jitendra.vegiraju@broadcom.com>

On Fri, Mar 20, 2026 at 02:19:20PM -0700, Jitendra Vegiraju wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> 
> Software node created in probe function is not being cleaned up if
> the probe function returns an error.
> The stmmac core provides mechanism to handle this error condition
> with plat->init, plat->exit helper functions.
> Move glue driver's initialization code to plat->init function.
> If the probe function returns an error, plat->exit function is
> called. Handle any glue driver level cleanup in the plat->exit
> handler.
> Use devm_add_action_or_reset() to register a callback to free
> irq vectors automatically, simplifying error handling in probe().
> 
> Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
> Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>

Oh, you did fix it. Please merge this into patch 4, there is no need
to have this fix seperate.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply

* Re: [PATCH net-next v2 4/4] net: phy: Introduce Airoha AN8801/R Gigabit Ethernet PHY driver
From: Andrew Lunn @ 2026-03-26 16:56 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Louis-Alexis Eyraud, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, AngeloGioacchino Del Regno, Heiner Kallweit,
	kevin-kw.huang, macpaul.lin, matthias.bgg, kernel, netdev,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <acVQQQiciPQBkQOG@shell.armlinux.org.uk>

> What is the timing requirements for a system going into suspend vs a WoL
> packet being sent? Should a WoL packet abort entry into suspend? If yes,
> then we need to program the MAC before the PHY is suspended, because
> suspend could already be in progress.

We would then need to hook into the NETDEV_CHANGEADDR notifier, and
call into the PHY driver to let it reprogram the MAC address.

     Andrew


^ permalink raw reply

* Re: [PATCH v2 0/2] PCI: dwc: Add multi-port controller support
From: Manivannan Sadhasivam @ 2026-03-26 16:56 UTC (permalink / raw)
  To: Sumit Kumar
  Cc: Bjorn Helgaas, Jingoo Han, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
	Alim Akhtar, Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Yue Wang, Neil Armstrong,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Paul Walmsley,
	Greentime Hu, Samuel Holland, Chuanhua Lei, Marek Vasut,
	Yoshihiro Shimoda, Geert Uytterhoeven, Magnus Damm,
	Pratyush Anand, Thierry Reding, Jonathan Hunter, linux-pci,
	linux-kernel, linux-arm-kernel, linux-samsung-soc, imx,
	linux-amlogic, linux-arm-msm, linux-renesas-soc, linux-tegra,
	linux-riscv
In-Reply-To: <20260305-dt-parser-v2-0-85836db8dc06@oss.qualcomm.com>

On Thu, Mar 05, 2026 at 11:50:35AM +0530, Sumit Kumar wrote:
> This series adds support for multi-port PCIe controllers in the DesignWare
> driver. Currently, the driver only supports a single Root Port with
> controller-level properties, which doesn't work for multi-port controllers
> where each port may have different configurations.
> 
> This series introduces a per-port structure and parsing API that allows 
> each Root Port to be configured independently via pcie@N child nodes in
> device tree, while maintaining backward compatibility with existing 
> single-port bindings.
> 

This patch touches multiple host controller drivers. I'd appreciate if the
platform maintainers could test this series and give their tested-by tag to make
sure we don't regress.

- Mani

> Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> ---
> Changes in v2:
> - Fix error code preservation in dw_pcie_resume_noirq() to return actual
>   error from dw_pcie_wait_for_link() instead of hardcoded -ETIMEDOUT (Mani).
> - Initialize ret variable to -ENOENT in dw_pcie_parse_root_ports() (Mani).
> - dw_pcie_host_init(): Remove -ENOENT error skipping to make parsing
>   failures fatal for now, add TODO comment about making properties
>   optional later (Mani).
> - Link to v1: https://lore.kernel.org/r/20260105-dt-parser-v1-0-b11c63cb5e2c@oss.qualcomm.com
> 
> ---
> Sumit Kumar (2):
>       PCI: API changes for multi-port controller support
>       PCI: dwc: Add multi-port controller support
> 
>  drivers/pci/controller/dwc/pci-exynos.c           |   4 +-
>  drivers/pci/controller/dwc/pci-imx6.c             |  15 +-
>  drivers/pci/controller/dwc/pci-meson.c            |   1 -
>  drivers/pci/controller/dwc/pcie-designware-host.c | 175 ++++++++++++++++++----
>  drivers/pci/controller/dwc/pcie-designware.c      |  32 ++--
>  drivers/pci/controller/dwc/pcie-designware.h      |  17 ++-
>  drivers/pci/controller/dwc/pcie-fu740.c           |   6 +-
>  drivers/pci/controller/dwc/pcie-intel-gw.c        |  13 +-
>  drivers/pci/controller/dwc/pcie-qcom-common.c     |   5 +-
>  drivers/pci/controller/dwc/pcie-qcom-ep.c         |   4 +-
>  drivers/pci/controller/dwc/pcie-qcom.c            |   4 +-
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c       |  13 +-
>  drivers/pci/controller/dwc/pcie-spear13xx.c       |   5 +-
>  drivers/pci/controller/dwc/pcie-tegra194.c        |   4 +-
>  drivers/pci/of.c                                  |   6 +-
>  drivers/pci/pci.h                                 |   2 +
>  16 files changed, 232 insertions(+), 74 deletions(-)
> ---
> base-commit: 097a6c336d0080725c626fda118ecfec448acd0f
> change-id: 20251010-dt-parser-98b50ce18fc1
> 
> Best regards,
> -- 
> Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> 

-- 
மணிவண்ணன் சதாசிவம்


^ permalink raw reply

* Re: [PATCH net-next v8 4/6] Add PCI driver support for BCM8958x
From: Russell King (Oracle) @ 2026-03-26 16:55 UTC (permalink / raw)
  To: Jitendra Vegiraju
  Cc: netdev, alexandre.torgue, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
	daniel, hawk, john.fastabend, rohan.g.thomas, linux-kernel,
	linux-stm32, linux-arm-kernel, bpf, andrew+netdev, horms, sdf, me,
	siyanteng, prabhakar.mahadev-lad.rj, weishangjuan, wens,
	vladimir.oltean, lizhi2, boon.khai.ng, maxime.chevallier,
	chenchuangyu, yangtiezhu, ovidiu.panait.rb, chenhuacai,
	florian.fainelli, quic_abchauha
In-Reply-To: <20260320211921.1202058-5-jitendra.vegiraju@broadcom.com>

On Fri, Mar 20, 2026 at 02:19:19PM -0700, Jitendra Vegiraju wrote:
> +static const struct property_entry fixed_link_properties[] = {
> +	PROPERTY_ENTRY_U32("speed", 10000),
> +	PROPERTY_ENTRY_BOOL("full-duplex"),
> +	PROPERTY_ENTRY_BOOL("pause"),
> +	{ }
> +};
> +
> +static const struct software_node parent_swnode = {
> +	.name = "phy-device",
> +};
> +
> +static const struct software_node fixed_link_swnode = {
> +	.name = "fixed-link",           /* MUST be named "fixed-link" */
> +	.parent = &parent_swnode,
> +	.properties = fixed_link_properties,
> +};
> +
> +static const struct software_node *brcm_swnodes[] = {
> +	&parent_swnode,
> +	&fixed_link_swnode,
> +	NULL
> +};

Looking at this structure, I'm not sure it's correct. You seem to have:

pci_device
- "phy-device" swnode attached here (which describes the PCI device,
  which isn't any kind of PHY)
	- "fixed-link" attached as a child

The "fixed-link" is a property for the local network device which
signifies that there isn't a PHY attached or there's an inaccessible
PHY that only operates with one set of settings.

Maybe rename "phy-device" to "ethernet"?

> +
> +struct brcm_priv_data {
> +	void __iomem *mbox_regs;    /* MBOX  Registers*/
> +	void __iomem *misc_regs;    /* MISC  Registers*/
> +	void __iomem *xgmac_regs;   /* XGMAC Registers*/
> +};
> +
> +struct dwxgmac_brcm_pci_info {
> +	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
> +};
> +
> +static void misc_iowrite(struct brcm_priv_data *brcm_priv,
> +			 u32 reg, u32 val)
> +{
> +	iowrite32(val, brcm_priv->misc_regs + reg);
> +}
> +
> +static void dwxgmac_brcm_common_default_data(struct plat_stmmacenet_data *plat)
> +{
> +	int i;
> +
> +	plat->force_sf_dma_mode = true;
> +	plat->mac_port_sel_speed = SPEED_10000;
> +	plat->clk_ptp_rate = 125000000;
> +	plat->clk_ref_rate = 250000000;
> +	plat->tx_coe = true;
> +	plat->rx_coe = STMMAC_RX_COE_TYPE1;
> +	plat->rss_en = 1;
> +	plat->max_speed = SPEED_10000;
> +
> +	/* Set default value for multicast hash bins */
> +	plat->multicast_filter_bins = HASH_TABLE_SIZE;

Already the default setup by stmmac_plat_dat_alloc().

> +
> +	/* Set default value for unicast filter entries */
> +	plat->unicast_filter_entries = 1;

Already the default setup by stmmac_plat_dat_alloc().

> +
> +	/* Set the maxmtu to device's default */
> +	plat->maxmtu = BRCM_MAX_MTU;
> +
> +	/* Set default number of RX and TX queues to use */
> +	plat->tx_queues_to_use = BRCM_TX_Q_COUNT;
> +	plat->rx_queues_to_use = BRCM_RX_Q_COUNT;
> +
> +	plat->tx_sched_algorithm = MTL_TX_ALGORITHM_SP;
> +	for (i = 0; i < plat->tx_queues_to_use; i++) {
> +		plat->tx_queues_cfg[i].use_prio = false;

Already false.

> +		plat->tx_queues_cfg[i].prio = 0;

Already zero.

> +		plat->tx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;

Since MTL_QUEUE_AVB is zero, this is already the case.

> +	}

All three points taken together mean that this loop is not required
as all these members are being explicitly set to values of zero,
which they already hold.

> +
> +	plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
> +	for (i = 0; i < plat->rx_queues_to_use; i++) {
> +		plat->rx_queues_cfg[i].use_prio = false;

Already false.

> +		plat->rx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;

Since MTL_QUEUE_AVB is zero, this is already the case.

> +		plat->rx_queues_cfg[i].pkt_route = 0x0;

Already zero.

> +		plat->rx_queues_cfg[i].chan = i;

stmmac_plat_dat_alloc() already initialises plat->rx_queues_cfg[].chan.

> +	}

Taking all these points together, it means that this loop also isn't
required, since you're not changing anything that hasn't already been
setup.

> +}
> +
> +static int dwxgmac_brcm_default_data(struct pci_dev *pdev,
> +				     struct plat_stmmacenet_data *plat)
> +{
> +	/* Set common default data first */
> +	dwxgmac_brcm_common_default_data(plat);
> +	plat->core_type = DWMAC_CORE_25GMAC;
> +	plat->bus_id = 0;

The underlying devm_kzalloc() which allocates "plat" will clear the
struct to zeros, so this assignment to bus_id shouldn't be necessary.

> +	plat->phy_addr = 0;

You said there's no MDIO bus, so I don't think you need to initialise
plat->phy_addr. stmmac_plat_dat_alloc() will set this to -1.

> +	plat->phy_interface = PHY_INTERFACE_MODE_XGMII;
> +
> +	plat->dma_cfg->pbl = DEFAULT_DMA_PBL;
> +	plat->dma_cfg->pblx8 = true;
> +	plat->dma_cfg->aal = false;
> +	plat->dma_cfg->eame = true;
> +
> +	plat->axi->axi_wr_osr_lmt = 31;
> +	plat->axi->axi_rd_osr_lmt = 31;
> +	plat->axi->axi_fb = false;

devm_kzalloc() which is used to allocate plat->axi in the probe function
will zero out this structure, so axi_fb will already be false.

> +	plat->axi->axi_blen_regval = DMA_AXI_BLEN64;
> +	return 0;
> +}
> +
> +static struct dwxgmac_brcm_pci_info dwxgmac_brcm_pci_info = {
> +	.setup = dwxgmac_brcm_default_data,
> +};

It looks to me like this is a copy of stmmac_pci.c / dwmac-intel.c etc.
Do you know for certain that you're going to need to do different
setups depending on the PCI device?

What's the reasoning for the split between
dwxgmac_brcm_common_default_data() and dwxgmac_brcm_default_data() ?

> +
> +static void brcm_config_misc_regs(struct pci_dev *pdev,
> +				  struct brcm_priv_data *brcm_priv)
> +{
> +	pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
> +			       XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
> +	pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
> +			       XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
> +
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
> +
> +	/* Enable Switch Link */
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
> +		     XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
> +		     XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
> +		     XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
> +}
> +
> +static int brcm_config_multi_msi(struct pci_dev *pdev,
> +				 struct plat_stmmacenet_data *plat,
> +				 struct stmmac_resources *res)
> +{
> +	int ret;
> +	int i;
> +
> +	ret = pci_alloc_irq_vectors(pdev, BRCM_XGMAC_MSI_VECTOR_MAX,
> +				    BRCM_XGMAC_MSI_VECTOR_MAX,
> +				    PCI_IRQ_MSI | PCI_IRQ_MSIX);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "%s: multi MSI enablement failed\n",
> +			__func__);
> +		return ret;
> +	}
> +
> +	/* For RX MSI */
> +	for (i = 0; i < plat->rx_queues_to_use; i++)
> +		res->rx_irq[i] =
> +			pci_irq_vector(pdev,
> +				       BRCM_XGMAC_MSI_RX_VECTOR_START + i * 2);
> +
> +	/* For TX MSI */
> +	for (i = 0; i < plat->tx_queues_to_use; i++)
> +		res->tx_irq[i] =
> +			pci_irq_vector(pdev,
> +				       BRCM_XGMAC_MSI_TX_VECTOR_START + i * 2);
> +
> +	res->irq = pci_irq_vector(pdev, BRCM_XGMAC_MSI_MAC_VECTOR);
> +
> +	plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
> +	plat->flags |= STMMAC_FLAG_TSO_EN;
> +	plat->flags |= STMMAC_FLAG_SPH_DISABLE;
> +	return 0;
> +}
> +
> +static int brcm_pci_resume(struct device *dev, void *bsp_priv)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	brcm_config_misc_regs(pdev, bsp_priv);

Is it worth declaring struct pdev for one place that it's used?

	brcm_config_misc_regs(to_pci_dev(dev), bsp_priv);

should work just as well.

> +
> +	return stmmac_pci_plat_resume(dev, bsp_priv);
> +}
> +
> +static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
> +				  const struct pci_device_id *id)
> +{
> +	struct dwxgmac_brcm_pci_info *info =
> +		(struct dwxgmac_brcm_pci_info *)id->driver_data;
> +	struct plat_stmmacenet_data *plat;
> +	struct brcm_priv_data *brcm_priv;
> +	struct stmmac_resources res;
> +	struct device *dev;
> +	int rx_offset;
> +	int tx_offset;
> +	int vector;
> +	int ret;
> +
> +	dev = &pdev->dev;

As you go to the effort of declaring a struct device pointer, and
assign it, do you think it would be a good idea to either use it for
all &pdev->dev instances below, or just get rid of the two instances
that you actually use "dev" ?

I count six instances of "&pdev->dev" below vs two making use of "dev"
directly.

> +
> +	brcm_priv = devm_kzalloc(&pdev->dev, sizeof(*brcm_priv), GFP_KERNEL);
> +	if (!brcm_priv)
> +		return -ENOMEM;
> +
> +	plat = stmmac_plat_dat_alloc(dev);
> +	if (!plat)
> +		return -ENOMEM;
> +
> +	plat->axi = devm_kzalloc(&pdev->dev, sizeof(*plat->axi), GFP_KERNEL);
> +	if (!plat->axi)
> +		return -ENOMEM;
> +
> +	/* This device is directly attached to the switch chip internal to the
> +	 * SoC using XGMII interface. Since no MDIO is present, register
> +	 * fixed-link software_node to create phylink.
> +	 */
> +	software_node_register_node_group(brcm_swnodes);
> +	device_set_node(dev, software_node_fwnode(&parent_swnode));
> +
> +	/* Disable D3COLD as our device does not support it */
> +	pci_d3cold_disable(pdev);
> +
> +	/* Enable PCI device */
> +	ret = pcim_enable_device(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n",
> +			__func__);
> +		return ret;

What about cleaning up the swnodes ?

> +	}
> +
> +	pci_set_master(pdev);
> +
> +	memset(&res, 0, sizeof(res));
> +	res.addr = pcim_iomap_region(pdev, 0, pci_name(pdev));
> +	if (IS_ERR(res.addr))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(res.addr),
> +				     "failed to map IO region\n");

Convention is to have a blank line here.

> +	/* MISC Regs */
> +	brcm_priv->misc_regs = res.addr + BRCM_XGMAC_IOMEM_MISC_REG_OFFSET;
> +	/* MBOX Regs */
> +	brcm_priv->mbox_regs = res.addr + BRCM_XGMAC_IOMEM_MBOX_REG_OFFSET;
> +	/* XGMAC config Regs */
> +	res.addr += BRCM_XGMAC_IOMEM_CFG_REG_OFFSET;
> +	brcm_priv->xgmac_regs = res.addr;
> +
> +	plat->suspend		= stmmac_pci_plat_suspend;
> +	plat->resume		= brcm_pci_resume;
> +	plat->bsp_priv = brcm_priv;
> +
> +	ret = info->setup(pdev, plat);
> +	if (ret)
> +		return ret;

What about cleaning up the swnodes ?

> +
> +	pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
> +			       XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
> +	pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
> +			       XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
> +
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
> +
> +	/* SBD Interrupt */
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE);
> +	/* EP_DOORBELL Interrupt */
> +	misc_iowrite(brcm_priv,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE);
> +	/* EP_H0 Interrupt */
> +	misc_iowrite(brcm_priv,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE);
> +	/* EP_H1 Interrupt */
> +	misc_iowrite(brcm_priv,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET,
> +		     XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE);
> +
> +	rx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET;
> +	tx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET;
> +	vector = BRCM_XGMAC_MSI_RX_VECTOR_START;
> +	for (int i = 0; i < BRCM_MAX_DMA_CHANNEL_PAIRS; i++) {
> +		/* RX Interrupt */
> +		misc_iowrite(brcm_priv, rx_offset, vector++);
> +		/* TX Interrupt */
> +		misc_iowrite(brcm_priv, tx_offset, vector++);
> +		rx_offset += 4;
> +		tx_offset += 4;
> +	}

It looks like this device can program the MSI vector numbers. Does
it make sense to interleave them, or would it be simpler to have
all the receive vectors and then all the transmit vectors?

This also hard-codes the fact that BRCM_XGMAC_MSI_TX_VECTOR_START
is one more than BRCM_XGMAC_MSI_RX_VECTOR_START, which isn't nice
given that you use these macros when claiming the MSI vectors.

> +
> +	/* Enable Switch Link */
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
> +		     XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
> +		     XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
> +		     XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
> +	/* Enable MSI-X */
> +	misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET,
> +		     XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX);
> +
> +	ret = brcm_config_multi_msi(pdev, plat, &res);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"%s: ERROR: failed to enable IRQ\n", __func__);
> +		goto err_disable_msi;
> +	}
> +
> +	ret = stmmac_dvr_probe(&pdev->dev, plat, &res);
> +	if (ret)
> +		goto err_disable_msi;
> +
> +	return ret;
> +
> +err_disable_msi:
> +	pci_free_irq_vectors(pdev);

This is still buggy. What about cleaning up the swnodes?

> +
> +	return ret;
> +}
> +
> +static void dwxgmac_brcm_pci_remove(struct pci_dev *pdev)
> +{
> +	stmmac_dvr_remove(&pdev->dev);
> +	pci_free_irq_vectors(pdev);
> +	device_set_node(&pdev->dev, NULL);
> +	software_node_unregister_node_group(brcm_swnodes);

As the remove function does way more cleanup than the probe function,
this is a sign that the probe function is buggy. This is exactly why
I suggested using ->init and ->exit in the previous review. I seem
to have been ignored on that though... and the problem I already
pointed out remains.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply

* Re: [PATCH v1] PCI: imx6: Skip waiting for L2/L3 Ready on i.MX6SX
From: Manivannan Sadhasivam @ 2026-03-26 16:53 UTC (permalink / raw)
  To: frank.li, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
	s.hauer, kernel, festevam, Richard Zhu
  Cc: linux-pci, linux-arm-kernel, imx, linux-kernel, stable
In-Reply-To: <20260228080925.1558395-1-hongxing.zhu@nxp.com>


On Sat, 28 Feb 2026 16:09:25 +0800, Richard Zhu wrote:
> On i.MX6SX, the LTSSM registers become inaccessible after the
> PME_Turn_Off message is sent to the link. This prevents verification
> of whether the link has successfully entered the L2/L3 Ready state.
> 
> Add a new flag 'IMX_PCIE_FLAG_SKIP_L23_READY' to skip the L2/L3 Ready
> state check specifically for i.MX6SX PCIe controllers.
> 
> [...]

Applied, thanks!

[1/1] PCI: imx6: Skip waiting for L2/L3 Ready on i.MX6SX
      commit: 5f73cf1db829c21b7fd44a8d2587cd395b1b2d76

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>



^ permalink raw reply

* Re: [PATCH v1 2/3] arm64: dts: freescale: Add support for Variscite DART-MX93
From: Andrew Lunn @ 2026-03-26 16:41 UTC (permalink / raw)
  To: Stefano Radaelli
  Cc: linux-kernel, devicetree, imx, linux-arm-kernel, pierluigi.p,
	Stefano Radaelli, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Shawn Guo, Dario Binacchi, Alexander Stein, Maud Spierings,
	Josua Mayer, Markus Niebel, Francesco Dolcini, Primoz Fiser
In-Reply-To: <304e5edc9ed11a552ac96c28d1046e4b13d5f960.1774539301.git.stefano.r@variscite.com>

> +&eqos {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&pinctrl_eqos>;
> +	pinctrl-1 = <&pinctrl_eqos_sleep>;
> +	/*
> +	 * The required RGMII TX and RX 2ns delays are implemented directly
> +	 * in hardware via passive delay elements on the SOM PCB.
> +	 * No delay configuration is needed in software via PHY driver.
> +	 */
> +	phy-mode = "rgmii";
> +	phy-handle = <&ethphy0>;
> +	snps,clk-csr = <5>;
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		clock-frequency = < 1000000>;

As far as i know, stmmac does not implement that property. There is
the danger somebody does implement it sometime in the future, and your
board breaks because it does not work a 10MHz.

I suggest removing this.

	Andrew


^ 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