diff mbox series

U-Boot atheros PHY support and cubox ethernet

Message ID CA+h21hpbj-uovT43bncro4fr4-zQUmSm-koFHLpMU19atTVUxw@mail.gmail.com
State New
Headers show
Series U-Boot atheros PHY support and cubox ethernet | expand

Commit Message

Vladimir Oltean June 16, 2020, 8:37 p.m. UTC 
On Tue, 16 Jun 2020 at 23:31, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Jun 16, 2020 at 11:21:08PM +0300, Vladimir Oltean wrote:
> > On Tue, 16 Jun 2020 at 23:10, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Tue, Jun 16, 2020 at 10:58:26PM +0300, Vladimir Oltean wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, 16 Jun 2020 at 22:55, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > Hey all,
> > > > >
> > > > > In commit 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually
> > > > > configurable for AR8035") we brought in changes to get in to line with
> > > > > upstream linux kernel support for this PHY and in turn deal with more
> > > > > "driver was wrong, DT was wrong too" changes.  Now the problem I have is
> > > > > that ethernet on my Hummingboard doesn't work AND as far as I can tell,
> > > > > the DTB is correct and saying phy-mode = "rgmii-id" now not "rgmii".  It
> > > > > also looks to match what's in v5.7 for the kernel.  What do I need to do
> > > > > here next?  Thanks!
> > > > >
> > > > > --
> > > > > Tom
> > > >
> > > > Reverting 4346df3392c0 makes your interface work?
> > >
> > > Yup.
> > >
> > > > What is the DTS path that your hummingboard uses?
> > >
> > > Good question.  Per board/solidrun/mx6cuboxi/mx6cuboxi.c:
> > > /*
> > >  * This is not a perfect match. Avoid dependency on the DM GPIO driver
> > >  * needed
> > >  * for accurate board detection. Hummingboard2 DT is good enough for
> > >  * U-Boot on
> > >  * all Hummingboard/Cubox-i platforms.
> > >  */
> > > so arch/arm/dts/imx6dl-hummingboard2-emmc-som-v15.dts is the base.  But
> > > the board really is the original hummingboard platform.  Thanks!
> > >
> > > --
> > > Tom
> >
> > Does it work if you say PHY_INTERFACE_MODE_RGMII_ID here?
> > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1232
>
> No change.  And to be clear, I bisect'd the issue down and at that
> commit, reverting it fixes ethernet.  It doesn't revert cleanly (nor
> obviously to me) on master.
>
> --
> Tom

On master, the revert would look like this:

The next thing to check is to print the value of phydev->interface there.
7 is PHY_INTERFACE_MODE_RGMII,
8 is PHY_INTERFACE_MODE_RGMII_ID,
9 is PHY_INTERFACE_MODE_RGMII_RXID,
10 is PHY_INTERFACE_MODE_RGMII_TXID,

You want it to be 8.

Thanks,
-Vladimir

Comments

Tom Rini June 16, 2020, 9:35 p.m. UTC | #1 
On Tue, Jun 16, 2020 at 11:37:17PM +0300, Vladimir Oltean wrote:
> On Tue, 16 Jun 2020 at 23:31, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Jun 16, 2020 at 11:21:08PM +0300, Vladimir Oltean wrote:
> > > On Tue, 16 Jun 2020 at 23:10, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Tue, Jun 16, 2020 at 10:58:26PM +0300, Vladimir Oltean wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, 16 Jun 2020 at 22:55, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > Hey all,
> > > > > >
> > > > > > In commit 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually
> > > > > > configurable for AR8035") we brought in changes to get in to line with
> > > > > > upstream linux kernel support for this PHY and in turn deal with more
> > > > > > "driver was wrong, DT was wrong too" changes.  Now the problem I have is
> > > > > > that ethernet on my Hummingboard doesn't work AND as far as I can tell,
> > > > > > the DTB is correct and saying phy-mode = "rgmii-id" now not "rgmii".  It
> > > > > > also looks to match what's in v5.7 for the kernel.  What do I need to do
> > > > > > here next?  Thanks!
> > > > > >
> > > > > > --
> > > > > > Tom
> > > > >
> > > > > Reverting 4346df3392c0 makes your interface work?
> > > >
> > > > Yup.
> > > >
> > > > > What is the DTS path that your hummingboard uses?
> > > >
> > > > Good question.  Per board/solidrun/mx6cuboxi/mx6cuboxi.c:
> > > > /*
> > > >  * This is not a perfect match. Avoid dependency on the DM GPIO driver
> > > >  * needed
> > > >  * for accurate board detection. Hummingboard2 DT is good enough for
> > > >  * U-Boot on
> > > >  * all Hummingboard/Cubox-i platforms.
> > > >  */
> > > > so arch/arm/dts/imx6dl-hummingboard2-emmc-som-v15.dts is the base.  But
> > > > the board really is the original hummingboard platform.  Thanks!
> > > >
> > > > --
> > > > Tom
> > >
> > > Does it work if you say PHY_INTERFACE_MODE_RGMII_ID here?
> > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1232
> >
> > No change.  And to be clear, I bisect'd the issue down and at that
> > commit, reverting it fixes ethernet.  It doesn't revert cleanly (nor
> > obviously to me) on master.
> >
> > --
> > Tom
> 
> On master, the revert would look like this:
> 
> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> index 13f7275d1706..5ba639e8119f 100644
> --- a/drivers/net/phy/atheros.c
> +++ b/drivers/net/phy/atheros.c
> @@ -124,6 +124,7 @@ static int ar8021_config(struct phy_device *phydev)
> 
>  static int ar803x_delay_config(struct phy_device *phydev)
>  {
> +       int regval;
>         int ret;
> 
>         if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> @@ -138,6 +139,10 @@ static int ar803x_delay_config(struct phy_device *phydev)
>         else
>                 ret = ar803x_enable_rx_delay(phydev, false);
> 
> +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
> +       regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
> +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
> +
>         return ret;
>  }

This alone is not enough.

> The next thing to check is to print the value of phydev->interface there.
> 7 is PHY_INTERFACE_MODE_RGMII,
> 8 is PHY_INTERFACE_MODE_RGMII_ID,
> 9 is PHY_INTERFACE_MODE_RGMII_RXID,
> 10 is PHY_INTERFACE_MODE_RGMII_TXID,

