[RESEND PATCH v2 3/5] net: dwc_eth_qos: Adapt probe() for PCI devices

[Philip Oberfichtner]

Hi Marek,

Thank you for the review!

On Sun, Jun 30, 2024 at 07:33:33AM +0200, Marek Vasut wrote:
> On 6/24/24 10:34 AM, Philip Oberfichtner wrote:
> > PCI devices do not necessarily use a device tree. In that case, the
> > driver currently fails to find eqos->config and eqos->regs.
> > 
> > This commit factors out the respective functionality. Device tree usage
> > remains default, but board specific implementations will be possible as
> > well.
> > 
> > Signed-off-by: Philip Oberfichtner <pro at denx.de>
> > ---
> >   drivers/net/dwc_eth_qos.c          | 28 +++++++++++++++++++++++++---
> >   drivers/net/dwc_eth_qos.h          |  3 +++
> >   drivers/net/dwc_eth_qos_imx.c      |  1 +
> >   drivers/net/dwc_eth_qos_qcom.c     |  1 +
> >   drivers/net/dwc_eth_qos_rockchip.c |  1 +
> >   drivers/net/dwc_eth_qos_starfive.c |  1 +
> >   drivers/net/dwc_eth_qos_stm32.c    |  1 +
> >   7 files changed, 33 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> > index b87634f1f5..34e29301cc 100644
> > --- a/drivers/net/dwc_eth_qos.c
> > +++ b/drivers/net/dwc_eth_qos.c
> > @@ -1376,6 +1376,21 @@ static int eqos_remove_resources_tegra186(struct udevice *dev)
> >   	return 0;
> >   }
> > +/*
> > + * Get driver data based on the device tree. Boards not using a device tree can
> > + * overwrite this function.
> > + */
> > +__weak void *eqos_get_driver_data(struct udevice *dev)
> > +{
> > +	return (void *)dev_get_driver_data(dev);
> > +}
> > +
> > +/* Function to get base address, useable for all boards having a device tree. */
> > +fdt_addr_t eqos_get_base_addr_dt(struct udevice *dev)
> > +{
> > +	return dev_read_addr(dev);
> > +}
> > +
> >   static int eqos_probe(struct udevice *dev)
> >   {
> >   	struct eqos_priv *eqos = dev_get_priv(dev);
> > @@ -1384,13 +1399,19 @@ static int eqos_probe(struct udevice *dev)
> >   	debug("%s(dev=%p):\n", __func__, dev);
> >   	eqos->dev = dev;
> > -	eqos->config = (void *)dev_get_driver_data(dev);
> > -	eqos->regs = dev_read_addr(dev);
> > +	eqos->config = eqos_get_driver_data(dev);
> > +	if (!eqos->config) {
> > +		pr_err("Failed to get driver data.\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	eqos->regs = eqos->config->ops->eqos_get_base_addr(dev);
> >   	if (eqos->regs == FDT_ADDR_T_NONE) {
> > -		pr_err("dev_read_addr() failed\n");
> > +		pr_err("Failed to get device address.\n");
> >   		return -ENODEV;
> >   	}
> > +
> >   	eqos->mac_regs = (void *)(eqos->regs + EQOS_MAC_REGS_BASE);
> >   	eqos->mtl_regs = (void *)(eqos->regs + EQOS_MTL_REGS_BASE);
> >   	eqos->dma_regs = (void *)(eqos->regs + EQOS_DMA_REGS_BASE);
> 
> Could you factor out the entirety of:
> 
>          eqos->regs = dev_read_addr(dev);
>          if (eqos->regs == FDT_ADDR_T_NONE) {
>                  pr_err("dev_read_addr() failed\n");
>                  return -ENODEV;
>          }
>          eqos->mac_regs = (void *)(eqos->regs + EQOS_MAC_REGS_BASE);
>          eqos->mtl_regs = (void *)(eqos->regs + EQOS_MTL_REGS_BASE);
>          eqos->dma_regs = (void *)(eqos->regs + EQOS_DMA_REGS_BASE);
>          eqos->tegra186_regs = (void *)(eqos->regs +
> EQOS_TEGRA186_REGS_BASE);
> 
> into some common function, like eqos_get_base_addr_dt() , and call it from
> .eqos_probe_resources callback instead ? That way you wouldn't have to add
> new callback specifically for register address.

Good point. I'll include it in V3.

Best regards,
Philip


-- 
=====================================================================
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-22  Fax: +49-8142-66989-80   Email: pro at denx.de
=====================================================================