[PATCHv5 00/18] powerpc: convert p1010, p1020 and p2020 RDB boards to DM_ETH

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RE: [PATCHv5 02/18] configs: enable DM_MDIO for LS1021A-TWR and LS1021A-TSN

Zhiqiang Hou-2
Hi Vladimir,

Sorry, I missed your email!

> -----Original Message-----
> From: Vladimir Oltean <[hidden email]>
> Sent: 2020年7月16日 23:34
> To: Z.q. Hou <[hidden email]>
> Cc: [hidden email]; [hidden email]; Priyanka Jain
> <[hidden email]>; Vladimir Oltean <[hidden email]>
> Subject: Re: [PATCHv5 02/18] configs: enable DM_MDIO for LS1021A-TWR
> and LS1021A-TSN
>
> On Thu, Jul 16, 2020 at 06:09:09PM +0800, Zhiqiang Hou wrote:
> > From: Vladimir Oltean <[hidden email]>
> >
> > The tsec driver now requires DM_MDIO when DM_ETH is enabled. To
> avoid
> > build errors, enable DM_MDIO in these boards' configs before we
> > actually add DM_MDIO support to tsec.
> >
> > Signed-off-by: Vladimir Oltean <[hidden email]>
> > Signed-off-by: Hou Zhiqiang <[hidden email]>
> > ---
>
> The series doesn't build at this patch for ls1021a family, and is broken until
> 05/18 "net: tsec: convert to use DM_MDIO when DM_ETH enabled". The
> reason is that fsl_pq_mdio_init is defined in drivers/net/fsl_mdio.c only
> when CONFIG_DM_MDIO=n.
>
> arm-none-linux-gnueabihf-ld.bfd: drivers/net/built-in.o: in function
> `tsec_probe':
> drivers/net/tsec.c:870: undefined reference to `fsl_pq_mdio_init'
> make: *** [Makefile:1762: u-boot] Error 1
>
> The build is finally fixed in 05/18 when you delete fsl_pq_mdio_init() from
> tsec.c which is not really needed.
>
> I don't think it's a good idea to break the build in between patches, it
> destroys bisectability. I think you would need to add one more patch, right
> before this one, that adds a dummy definition for this function.
> You can then remove it in the "net: tsec: convert to use DM_MDIO when
> DM_ETH enabled".
>
> Like this:
>
> diff --git a/drivers/net/fsl_mdio.c b/drivers/net/fsl_mdio.c index
> 43040d4c3f89..4128b30f7412 100644
> --- a/drivers/net/fsl_mdio.c
> +++ b/drivers/net/fsl_mdio.c
> @@ -149,6 +149,12 @@ int fsl_pq_mdio_init(bd_t *bis, struct
> fsl_pq_mdio_info *info)
>   return mdio_register(bus);
>  }
>  #else /* CONFIG_DM_MDIO */
> +
> +int fsl_pq_mdio_init(bd_t *bis, struct fsl_pq_mdio_info *info) {
> + return 0;
> +}
> +
>  #if defined(CONFIG_PHYLIB)
>  static int tsec_mdio_read(struct udevice *dev, int addr, int devad, int reg)
> {
>
> This is really fixing an issue of commit 2932c5a802a9 ("net: tsec:
> fsl_mdio: add DM MDIO support"), maybe you could even add a Fixes: tag
> for that.

Instead of adding a fix patch, I'd like to move this patch to the tail of this series.

Thanks,
Zhiqiang

>
> > V5:
> >  - Pick from
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20200503185227.28731-
> 3-olteanv%40gmail.com%2F&amp;data=02%7C01%7CZhiqiang.Hou%40nxp.
> com%7Cd458470893954ef37a3308d8299da29b%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C637305104294481939&amp;sdata=4zfV%2F%
> 2FjH%2Bi%2FlyyPpCI9164ntFzYVIqOchTufKwtZe0g%3D&amp;reserved=0.
> >
> >  configs/ls1021atsn_qspi_defconfig                   | 1 +
> >  configs/ls1021atsn_sdcard_defconfig                 | 1 +
> >  configs/ls1021atwr_nor_SECURE_BOOT_defconfig        | 1 +
> >  configs/ls1021atwr_nor_defconfig                    | 1 +
> >  configs/ls1021atwr_nor_lpuart_defconfig             | 1 +
> >  configs/ls1021atwr_qspi_defconfig                   | 1 +
> >  configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig | 1 +
> >  configs/ls1021atwr_sdcard_ifc_defconfig             | 1 +
> >  configs/ls1021atwr_sdcard_qspi_defconfig            | 1 +
> >  9 files changed, 9 insertions(+)
> >
> > diff --git a/configs/ls1021atsn_qspi_defconfig
> > b/configs/ls1021atsn_qspi_defconfig
> > index a62e04e92a..9a659c5512 100644
> > --- a/configs/ls1021atsn_qspi_defconfig
> > +++ b/configs/ls1021atsn_qspi_defconfig
> > @@ -43,6 +43,7 @@ CONFIG_PHY_ATHEROS=y
> CONFIG_PHY_BROADCOM=y
> > CONFIG_PHY_FIXED=y  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_MII=y
> >  CONFIG_TSEC_ENET=y
> > diff --git a/configs/ls1021atsn_sdcard_defconfig
> > b/configs/ls1021atsn_sdcard_defconfig
> > index db4f0ab796..235265bd74 100644
> > --- a/configs/ls1021atsn_sdcard_defconfig
> > +++ b/configs/ls1021atsn_sdcard_defconfig
> > @@ -54,6 +54,7 @@ CONFIG_PHY_ATHEROS=y
> CONFIG_PHY_BROADCOM=y
> > CONFIG_PHY_FIXED=y  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_MII=y
> >  CONFIG_TSEC_ENET=y
> > diff --git a/configs/ls1021atwr_nor_SECURE_BOOT_defconfig
> > b/configs/ls1021atwr_nor_SECURE_BOOT_defconfig
> > index 0bf4aad102..8946bc1a70 100644
> > --- a/configs/ls1021atwr_nor_SECURE_BOOT_defconfig
> > +++ b/configs/ls1021atwr_nor_SECURE_BOOT_defconfig
> > @@ -45,6 +45,7 @@ CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> >  CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_ATHEROS=y
> >  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_nor_defconfig
> > b/configs/ls1021atwr_nor_defconfig
> > index 5507dbe3c1..12b39703d4 100644
> > --- a/configs/ls1021atwr_nor_defconfig
> > +++ b/configs/ls1021atwr_nor_defconfig
> > @@ -47,6 +47,7 @@ CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> >  CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_ATHEROS=y
> >  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_nor_lpuart_defconfig
> > b/configs/ls1021atwr_nor_lpuart_defconfig
> > index cdf81957a8..84b724930f 100644
> > --- a/configs/ls1021atwr_nor_lpuart_defconfig
> > +++ b/configs/ls1021atwr_nor_lpuart_defconfig
> > @@ -49,6 +49,7 @@ CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> >  CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_ATHEROS=y
> >  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_qspi_defconfig
> > b/configs/ls1021atwr_qspi_defconfig
> > index 4c82d66899..52a4c5106b 100644
> > --- a/configs/ls1021atwr_qspi_defconfig
> > +++ b/configs/ls1021atwr_qspi_defconfig
> > @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH_ATMEL=y
> CONFIG_SPI_FLASH_STMICRO=y
> > CONFIG_PHY_ATHEROS=y  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig
> > b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig
> > index 9039ccc8f2..f6db8850c4 100644
> > --- a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig
> > +++ b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig
> > @@ -60,6 +60,7 @@ CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> >  CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_ATHEROS=y
> >  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_sdcard_ifc_defconfig
> > b/configs/ls1021atwr_sdcard_ifc_defconfig
> > index 478d3df1b8..f011b9ef50 100644
> > --- a/configs/ls1021atwr_sdcard_ifc_defconfig
> > +++ b/configs/ls1021atwr_sdcard_ifc_defconfig
> > @@ -62,6 +62,7 @@ CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> >  CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_ATHEROS=y
> >  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > diff --git a/configs/ls1021atwr_sdcard_qspi_defconfig
> > b/configs/ls1021atwr_sdcard_qspi_defconfig
> > index de783a8d3d..524ea054aa 100644
> > --- a/configs/ls1021atwr_sdcard_qspi_defconfig
> > +++ b/configs/ls1021atwr_sdcard_qspi_defconfig
> > @@ -61,6 +61,7 @@ CONFIG_SPI_FLASH_ATMEL=y
> CONFIG_SPI_FLASH_STMICRO=y
> > CONFIG_PHY_ATHEROS=y  CONFIG_DM_ETH=y
> > +CONFIG_DM_MDIO=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > --
> > 2.25.1
> >
>
> Thanks!
> -Vladimir
12