With only this patch, it's 7.  With this added to mx6cuboxi.c:
-       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);
it's 8, and doesn't work either.  Moving on to the next emails now.
Vladimir Oltean June 16, 2020, 9:57 p.m. UTC | #2 
On Wed, 17 Jun 2020 at 00:35, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Jun 16, 2020 at 11:37:17PM +0300, Vladimir Oltean wrote:
> > On Tue, 16 Jun 2020 at 23:31, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Tue, Jun 16, 2020 at 11:21:08PM +0300, Vladimir Oltean wrote:
> > > > On Tue, 16 Jun 2020 at 23:10, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Tue, Jun 16, 2020 at 10:58:26PM +0300, Vladimir Oltean wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Tue, 16 Jun 2020 at 22:55, Tom Rini <trini at konsulko.com> wrote:
> > > > > > >
> > > > > > > Hey all,
> > > > > > >
> > > > > > > In commit 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually
> > > > > > > configurable for AR8035") we brought in changes to get in to line with
> > > > > > > upstream linux kernel support for this PHY and in turn deal with more
> > > > > > > "driver was wrong, DT was wrong too" changes.  Now the problem I have is
> > > > > > > that ethernet on my Hummingboard doesn't work AND as far as I can tell,
> > > > > > > the DTB is correct and saying phy-mode = "rgmii-id" now not "rgmii".  It
> > > > > > > also looks to match what's in v5.7 for the kernel.  What do I need to do
> > > > > > > here next?  Thanks!
> > > > > > >
> > > > > > > --
> > > > > > > Tom
> > > > > >
> > > > > > Reverting 4346df3392c0 makes your interface work?
> > > > >
> > > > > Yup.
> > > > >
> > > > > > What is the DTS path that your hummingboard uses?
> > > > >
> > > > > Good question.  Per board/solidrun/mx6cuboxi/mx6cuboxi.c:
> > > > > /*
> > > > >  * This is not a perfect match. Avoid dependency on the DM GPIO driver
> > > > >  * needed
> > > > >  * for accurate board detection. Hummingboard2 DT is good enough for
> > > > >  * U-Boot on
> > > > >  * all Hummingboard/Cubox-i platforms.
> > > > >  */
> > > > > so arch/arm/dts/imx6dl-hummingboard2-emmc-som-v15.dts is the base.  But
> > > > > the board really is the original hummingboard platform.  Thanks!
> > > > >
> > > > > --
> > > > > Tom
> > > >
> > > > Does it work if you say PHY_INTERFACE_MODE_RGMII_ID here?
> > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1232
> > >
> > > No change.  And to be clear, I bisect'd the issue down and at that
> > > commit, reverting it fixes ethernet.  It doesn't revert cleanly (nor
> > > obviously to me) on master.
> > >
> > > --
> > > Tom
> >
> > On master, the revert would look like this:
> >
> > diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> > index 13f7275d1706..5ba639e8119f 100644
> > --- a/drivers/net/phy/atheros.c
> > +++ b/drivers/net/phy/atheros.c
> > @@ -124,6 +124,7 @@ static int ar8021_config(struct phy_device *phydev)
> >
> >  static int ar803x_delay_config(struct phy_device *phydev)
> >  {
> > +       int regval;
> >         int ret;
> >
> >         if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> > @@ -138,6 +139,10 @@ static int ar803x_delay_config(struct phy_device *phydev)
> >         else
> >                 ret = ar803x_enable_rx_delay(phydev, false);
> >
> > +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
> > +       regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
> > +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
> > +
> >         return ret;
> >  }
>
> This alone is not enough.
>

