diff mbox series

[U-Boot] cmd: mdio: Fix access to arbitrary PHY addresses

Message ID 20190530000832.23190-1-olteanv@gmail.com
State Accepted
Commit b4c20f20adad8d246b95be5bebacb730462c8c01
Delegated to: Joe Hershberger
Headers show
Series [U-Boot] cmd: mdio: Fix access to arbitrary PHY addresses | expand

Commit Message

Vladimir Oltean May 30, 2019, 12:08 a.m. UTC
Alex reported the following:

  "
  I'm doing some MDIO work on a freescale/NXP platform and I bumped into
  errors with this command:
  => mdio r emdio#3 5 3
  Reading from bus emdio#3
  "Synchronous Abort" handler, esr 0x8600000e
  elr: ffffffff862b8000 lr : 000000008200cce4 (reloc)
  ...

  mdio list does not list any PHYs currently because ethernet is using DM
  and the interfaces are not probed at this time.  The PHY does exist
  on the bus though.
  The above scenario works with this commit reverted:
  e55047ec51a662c12ed53ff543ec7cdf158b2137 cmd: mdio: Switch to generic
  helpers when accessing the registers

  The current code using generic helpers only works for PHYs that have
  been registered and show up in bus->phymap and crashes for arbitrary
  IDs.  I find it useful to allow reading from other addresses over MDIO
  too, certainly helpful for people debugging MDIO on various boards.
  "

Fix this by reverting to use the raw MDIO bus operations in case there
is no PHY probed based on DT at the specified address.

This restores the old behavior for these PHYs, which means that the
newly introduced MMD-over-C22 helpers won't be available for them, but
at least they will be accessible again without crashing the system.

Fixes: e55047ec51a6 ("cmd: mdio: Switch to generic helpers when accessing the registers")
Reported-by: Alex Marginean <alexm.osslist@gmail.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 cmd/mdio.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Alexandru Marginean May 30, 2019, 8:45 a.m. UTC | #1
On 5/30/2019 3:08 AM, Vladimir Oltean wrote:
> Alex reported the following:
> 
>    "
>    I'm doing some MDIO work on a freescale/NXP platform and I bumped into
>    errors with this command:
>    => mdio r emdio#3 5 3
>    Reading from bus emdio#3
>    "Synchronous Abort" handler, esr 0x8600000e
>    elr: ffffffff862b8000 lr : 000000008200cce4 (reloc)
>    ...
> 
>    mdio list does not list any PHYs currently because ethernet is using DM
>    and the interfaces are not probed at this time.  The PHY does exist
>    on the bus though.
>    The above scenario works with this commit reverted:
>    e55047ec51a662c12ed53ff543ec7cdf158b2137 cmd: mdio: Switch to generic
>    helpers when accessing the registers
> 
>    The current code using generic helpers only works for PHYs that have
>    been registered and show up in bus->phymap and crashes for arbitrary
>    IDs.  I find it useful to allow reading from other addresses over MDIO
>    too, certainly helpful for people debugging MDIO on various boards.
>    "
> 
> Fix this by reverting to use the raw MDIO bus operations in case there
> is no PHY probed based on DT at the specified address.
> 
> This restores the old behavior for these PHYs, which means that the
> newly introduced MMD-over-C22 helpers won't be available for them, but
> at least they will be accessible again without crashing the system.
> 
> Fixes: e55047ec51a6 ("cmd: mdio: Switch to generic helpers when accessing the registers")
> Reported-by: Alex Marginean <alexm.osslist@gmail.com>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> ---
>   cmd/mdio.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/cmd/mdio.c b/cmd/mdio.c
> index efe8c9ef0954..5e219f699d8d 100644
> --- a/cmd/mdio.c
> +++ b/cmd/mdio.c
> @@ -54,7 +54,10 @@ static int mdio_write_ranges(struct mii_dev *bus,
>   
>   		for (devad = devadlo; devad <= devadhi; devad++) {
>   			for (reg = reglo; reg <= reghi; reg++) {
> -				if (!extended)
> +				if (!phydev)
> +					err = bus->write(bus, addr, devad,
> +							 reg, data);
> +				else if (!extended)
>   					err = phy_write_mmd(phydev, devad,
>   							    reg, data);
>   				else
> @@ -88,7 +91,9 @@ static int mdio_read_ranges(struct mii_dev *bus,
>   			for (reg = reglo; reg <= reghi; reg++) {
>   				int val;
>   
> -				if (!extended)
> +				if (!phydev)
> +					val = bus->read(bus, addr, devad, reg);
> +				else if (!extended)
>   					val = phy_read_mmd(phydev, devad, reg);
>   				else
>   					val = phydev->drv->readext(phydev, addr,
> 

Reviewed-by: Alex Marginean <alexm.osslist@gmail.com>

Thanks!
Alex
Joe Hershberger June 1, 2019, 11:28 a.m. UTC | #2
On Wed, May 29, 2019 at 8:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Alex reported the following:
>
>   "
>   I'm doing some MDIO work on a freescale/NXP platform and I bumped into
>   errors with this command:
>   => mdio r emdio#3 5 3
>   Reading from bus emdio#3
>   "Synchronous Abort" handler, esr 0x8600000e
>   elr: ffffffff862b8000 lr : 000000008200cce4 (reloc)
>   ...
>
>   mdio list does not list any PHYs currently because ethernet is using DM
>   and the interfaces are not probed at this time.  The PHY does exist
>   on the bus though.
>   The above scenario works with this commit reverted:
>   e55047ec51a662c12ed53ff543ec7cdf158b2137 cmd: mdio: Switch to generic
>   helpers when accessing the registers
>
>   The current code using generic helpers only works for PHYs that have
>   been registered and show up in bus->phymap and crashes for arbitrary
>   IDs.  I find it useful to allow reading from other addresses over MDIO
>   too, certainly helpful for people debugging MDIO on various boards.
>   "
>
> Fix this by reverting to use the raw MDIO bus operations in case there
> is no PHY probed based on DT at the specified address.
>
> This restores the old behavior for these PHYs, which means that the
> newly introduced MMD-over-C22 helpers won't be available for them, but
> at least they will be accessible again without crashing the system.
>
> Fixes: e55047ec51a6 ("cmd: mdio: Switch to generic helpers when accessing the registers")
> Reported-by: Alex Marginean <alexm.osslist@gmail.com>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Joe Hershberger June 1, 2019, 11:13 p.m. UTC | #3
Hi Vladimir,

https://patchwork.ozlabs.org/patch/1107478/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe
diff mbox series

Patch

diff --git a/cmd/mdio.c b/cmd/mdio.c
index efe8c9ef0954..5e219f699d8d 100644
--- a/cmd/mdio.c
+++ b/cmd/mdio.c
@@ -54,7 +54,10 @@  static int mdio_write_ranges(struct mii_dev *bus,
 
 		for (devad = devadlo; devad <= devadhi; devad++) {
 			for (reg = reglo; reg <= reghi; reg++) {
-				if (!extended)
+				if (!phydev)
+					err = bus->write(bus, addr, devad,
+							 reg, data);
+				else if (!extended)
 					err = phy_write_mmd(phydev, devad,
 							    reg, data);
 				else
@@ -88,7 +91,9 @@  static int mdio_read_ranges(struct mii_dev *bus,
 			for (reg = reglo; reg <= reghi; reg++) {
 				int val;
 
-				if (!extended)
+				if (!phydev)
+					val = bus->read(bus, addr, devad, reg);
+				else if (!extended)
 					val = phy_read_mmd(phydev, devad, reg);
 				else
 					val = phydev->drv->readext(phydev, addr,