Re: [PATCH 1/2] net: gianfar: add device tree support

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Jan 27, 2026 at 12:12:16PM +0000, Renaud Barbier wrote:
> Like the PPC platforms, the NXP LS1021A Ethernet controller is managed
> by the gianfar driver. Since the LS1021A configuration uses a device tree,
> add probing through the device tree to the gianfar driver while preserving
> the tree-less PPC support.
> 
> Signed-off-by: Renaud Barbier <renaud.barbier@ametek.com>
> ---
>  arch/powerpc/include/asm/dma.h |  35 ++++-
>  drivers/net/Kconfig            |   2 +-
>  drivers/net/gianfar.c          | 261 +++++++++++++++++++++++++++++----
>  drivers/net/gianfar.h          |  36 +++++
>  4 files changed, 300 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/dma.h b/arch/powerpc/include/asm/dma.h
> index 27d269f491..dcd9e3d748 100644
> --- a/arch/powerpc/include/asm/dma.h
> +++ b/arch/powerpc/include/asm/dma.h
> @@ -8,6 +8,39 @@
>  #ifndef __ASM_DMA_H
>  #define __ASM_DMA_H
>  
> -/* empty */
> +#include <io.h>
> +
> +#define arch_sync_dma_for_cpu arch_sync_dma_for_cpu
> +static inline void arch_sync_dma_for_cpu(void *vaddr, size_t size,
> +					 enum dma_data_direction dir)
> +{
> +}
> +
> +#define arch_sync_dma_for_device arch_sync_dma_for_device
> +static inline void arch_sync_dma_for_device(void *vaddr, size_t size,
> +					    enum dma_data_direction dir)
> +{
> +}
> +
> +static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t address,
> +					   size_t size, enum dma_data_direction dir)
> +{
> +}
> +
> +static inline void dma_sync_single_for_device(struct device *dev, dma_addr_t address,
> +					      size_t size, enum dma_data_direction dir)
> +{
> +}
> +
> +static inline dma_addr_t dma_map_single(struct device *dev, void *ptr,
> +					size_t size, enum dma_data_direction dir)
> +{
> +	return virt_to_phys(ptr);
> +}
> +
> +static inline void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
> +				    size_t size, enum dma_data_direction dir)
> +{
> +}
>  
>  #endif /* __ASM_DMA_H */
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 068906078c..bd18416d3d 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -237,7 +237,7 @@ config DRIVER_NET_FSL_FMAN
>  
>  config DRIVER_NET_GIANFAR
>  	bool "Gianfar Ethernet"
> -	depends on ARCH_MPC85XX
> +	depends on ARCH_MPC85XX || ARCH_LS1021
>  	select PHYLIB
>  
>  config DRIVER_NET_KS8851_MLL
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 18b3c8849c..7a947a21da 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -8,14 +8,18 @@
>   * based on work by Andy Fleming
>   */
>  
> +#ifdef CONFIG_PPC
>  #include <asm/config.h>
> +#endif
>  #include <common.h>
>  #include <malloc.h>
>  #include <net.h>
> +#include <dma.h>
>  #include <init.h>
>  #include <driver.h>
>  #include <command.h>
>  #include <errno.h>
> +#include <of_address.h>
>  #include <asm/io.h>
>  #include <linux/phy.h>
>  #include <linux/err.h>
> @@ -77,11 +81,11 @@ static void gfar_adjust_link(struct eth_device *edev)
>  	u32 ecntrl, maccfg2;
>  
>  	priv->link = edev->phydev->link;
> -	priv->duplexity =edev->phydev->duplex;
> +	priv->duplexity = edev->phydev->duplex;
>  
>  	if (edev->phydev->speed == SPEED_1000)
>  		priv->speed = 1000;
> -	if (edev->phydev->speed == SPEED_100)
> +	else if (edev->phydev->speed == SPEED_100)
>  		priv->speed = 100;
>  	else
>  		priv->speed = 10;
> @@ -121,11 +125,11 @@ static void gfar_adjust_link(struct eth_device *edev)
>  		out_be32(regs + GFAR_ECNTRL_OFFSET, ecntrl);
>  		out_be32(regs + GFAR_MACCFG2_OFFSET, maccfg2);
>  
> -		dev_info(&edev->dev, "Speed: %d, %s duplex\n", priv->speed,
> +		dev_dbg(&edev->dev, "Speed: %d, %s duplex\n", priv->speed,
>  		       (priv->duplexity) ? "full" : "half");
>  
>  	} else {
> -		dev_info(&edev->dev, "No link.\n");
> +		dev_dbg(&edev->dev, "No link.\n");

Could you factor out the good and harmless changes out to a separate
patch? It makes it a bit more obvious what's needed to add Layerscape
support.

>  	}
>  }
>  
> @@ -173,28 +177,45 @@ static int gfar_init(struct eth_device *edev)
>  	return  0;
>  }
>  
> +static int gfar_receive_packets_setup(struct gfar_private *priv, int ix)
> +{
> +	dma_addr_t dma;
> +
> +	dma = dma_map_single(priv->dev, priv->rx_buffer[ix], PKTSIZE,
> +			DMA_FROM_DEVICE);
> +	if (dma_mapping_error(priv->dev, dma)) {
> +		return -EFAULT;
> +	}
> +
> +	out_be32(&priv->rxbd[ix].bufPtr, dma);
> +
> +	return 0;
> +}
> +
>  static int gfar_open(struct eth_device *edev)
>  {
> -	int ix;
>  	struct gfar_private *priv = edev->priv;
>  	struct gfar_phy *phy = priv->gfar_mdio;
>  	void __iomem *regs = priv->regs;
> -	int ret;
> +	int ix, ret;
>  
> -	ret = phy_device_connect(edev, &phy->miibus, priv->phyaddr,
> -				 gfar_adjust_link, 0, PHY_INTERFACE_MODE_NA);
> +	ret = phy_device_connect(edev, (phy?&phy->miibus:NULL), priv->phyaddr,
> +				 gfar_adjust_link, 0, priv->interface);

Spaces around operators please.

> +	priv->tbiaddr = tbiaddr;
> +	out_be32(priv->regs + GFAR_TBIPA_OFFSET, priv->tbiaddr);
> +
> +	size = ((TX_BUF_CNT * sizeof(struct txbd8)) +
> +		(RX_BUF_CNT * sizeof(struct rxbd8)));
> +	base = dma_alloc_coherent(DMA_DEVICE_BROKEN, size, DMA_ADDRESS_BROKEN);
> +	priv->txbd = (struct txbd8 __iomem *)base;
> +	priv->rxbd = (struct rxbd8 __iomem *)(base + (TX_BUF_CNT * sizeof(struct txbd8)));
> +	dma_set_mask(dev, DMA_BIT_MASK(32));
> +
> +	edev->priv = priv;
> +	edev->init = gfar_init;
> +	edev->open = gfar_open;
> +	edev->halt = gfar_halt;
> +	edev->send = gfar_send;
> +	edev->recv = gfar_recv;
> +	edev->get_ethaddr = gfar_get_ethaddr;
> +	edev->set_ethaddr = gfar_set_ethaddr;
> +	edev->parent = dev;
> +
> +	setbits_be32(priv->regs + GFAR_MACCFG1_OFFSET, GFAR_MACCFG1_SOFT_RESET);
> +	udelay(2);
> +	clrbits_be32(priv->regs + GFAR_MACCFG1_OFFSET, GFAR_MACCFG1_SOFT_RESET);
> +
> +	gfar_init_phy(edev);
> +
> +	return eth_register(edev);

This duplicates quite some code from the PPC gfar_probe(). Could you
refactor to share the common parts?

> +}
> +#else
>  /*
> + * PPC only as there is no device tree support.
>   * Initialize device structure. Returns success if
>   * initialization succeeded.
>   */
> @@ -469,8 +631,10 @@ static int gfar_probe(struct device *dev)
>  	priv = xzalloc(sizeof(struct gfar_private));
>  
>  	ret = net_alloc_packets(priv->rx_buffer, ARRAY_SIZE(priv->rx_buffer));
> -	if (ret)
> +	if (ret) {
> +		free(priv);
>  		return ret;
> +	}
>  
>  	edev = &priv->edev;
>  
> @@ -527,21 +691,49 @@ static int gfar_probe(struct device *dev)
>  
>  	return eth_register(edev);
>  }
> +#endif
> +
> +static struct tsec_data etsec2_data = {
> +	.mdio_regs_off = 0x520,
> +};
> +
> +static struct tsec_data gianfar_data = {
> +	.mdio_regs_off = 0x0,
> +};
> +
> +static const struct of_device_id gfar_ids[] = {
> +	{ .compatible = "fsl,etsec2", .data = &etsec2_data },
> +	{ .compatible = "gianfar", .data = &gianfar_data },
> +	{ /* sentinel */ }
> +};
>  
>  static struct driver gfar_eth_driver = {
>  	.name  = "gfar",
> +	.of_compatible = DRV_OF_COMPAT(gfar_ids),
>  	.probe = gfar_probe,
>  };
>  device_platform_driver(gfar_eth_driver);
>  
> +/* MII bus */
> +static struct fsl_pq_mdio_data mdio_gianfar_data = {
> +	.mdio_regs_off = 0x0,
> +};
> +
> +static __maybe_unused const struct of_device_id gfar_mdio_ids[] = {
> +	{ .compatible = "fsl,etsec2-mdio", .data = &mdio_gianfar_data },
> +	{ /* sentinel */ }
> +};
> +
>  static int gfar_phy_probe(struct device *dev)
>  {
>  	struct gfar_phy *phy;
> +	struct fsl_pq_mdio_data *data;
>  	int ret;
>  
> +	data = (struct fsl_pq_mdio_data *)device_get_match_data(dev);
>  	phy = xzalloc(sizeof(*phy));
>  	phy->dev = dev;
> -	phy->regs = dev_get_mem_region(dev, 0);
> +	phy->regs = dev_get_mem_region(dev, 0) + data->mdio_regs_off;
>  	if (IS_ERR(phy->regs))
>  		return PTR_ERR(phy->regs);
>  
> @@ -561,10 +753,14 @@ static int gfar_phy_probe(struct device *dev)
>  
>  static struct driver gfar_phy_driver = {
>  	.name  = "gfar-mdio",
> +#ifdef CONFIG_OFDEVICE
> +	.of_compatible = DRV_OF_COMPAT(gfar_mdio_ids),
> +#endif

No need to #ifdef this. DRV_OF_COMPAT expands to NULL when
CONFIG_OFDEVICE is disabled.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |