Hi Vladimir,
Sorry, I missed your email!
> -----Original Message-----
> From: Vladimir Oltean <olte...@gmail.com>
> Sent: 2020年7月16日 23:34
> To: Z.q. Hou <zhiqiang....@nxp.com>
> Cc: u-boot@lists.denx.de; bmeng...@gmail.com; Priyanka Jain
> <priyanka.j...@nxp.com>; Vladimir Oltean <vladimir.olt...@nxp.com>
> 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 <vladimir.olt...@nxp.com>
> >
> > 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 <vladimir.olt...@nxp.com>
> > Signed-off-by: Hou Zhiqiang <zhiqiang....@nxp.com>
> > ---
>
> 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&data=02%7C01%7CZhiqiang.Hou%40nxp.
> com%7Cd458470893954ef37a3308d8299da29b%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C637305104294481939&sdata=4zfV%2F%
> 2FjH%2Bi%2FlyyPpCI9164ntFzYVIqOchTufKwtZe0g%3D&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
