Message ID | CAOMZO5B4gvNR=C6fYycnxR0Xnf8kPCS2A+gWa8oLttbmURRFrw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | U-Boot atheros PHY support and cubox ethernet | expand |
On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > Ah. So this is probably why the DT being right isn't helping then. If > > you want to blind-convert I'm happy to test, otherwise do you have a > > similar board conversion for me to look at? Thanks! > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > if (!bus) > return -EINVAL; > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > PHY_INTERFACE_MODE_RGMII_ID); > if (!phydev) { > ret = -EINVAL; > goto free_bus; > OK, tried that and not enough. Console says PHY autoneg completes, but DHCP still doesn't reply. > Here is a similar DM_ETH board conversion: > > commit 02ee7a4aa57b37d6003263b69b1852c4cda5975e > Author: Alifer Moraes <alifer.wsdm at gmail.com> > Date: Mon Feb 10 11:28:01 2020 -0300 > > mx6sabresd: Convert ethernet to driver model > > Convert imx6sabresd ethernet to driver model to fix the following warning: > > ===================== WARNING ====================== > This board does not use CONFIG_DM_ETH (Driver Model > for Ethernet drivers). Please update the board to use > CONFIG_DM_ETH before the v2020.07 release. Failure to > update by the deadline may result in board removal. > See doc/driver-model/migration.rst for more info. > ==================================================== > > Signed-off-by: Alifer Moraes <alifer.wsdm at gmail.com> > Reviewed-by: Fabio Estevam <festevam at gmail.com> > > Let us know if this helps. I'm looking at the conversion now, thanks.
On Tue, Jun 16, 2020 at 5:58 PM Tom Rini <trini at konsulko.com> wrote: > I'm looking at the conversion now, thanks. When doing the DM_ETH conversion, this kernel commit may also help as 'qca,clk-out-frequency' is also supported in U-Boot: commit 86b08bd5b99480b79a25343f24c1b8c4ddcb5c09 Author: Russell King <rmk+kernel at armlinux.org.uk> Date: Wed Apr 15 16:44:17 2020 +0100 ARM: dts: imx6-sr-som: add ethernet PHY configuration Add ethernet PHY configuration ahead of removing the quirk that configures the clocking mode for the PHY. The RGMII delay is already set correctly. Signed-off-by: Russell King <rmk+kernel at armlinux.org.uk> Reviewed-by: Fabio Estevam <festevam at gmail.com> Signed-off-by: Shawn Guo <shawnguo at kernel.org>
On Tue, 16 Jun 2020 at 23:58, Tom Rini <trini at konsulko.com> wrote: > > On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > > > Ah. So this is probably why the DT being right isn't helping then. If > > > you want to blind-convert I'm happy to test, otherwise do you have a > > > similar board conversion for me to look at? Thanks! > > > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > > if (!bus) > > return -EINVAL; > > > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > > PHY_INTERFACE_MODE_RGMII_ID); > > if (!phydev) { > > ret = -EINVAL; > > goto free_bus; > > > > OK, tried that and not enough. Console says PHY autoneg completes, but > DHCP still doesn't reply. > What about with the manual revert in place? What does phydev->interface print? Not only in atheros.c, but also in mx6cuboxi.c, right below this phy_find_by_mask call. Trying to understand if this is your only problem or if there are more.
On Wed, 17 Jun 2020 at 00:01, Vladimir Oltean <olteanv at gmail.com> wrote: > > On Tue, 16 Jun 2020 at 23:58, Tom Rini <trini at konsulko.com> wrote: > > > > On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > > > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > Ah. So this is probably why the DT being right isn't helping then. If > > > > you want to blind-convert I'm happy to test, otherwise do you have a > > > > similar board conversion for me to look at? Thanks! > > > > > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > > > > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > > > if (!bus) > > > return -EINVAL; > > > > > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > > > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > > > PHY_INTERFACE_MODE_RGMII_ID); > > > if (!phydev) { > > > ret = -EINVAL; > > > goto free_bus; > > > > > > > OK, tried that and not enough. Console says PHY autoneg completes, but > > DHCP still doesn't reply. > > > > What about with the manual revert in place? What does > phydev->interface print? Not only in atheros.c, but also in > mx6cuboxi.c, right below this phy_find_by_mask call. > Trying to understand if this is your only problem or if there are more. In fact there's one more 'manual' thing you can do. Check if RX delay is enabled (bit 15 of debug register 0): => mdio write eTSEC1 0x1D 0 => mdio read eTSEC1 0x1E => mdio write eTSEC1 0x1E <new value> Check if TX delay is enabled (bit 8 of debug register 5): => mdio write eTSEC1 0x1D 5 => mdio read eTSEC1 0x1E => mdio write eTSEC1 0x1E <new value> (replace eTSEC1 with your mdio bus name from "mdio list")
On Wed, Jun 17, 2020 at 12:01:39AM +0300, Vladimir Oltean wrote: > On Tue, 16 Jun 2020 at 23:58, Tom Rini <trini at konsulko.com> wrote: > > > > On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > > > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > Ah. So this is probably why the DT being right isn't helping then. If > > > > you want to blind-convert I'm happy to test, otherwise do you have a > > > > similar board conversion for me to look at? Thanks! > > > > > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > > > > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > > > if (!bus) > > > return -EINVAL; > > > > > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > > > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > > > PHY_INTERFACE_MODE_RGMII_ID); > > > if (!phydev) { > > > ret = -EINVAL; > > > goto free_bus; > > > > > > > OK, tried that and not enough. Console says PHY autoneg completes, but > > DHCP still doesn't reply. > > > > What about with the manual revert in place? What does > phydev->interface print? Not only in atheros.c, but also in > mx6cuboxi.c, right below this phy_find_by_mask call. > Trying to understand if this is your only problem or if there are more. In both functions the value printed out for phydev->interface is the value passed to phy_find_by_mask(...).
On Wed, Jun 17, 2020 at 12:04:16AM +0300, Vladimir Oltean wrote: > On Wed, 17 Jun 2020 at 00:01, Vladimir Oltean <olteanv at gmail.com> wrote: > > > > On Tue, 16 Jun 2020 at 23:58, Tom Rini <trini at konsulko.com> wrote: > > > > > > On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > > > > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > > > Ah. So this is probably why the DT being right isn't helping then. If > > > > > you want to blind-convert I'm happy to test, otherwise do you have a > > > > > similar board conversion for me to look at? Thanks! > > > > > > > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > > > > > > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > > > > if (!bus) > > > > return -EINVAL; > > > > > > > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > > > > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > > > > PHY_INTERFACE_MODE_RGMII_ID); > > > > if (!phydev) { > > > > ret = -EINVAL; > > > > goto free_bus; > > > > > > > > > > OK, tried that and not enough. Console says PHY autoneg completes, but > > > DHCP still doesn't reply. > > > > > > > What about with the manual revert in place? What does > > phydev->interface print? Not only in atheros.c, but also in > > mx6cuboxi.c, right below this phy_find_by_mask call. > > Trying to understand if this is your only problem or if there are more. > > In fact there's one more 'manual' thing you can do. For this test, based on your previous email and the kernel dts files, I made the change to mx6cuboxi.c to pass PHY_INTERFACE_MODE_RGMII_ID to phy_find_by_mask(...). > Check if RX delay is enabled (bit 15 of debug register 0): > => mdio write eTSEC1 0x1D 0 > => mdio read eTSEC1 0x1E > => mdio write eTSEC1 0x1E <new value> => mdio list FEC: 0 - AR8035 <--> FEC => mdio write FEC 0x1D 0 => mdio read FEC 0x1E Reading from bus FEC PHY at address 0: 30 - 0x82ee ... so it's enabled already. > Check if TX delay is enabled (bit 8 of debug register 5): > => mdio write eTSEC1 0x1D 5 > => mdio read eTSEC1 0x1E > => mdio write eTSEC1 0x1E <new value> => mdio write FEC 0x1D 5 => mdio read FEC 0x1E Reading from bus FEC PHY at address 0: 30 - 0x2d47 ... so it's enabled already. Or did I make a mistake? Thanks!
On Wed, 17 Jun 2020 at 00:57, Tom Rini <trini at konsulko.com> wrote: > > On Wed, Jun 17, 2020 at 12:04:16AM +0300, Vladimir Oltean wrote: > > On Wed, 17 Jun 2020 at 00:01, Vladimir Oltean <olteanv at gmail.com> wrote: > > > > > > On Tue, 16 Jun 2020 at 23:58, Tom Rini <trini at konsulko.com> wrote: > > > > > > > > On Tue, Jun 16, 2020 at 05:55:10PM -0300, Fabio Estevam wrote: > > > > > On Tue, Jun 16, 2020 at 5:51 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > > > > > Ah. So this is probably why the DT being right isn't helping then. If > > > > > > you want to blind-convert I'm happy to test, otherwise do you have a > > > > > > similar board conversion for me to look at? Thanks! > > > > > > > > > > Please try Vladimir's suggestion as it seems to be the less intrusive approach: > > > > > > > > > > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > > > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > > > > > @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) > > > > > if (!bus) > > > > > return -EINVAL; > > > > > > > > > > - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); > > > > > + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, > > > > > PHY_INTERFACE_MODE_RGMII_ID); > > > > > if (!phydev) { > > > > > ret = -EINVAL; > > > > > goto free_bus; > > > > > > > > > > > > > OK, tried that and not enough. Console says PHY autoneg completes, but > > > > DHCP still doesn't reply. > > > > > > > > > > What about with the manual revert in place? What does > > > phydev->interface print? Not only in atheros.c, but also in > > > mx6cuboxi.c, right below this phy_find_by_mask call. > > > Trying to understand if this is your only problem or if there are more. > > > > In fact there's one more 'manual' thing you can do. > > For this test, based on your previous email and the kernel dts files, I > made the change to mx6cuboxi.c to pass PHY_INTERFACE_MODE_RGMII_ID to > phy_find_by_mask(...). > > > Check if RX delay is enabled (bit 15 of debug register 0): > > => mdio write eTSEC1 0x1D 0 > > => mdio read eTSEC1 0x1E > > => mdio write eTSEC1 0x1E <new value> > > => mdio list > FEC: > 0 - AR8035 <--> FEC > => mdio write FEC 0x1D 0 > => mdio read FEC 0x1E > Reading from bus FEC > PHY at address 0: > 30 - 0x82ee > ... so it's enabled already. > > > Check if TX delay is enabled (bit 8 of debug register 5): > > => mdio write eTSEC1 0x1D 5 > > => mdio read eTSEC1 0x1E > > => mdio write eTSEC1 0x1E <new value> > > => mdio write FEC 0x1D 5 > => mdio read FEC 0x1E > Reading from bus FEC > PHY at address 0: > 30 - 0x2d47 > ... so it's enabled already. > > Or did I make a mistake? Thanks! > > -- > Tom Yes, the PHY has RGMII delays enabled in both directions.
On Tue, Jun 16, 2020 at 06:01:24PM -0300, Fabio Estevam wrote: > On Tue, Jun 16, 2020 at 5:58 PM Tom Rini <trini at konsulko.com> wrote: > > > I'm looking at the conversion now, thanks. > > When doing the DM_ETH conversion, this kernel commit may also help as > 'qca,clk-out-frequency' is also supported in U-Boot: > > commit 86b08bd5b99480b79a25343f24c1b8c4ddcb5c09 > Author: Russell King <rmk+kernel at armlinux.org.uk> > Date: Wed Apr 15 16:44:17 2020 +0100 > > ARM: dts: imx6-sr-som: add ethernet PHY configuration > > Add ethernet PHY configuration ahead of removing the quirk that > configures the clocking mode for the PHY. The RGMII delay is > already set correctly. > > Signed-off-by: Russell King <rmk+kernel at armlinux.org.uk> > Reviewed-by: Fabio Estevam <festevam at gmail.com> > Signed-off-by: Shawn Guo <shawnguo at kernel.org> Yeah, OK, I got far enough on trying to convert this that I see I don't quite have time to pick it up and run with it. It looks like there's going to be some extra fun around resetting the PHY perhaps? My quick pass got things seeing a generic PHY and not the atheros one. If you have time to make something you'd like me to test for conversion let me know. But I'm going to pick up debugging the problem I see on the platform right now in case there's a wider problem it's showing.
Hi Tom, On Tue, Jun 16, 2020 at 7:30 PM Tom Rini <trini at konsulko.com> wrote: > Yeah, OK, I got far enough on trying to convert this that I see I don't > quite have time to pick it up and run with it. It looks like there's > going to be some extra fun around resetting the PHY perhaps? My quick > pass got things seeing a generic PHY and not the atheros one. If you > have time to make something you'd like me to test for conversion let me > know. But I'm going to pick up debugging the problem I see on the > platform right now in case there's a wider problem it's showing. Here are two untested patches you could try. -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-imx6qdlsrsomupdate.patch Type: text/x-patch Size: 1653 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200617/bdfa34fa/attachment.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-cuboxdmethconversion.patch Type: text/x-patch Size: 5319 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200617/bdfa34fa/attachment-0001.bin>
On Wed, Jun 17, 2020 at 9:04 AM Fabio Estevam <festevam at gmail.com> wrote: > > Hi Tom, > > On Tue, Jun 16, 2020 at 7:30 PM Tom Rini <trini at konsulko.com> wrote: > > > Yeah, OK, I got far enough on trying to convert this that I see I don't > > quite have time to pick it up and run with it. It looks like there's > > going to be some extra fun around resetting the PHY perhaps? My quick > > pass got things seeing a generic PHY and not the atheros one. If you > > have time to make something you'd like me to test for conversion let me > > know. But I'm going to pick up debugging the problem I see on the > > platform right now in case there's a wider problem it's showing. > > Here are two untested patches you could try. Just tested Ethernet on a mx6qsabresd board and I see Ethernet is broken in master. It works fine in 2020.04. Reverting 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually configurable for AR8035") did not help. git bisect is running into problems: Error: Cannot open output file scripts/Makefile.spl:465: recipe for target 'spl/dts/imx6dl-sabresd.dtb' failed make[1]: *** [spl/dts/imx6dl-sabresd.dtb] Error 1 make[1]: *** Waiting for unfinished jobs.... UPD spl/./include/generated/asm-offsets.h UPD spl/./include/generated/generic-asm-offsets.h Makefile:1940: recipe for target 'spl/u-boot-spl' failed make: *** [spl/u-boot-spl] Error 2 So not able to run a bit bisect.
On Wed, Jun 17, 2020 at 10:06:25AM -0300, Fabio Estevam wrote: > On Wed, Jun 17, 2020 at 9:04 AM Fabio Estevam <festevam at gmail.com> wrote: > > > > Hi Tom, > > > > On Tue, Jun 16, 2020 at 7:30 PM Tom Rini <trini at konsulko.com> wrote: > > > > > Yeah, OK, I got far enough on trying to convert this that I see I don't > > > quite have time to pick it up and run with it. It looks like there's > > > going to be some extra fun around resetting the PHY perhaps? My quick > > > pass got things seeing a generic PHY and not the atheros one. If you > > > have time to make something you'd like me to test for conversion let me > > > know. But I'm going to pick up debugging the problem I see on the > > > platform right now in case there's a wider problem it's showing. > > > > Here are two untested patches you could try. > > Just tested Ethernet on a mx6qsabresd board and I see Ethernet is > broken in master. It works fine in 2020.04. > > Reverting 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually > configurable for AR8035") did not help. > > git bisect is running into problems: > > Error: Cannot open output file > scripts/Makefile.spl:465: recipe for target 'spl/dts/imx6dl-sabresd.dtb' failed > make[1]: *** [spl/dts/imx6dl-sabresd.dtb] Error 1 > make[1]: *** Waiting for unfinished jobs.... > UPD spl/./include/generated/asm-offsets.h > UPD spl/./include/generated/generic-asm-offsets.h > Makefile:1940: recipe for target 'spl/u-boot-spl' failed > make: *** [spl/u-boot-spl] Error 2 > > So not able to run a bit bisect. You probably need 5f09f9af3cc335fe6a74c031cfa0b1d8bdf4b9db applied when you see that failure, or a smaller make -j number.
On Wed, Jun 17, 2020 at 10:23 AM Tom Rini <trini at konsulko.com> wrote: > You probably need 5f09f9af3cc335fe6a74c031cfa0b1d8bdf4b9db applied when > you see that failure, or a smaller make -j number. Ok, I fixed the issue for mx6sabresd. It was dts problem. I will send the patch.
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -321,7 +321,7 @@ int board_eth_init(bd_t *bis) if (!bus) return -EINVAL; - phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII); + phydev = phy_find_by_mask(bus, ETH_PHY_MASK, PHY_INTERFACE_MODE_RGMII_ID); if (!phydev) { ret = -EINVAL;