[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[Martin Blumenstingl]

mdio_mux_init parses the child nodes of the MDIO mux. When using
"mdio-mux-mmioreg" the child nodes are describing the register value
that is written to switch between the MDIO busses.

The change which makes the error messages more verbose changed the
parsing of the "reg" property from a simple of_property_read_u32 call
to of_mdio_parse_addr. On a Khadas VIM (based on the Meson GXL SoC,
which uses mdio-mux-mmioreg) this prevents registering the MDIO mux
(because the "reg" values on the MDIO mux child nodes are 0x2009087f
and 0xe40908ff) and leads to the following errors:
  mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff PHY address -469169921 is too large
  mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff
  mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f PHY address 537462911 is too large
  mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f
  mdio-mux-mmioreg c883455c.eth-phy-mux: Error: No acceptable child buses found
  mdio-mux-mmioreg c883455c.eth-phy-mux: failed to register mdio-mux bus /soc/periphs at c8834000/eth-phy-mux
(as a result of that ethernet is not working, because the PHY which is
connected through the mux' child MDIO bus, which is not being
registered).

Fix this by reverting the change from of_mdio_parse_addr to
of_mdio_parse_addr.

Fixes: 342fa1964439 ("mdio: mux: make child bus walking more permissive and errors more verbose")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/phy/mdio-mux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 00755b6a42cf..c608e1dfaf09 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -135,8 +135,8 @@ int mdio_mux_init(struct device *dev,
 	for_each_available_child_of_node(dev->of_node, child_bus_node) {
 		int v;
 
-		v = of_mdio_parse_addr(dev, child_bus_node);
-		if (v < 0) {
+		r = of_property_read_u32(child_bus_node, "reg", &v);
+		if (r) {
 			dev_err(dev,
 				"Error: Failed to find reg for child %s\n",
 				of_node_full_name(child_bus_node));
-- 
2.13.2

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[Neil Armstrong]

On 07/10/2017 02:35 PM, Martin Blumenstingl wrote:
> mdio_mux_init parses the child nodes of the MDIO mux. When using
> "mdio-mux-mmioreg" the child nodes are describing the register value
> that is written to switch between the MDIO busses.
> 
> The change which makes the error messages more verbose changed the
> parsing of the "reg" property from a simple of_property_read_u32 call
> to of_mdio_parse_addr. On a Khadas VIM (based on the Meson GXL SoC,
> which uses mdio-mux-mmioreg) this prevents registering the MDIO mux
> (because the "reg" values on the MDIO mux child nodes are 0x2009087f
> and 0xe40908ff) and leads to the following errors:
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff PHY address -469169921 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f PHY address 537462911 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: No acceptable child buses found
>   mdio-mux-mmioreg c883455c.eth-phy-mux: failed to register mdio-mux bus /soc/periphs at c8834000/eth-phy-mux
> (as a result of that ethernet is not working, because the PHY which is
> connected through the mux' child MDIO bus, which is not being
> registered).
> 
> Fix this by reverting the change from of_mdio_parse_addr to
> of_mdio_parse_addr.
> 
> Fixes: 342fa1964439 ("mdio: mux: make child bus walking more permissive and errors more verbose")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/net/phy/mdio-mux.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
> index 00755b6a42cf..c608e1dfaf09 100644
> --- a/drivers/net/phy/mdio-mux.c
> +++ b/drivers/net/phy/mdio-mux.c
> @@ -135,8 +135,8 @@ int mdio_mux_init(struct device *dev,
>  	for_each_available_child_of_node(dev->of_node, child_bus_node) {
>  		int v;
>  
> -		v = of_mdio_parse_addr(dev, child_bus_node);
> -		if (v < 0) {
> +		r = of_property_read_u32(child_bus_node, "reg", &v);
> +		if (r) {
>  			dev_err(dev,
>  				"Error: Failed to find reg for child %s\n",
>  				of_node_full_name(child_bus_node));
> 

I was going to push this, thanks martin !!

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[Andrew Lunn]

On Mon, Jul 10, 2017 at 02:35:23PM +0200, Martin Blumenstingl wrote:
> mdio_mux_init parses the child nodes of the MDIO mux. When using
> "mdio-mux-mmioreg" the child nodes are describing the register value
> that is written to switch between the MDIO busses.
> 
> The change which makes the error messages more verbose changed the
> parsing of the "reg" property from a simple of_property_read_u32 call
> to of_mdio_parse_addr. On a Khadas VIM (based on the Meson GXL SoC,
> which uses mdio-mux-mmioreg) this prevents registering the MDIO mux
> (because the "reg" values on the MDIO mux child nodes are 0x2009087f
> and 0xe40908ff) and leads to the following errors:
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff PHY address -469169921 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f PHY address 537462911 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: No acceptable child buses found
>   mdio-mux-mmioreg c883455c.eth-phy-mux: failed to register mdio-mux bus /soc/periphs at c8834000/eth-phy-mux
> (as a result of that ethernet is not working, because the PHY which is
> connected through the mux' child MDIO bus, which is not being
> registered).
> 
> Fix this by reverting the change from of_mdio_parse_addr to
> of_mdio_parse_addr.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Yes, validating the reg property needs to be done separately in each
user of the generic mdio-mix code. The reg for the gpio mux must be <=
number of gpios, mmioreg must be somewhere within the address space,
bcm-iproc < 1024?

Jon, please feel free to add such code.

    Andrew

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[Jon Mason]

On Mon, Jul 10, 2017 at 8:56 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Jul 10, 2017 at 02:35:23PM +0200, Martin Blumenstingl wrote:
>> mdio_mux_init parses the child nodes of the MDIO mux. When using
>> "mdio-mux-mmioreg" the child nodes are describing the register value
>> that is written to switch between the MDIO busses.
>>
>> The change which makes the error messages more verbose changed the
>> parsing of the "reg" property from a simple of_property_read_u32 call
>> to of_mdio_parse_addr. On a Khadas VIM (based on the Meson GXL SoC,
>> which uses mdio-mux-mmioreg) this prevents registering the MDIO mux
>> (because the "reg" values on the MDIO mux child nodes are 0x2009087f
>> and 0xe40908ff) and leads to the following errors:
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff PHY address -469169921 is too large
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f PHY address 537462911 is too large
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: No acceptable child buses found
>>   mdio-mux-mmioreg c883455c.eth-phy-mux: failed to register mdio-mux bus /soc/periphs at c8834000/eth-phy-mux
>> (as a result of that ethernet is not working, because the PHY which is
>> connected through the mux' child MDIO bus, which is not being
>> registered).
>>
>> Fix this by reverting the change from of_mdio_parse_addr to
>> of_mdio_parse_addr.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Yes, validating the reg property needs to be done separately in each
> user of the generic mdio-mix code. The reg for the gpio mux must be <=
> number of gpios, mmioreg must be somewhere within the address space,
> bcm-iproc < 1024?
>
> Jon, please feel free to add such code.

To be clear, are you suggesting that we add an additional property to
of_mdio_parse_addr() that specifies the limit to check against, or
remove the check and add it to each additional caller?

Thanks,
Jon

>
>     Andrew

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[Andrew Lunn]

> To be clear, are you suggesting that we add an additional property to
> of_mdio_parse_addr() that specifies the limit to check against, or
> remove the check and add it to each additional caller?

Hi Jon

Probably the simplest is to add an extra parameter to mdio_mux_init()
which is the maximum allowed reg value.

We should not touch of_mdio_parse_addr(). reg is not an mdio
address. It is a count of gpios, or a value to be poked into an
register.

      Andrew

[PATCH net] mdio: mux: fix parsing mux registers outside of the PHY address range

[David Miller]

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Mon, 10 Jul 2017 14:35:23 +0200

> mdio_mux_init parses the child nodes of the MDIO mux. When using
> "mdio-mux-mmioreg" the child nodes are describing the register value
> that is written to switch between the MDIO busses.
> 
> The change which makes the error messages more verbose changed the
> parsing of the "reg" property from a simple of_property_read_u32 call
> to of_mdio_parse_addr. On a Khadas VIM (based on the Meson GXL SoC,
> which uses mdio-mux-mmioreg) this prevents registering the MDIO mux
> (because the "reg" values on the MDIO mux child nodes are 0x2009087f
> and 0xe40908ff) and leads to the following errors:
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff PHY address -469169921 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at e40908ff
>   mdio-mux-mmioreg c883455c.eth-phy-mux: /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f PHY address 537462911 is too large
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: Failed to find reg for child /soc/periphs at c8834000/eth-phy-mux/mdio at 2009087f
>   mdio-mux-mmioreg c883455c.eth-phy-mux: Error: No acceptable child buses found
>   mdio-mux-mmioreg c883455c.eth-phy-mux: failed to register mdio-mux bus /soc/periphs at c8834000/eth-phy-mux
> (as a result of that ethernet is not working, because the PHY which is
> connected through the mux' child MDIO bus, which is not being
> registered).
> 
> Fix this by reverting the change from of_mdio_parse_addr to
> of_mdio_parse_addr.
> 
> Fixes: 342fa1964439 ("mdio: mux: make child bus walking more permissive and errors more verbose")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Applied, thanks.