So if this is not enough, then I'd go back to bisecting. I'd rebase
right after 8737c65fe4e3 ("phy: atheros: consolidate
{ar8031|ar8035}_config()"), create a new patch on
board/solidrun/mx6cuboxi/mx6cuboxi.c which changes RGMII to RGMII_ID,
confirm it works (according to what you've said so far, it should),
then bless it as "git bisect good", and let the fun start :)
Michael Walle June 17, 2020, 7:19 a.m. UTC | #3 
Am 2020-06-16 23:57, schrieb Vladimir Oltean:
> On Wed, 17 Jun 2020 at 00:35, Tom Rini <trini at konsulko.com> wrote:
>> 
>> On Tue, Jun 16, 2020 at 11:37:17PM +0300, Vladimir Oltean wrote:
>> > On Tue, 16 Jun 2020 at 23:31, Tom Rini <trini at konsulko.com> wrote:
>> > >
>> > > On Tue, Jun 16, 2020 at 11:21:08PM +0300, Vladimir Oltean wrote:
>> > > > On Tue, 16 Jun 2020 at 23:10, Tom Rini <trini at konsulko.com> wrote:
>> > > > >
>> > > > > On Tue, Jun 16, 2020 at 10:58:26PM +0300, Vladimir Oltean wrote:
>> > > > > > Hi Tom,
>> > > > > >
>> > > > > > On Tue, 16 Jun 2020 at 22:55, Tom Rini <trini at konsulko.com> wrote:
>> > > > > > >
>> > > > > > > Hey all,
>> > > > > > >
>> > > > > > > In commit 4346df3392c0 ("phy: atheros: Make RGMII Tx delays actually
>> > > > > > > configurable for AR8035") we brought in changes to get in to line with
>> > > > > > > upstream linux kernel support for this PHY and in turn deal with more
>> > > > > > > "driver was wrong, DT was wrong too" changes.  Now the problem I have is
>> > > > > > > that ethernet on my Hummingboard doesn't work AND as far as I can tell,
>> > > > > > > the DTB is correct and saying phy-mode = "rgmii-id" now not "rgmii".  It
>> > > > > > > also looks to match what's in v5.7 for the kernel.  What do I need to do
>> > > > > > > here next?  Thanks!
>> > > > > > >
>> > > > > > > --
>> > > > > > > Tom
>> > > > > >
>> > > > > > Reverting 4346df3392c0 makes your interface work?
>> > > > >
>> > > > > Yup.
>> > > > >
>> > > > > > What is the DTS path that your hummingboard uses?
>> > > > >
>> > > > > Good question.  Per board/solidrun/mx6cuboxi/mx6cuboxi.c:
>> > > > > /*
>> > > > >  * This is not a perfect match. Avoid dependency on the DM GPIO driver
>> > > > >  * needed
>> > > > >  * for accurate board detection. Hummingboard2 DT is good enough for
>> > > > >  * U-Boot on
>> > > > >  * all Hummingboard/Cubox-i platforms.
>> > > > >  */
>> > > > > so arch/arm/dts/imx6dl-hummingboard2-emmc-som-v15.dts is the base.  But
>> > > > > the board really is the original hummingboard platform.  Thanks!
>> > > > >
>> > > > > --
>> > > > > Tom
>> > > >
>> > > > Does it work if you say PHY_INTERFACE_MODE_RGMII_ID here?
>> > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1232
>> > >
>> > > No change.  And to be clear, I bisect'd the issue down and at that
>> > > commit, reverting it fixes ethernet.  It doesn't revert cleanly (nor
>> > > obviously to me) on master.
>> > >
>> > > --
>> > > Tom
>> >
>> > On master, the revert would look like this:
>> >
>> > diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
>> > index 13f7275d1706..5ba639e8119f 100644
>> > --- a/drivers/net/phy/atheros.c
>> > +++ b/drivers/net/phy/atheros.c
>> > @@ -124,6 +124,7 @@ static int ar8021_config(struct phy_device *phydev)
>> >
>> >  static int ar803x_delay_config(struct phy_device *phydev)
>> >  {
>> > +       int regval;
>> >         int ret;
>> >
>> >         if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
>> > @@ -138,6 +139,10 @@ static int ar803x_delay_config(struct phy_device *phydev)
>> >         else
>> >                 ret = ar803x_enable_rx_delay(phydev, false);
>> >
>> > +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
>> > +       regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
>> > +       phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
>> > +
>> >         return ret;
>> >  }
>> 
>> This alone is not enough.
>> 
> 
> So if this is not enough, then I'd go back to bisecting. I'd rebase
> right after 8737c65fe4e3 ("phy: atheros: consolidate
> {ar8031|ar8035}_config()"), create a new patch on
> board/solidrun/mx6cuboxi/mx6cuboxi.c which changes RGMII to RGMII_ID,
> confirm it works (according to what you've said so far, it should),
> then bless it as "git bisect good", and let the fun start :)

If these are the correct schematics, it looks like you also need the 
rgmii clock:
https://developer.solid-run.com/download/i-mx6-som-simplified-schematics-rev-1-5/

https://gitlab.denx.de/u-boot/u-boot/-/commit/6333cbb3817ed551cd7d4e92f7359c73ccc567fc

So I guess you'd need both, the clock output and the correct RGMII pad 
delay.

-michael
diff mbox series

Patch

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 13f7275d1706..5ba639e8119f 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -124,6 +124,7 @@  static int ar8021_config(struct phy_device *phydev)

 static int ar803x_delay_config(struct phy_device *phydev)
 {
+       int regval;
        int ret;

        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
@@ -138,6 +139,10 @@  static int ar803x_delay_config(struct phy_device *phydev)
        else
                ret = ar803x_enable_rx_delay(phydev, false);

+       phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
+       regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
+       phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
+
        return ret;
 }