Allow MACB to connect to a higher addresses PHY.

From: Anders Darander <ad@datarespons.se>

Using the Atmel MACB together with an integrated switch, can make only port 1
work. This is caused by macb_mii_probe trying to attach the MAC to the first
PHY, which often is on one of the external ports.

E.g. the Micrel KSZ8873 connects to the MAC on port 3, thus phy_addr should be
set to 3.

Signed-off-by: Anders Darander <ad@datarespons.se>
---
 drivers/net/phy/Kconfig      |   19 ++++++++++++++++++-
 drivers/net/phy/phy_device.c |    4 ++++
 2 files changed, 22 insertions(+), 1 deletions(-)

On Tue, Mar 30, 2010 at 3:58 AM, Anders Darander
<anders.darander@gmail.com> wrote:
> From: Anders Darander <ad@datarespons.se>
>
> Using the Atmel MACB together with an integrated switch, can make only port 1
> work. This is caused by macb_mii_probe trying to attach the MAC to the first
> PHY, which often is on one of the external ports.
>
> E.g. the Micrel KSZ8873 connects to the MAC on port 3, thus phy_addr should be
> set to 3.
>
> Signed-off-by: Anders Darander <ad@datarespons.se>

Hi Anders,

I understand what you are trying to do, but this is the wrong way to
go about it.  Hard coding it into Kconfig breaks multiplatform
kernels.  Besides, systems may have more than one physical MDIO bus.
This patch would make CONFIG_SWITCHING_PHY_ADDR the only address
accessible on all MDIO busses.

Nak.

The right thing to do is to add a runtime configuration option (ie.
kernel parameter or platform data) to the mac driver to specify
exactly which PHY address it is supposed to use.

g.

> ---
>  drivers/net/phy/Kconfig      |   19 ++++++++++++++++++-
>  drivers/net/phy/phy_device.c |    4 ++++
>  2 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index fc5938b..64aa003 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -15,11 +15,28 @@ if PHYLIB
>
>  comment "MII PHY device drivers"
>
> +config SWITCHING_PHY
> +       bool "An Ethernet switch is connected to the MAC"
> +       depends on MACB
> +       help
> +         If an Ethernet switch is connected to the MAC, it
> +         is quite common to have the connection on a port
> +         higher than the first port. This option allows to
> +         select the desired port number
> +
> +config SWITCHING_PHY_ADDR
> +       int "PHYA address"
> +       depends on SWITCHING_PHY
> +       default "3"
> +       help
> +         On e.g. the Micrel KSZ8873MLL, port 3 (and thus phy_addr
> +         3) is the one connected to the MAC of the MCU.
> +
>  config MARVELL_PHY
>        tristate "Drivers for Marvell PHYs"
>        ---help---
>          Currently has a driver for the 88E1011S
> -
> +
>  config DAVICOM_PHY
>        tristate "Drivers for Davicom PHYs"
>        ---help---
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index db17945..891bd0d 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -284,7 +284,11 @@ struct phy_device *phy_find_first(struct mii_bus *bus)
>  {
>        int addr;
>
> +#ifdef CONFIG_SWITCHING_PHY
> +       for (addr = CONFIG_SWITCHING_PHY_ADDR; addr < PHY_MAX_ADDR; addr++) {
> +#else
>        for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> +#endif
>                if (bus->phy_map[addr])
>                        return bus->phy_map[addr];
>        }
> --
> 1.7.0.3
>
>

Hi Grant,

Thanks for the feedback!

* Grant Likely <grant.likely@secretlab.ca> [100330 16:42]:
> On Tue, Mar 30, 2010 at 3:58 AM, Anders Darander
> <anders.darander@gmail.com> wrote:
> > Using the Atmel MACB together with an integrated switch, can make only port 1
> > work. This is caused by macb_mii_probe trying to attach the MAC to the first
> > PHY, which often is on one of the external ports.
> >
> > E.g. the Micrel KSZ8873 connects to the MAC on port 3, thus phy_addr should be
> > set to 3.

> I understand what you are trying to do, but this is the wrong way to
> go about it.  Hard coding it into Kconfig breaks multiplatform
> kernels.  Besides, systems may have more than one physical MDIO bus.
> This patch would make CONFIG_SWITCHING_PHY_ADDR the only address
> accessible on all MDIO busses.

True, it would at least make all addresses < CONFIG_SWITCHING_PHY_ADDR
non-accessible.

We developed the patch to solve this problem on a 2.6.29-kernel, in
which the loop in question is directly in the macb_mii_probe() in the
macb.c, thus, the original patch is at least a _little_ bit less ugly...

> Nak.

I fully agree on the nak; I completely forgot that the functions in
phy_devices should be used from more interfaces than only the macb.

> The right thing to do is to add a runtime configuration option (ie.
> kernel parameter or platform data) to the mac driver to specify
> exactly which PHY address it is supposed to use.

I'll see if I can get some time to do such a thing. Unfortunately, it'll
probably have to be dealt with on my sparetime (the customer is quite
unlikely to let me work on that).

One question, if I add such an kernel parameter, would it be OK to
implement something like this in the macb-driver:

if parameter == SOME_DEFAULT
	phydev = phy_find_first(bp->mii_bus);
else
	phydev = bp->mii_bus->phy_map[parameter];

I.e. if the parameter is set to some default value, the old behaviour is
retained, otherwise the specified parameter is used for the phy_addr?

I'd appreciate your input on such a design.

Best regards,
Anders Darander

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Mar 30, 2010 at 11:53 PM, Anders Darander
<anders.darander@datarespons.se> wrote:
> * Grant Likely <grant.likely@secretlab.ca> [100330 16:42]:
> One question, if I add such an kernel parameter, would it be OK to
> implement something like this in the macb-driver:
>
> if parameter == SOME_DEFAULT
>        phydev = phy_find_first(bp->mii_bus);
> else
>        phydev = bp->mii_bus->phy_map[parameter];
>
> I.e. if the parameter is set to some default value, the old behaviour is
> retained, otherwise the specified parameter is used for the phy_addr?

Absolutely.

g.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html