On 10/15/07, Domen Puncer <[EMAIL PROTECTED]> wrote:
> On 14/10/07 16:05 -0600, Grant Likely wrote:
> > On 10/14/07, Domen Puncer <[EMAIL PROTECTED]> wrote:
> > > PHY part of the driver for mpc5200(b) ethernet.
> >
> > Assuming I understand correctly, this comment is not correct and this
> > patch just adds an MDIO bus driver. PHY drivers are in phylib and
> > data transfer is setup via the core driver, correct?
>
> Right.
>
> >
> > It is conceivable that the PHY is connected to an alternate MDIO bus,
> > or the MDIO bus is used for a PHY connected to an external Ethernet
> > controller.
> >
> > Speaking of which, is it possible to use this MDIO bus without the
> > core FEC being initialized?
>
> IIRC fec doesn't need any initialization for MDIO bus registers to work.
>
> >
> > > +static struct of_device_id fec_mdio_match[] = {
> > > + {
> > > + .type = "mdio",
> > > + .compatible = "mpc5200b-fec-phy",
> >
> > This is not a phy; it's an MDIO bus. Also, shouldn't this be
> > "mpc5200-..." instead of "mpc5200b-..."?
>
> Didn't know if it's ok for mpc5200, guess it is?
I believe it is.
>
> >
> > > + },
> > > + {},
> > > +};
> > > +
> > > +struct of_platform_driver mpc52xx_fec_mdio_driver = {
> > > + .name = "mpc5200b-fec-phy",
> > > + .probe = fec_mdio_probe,
> > > + .remove = fec_mdio_remove,
> > > + .match_table = fec_mdio_match,
> >
> > Inconsistent naming. Please use the same prefix on all global/static
> > symbols (ie. use "mpc52xx_mdio_" instead of the mix of
> > "mpc52xx_fec_mdio_", "fec_mdio_", etc.) I also thing that "fec_mdio_"
> > is too generic because there are a number of different incompatible
> > FEC devices.
>
> OK.
>
> >
> > > +};
> > > +
> > > +/* let fec driver call it, since this has to be registered before it */
> > > +EXPORT_SYMBOL_GPL(mpc52xx_fec_mdio_driver);
> >
> > Why not have a module_init()/module_exit() in this file? I don't
> > think the FEC driver calls this driver's functions directly anymore,
> > and it's still dependent on the of_platform bus probe order anyway.
>
> It was one way of making sure mdio driver is registered before fec.
> (and of_platform bus probe order won't work for modules)
> Nicer alternatives?
However, that assumption only works when the PHY is accessed via the
on-chip MDIO controller. What happens with a different MDIO bus is
used?
...
Need to take a look at how phylib handles the probing order problem;
but really this is something that PHYLIB needs to handle; not the FEC
driver.
> > > --- linux.git.orig/drivers/net/fec_mpc52xx/Kconfig
> > > +++ linux.git/drivers/net/fec_mpc52xx/Kconfig
> > > @@ -11,5 +11,18 @@ config FEC_MPC52xx
> > > ---help---
> > > This option enables support for the MPC5200's on-chip
> > > Fast Ethernet Controller
> > > + If compiled as module, it will be called 'fec_mpc52xx.ko'.
> >
> > Drop this line and make the help text the same format as the other eth
> > drivers in drivers/net.
>
> How exactly is it different now?
> And most of them have the "Module will be called xxx" line.
Okay, you're right. There is a real hodgepodge of help text format in
drivers/net/Kconfig. Do what seems best to you.
>
> >
> > > +
> > > +config FEC_MPC52xx_MDIO
> > > + bool "FEC MII PHY driver"
> > > + depends on FEC_MPC52xx
> > > + default y
> > > + ---help---
> > > + The MPC5200's FEC can connect to the Ethernet either with
> > > + an external MII PHY chip or 10 Mbps 7-wire interface
> > > + (Motorola? industry standard).
> > > + If your board uses an external PHY, enable this.
> >
> > Not strictly true. This enables talking to a PHY using the internal
> > MDIO controller. PHY register access could just as easily be accessed
> > via an alternate interface.
>
> Not just that is also selects which mode will it use.
> If you don't enable this, fec will be set up as 7-wire
> ( 696 rcntrl |= FEC_RCNTRL_MII_MODE;)
Looking at fec.c:
That line is dependent on priv->has_phy. priv->has_phy is set on line
949 which is conditional on the node having the property "phy-handle".
It doesn't look like it is conditional on CONFIG_FEC_MPC52xx_MDIO at
all.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
