From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linuxppc-dev@ozlabs.org, jgarzik@pobox.com,
afleming@freescale.com, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure
Date: Tue, 10 Mar 2009 23:29:49 +0300 [thread overview]
Message-ID: <20090310202949.GA17184@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <fa686aa40903101248n33ee35c2o32c4b24b349b14fc@mail.gmail.com>
On Tue, Mar 10, 2009 at 01:48:26PM -0600, Grant Likely wrote:
[...]
> >> eliminates the assumption that the PHY for the FEC is always
> >> attached to the FEC's own MDIO bus. With this patch, the FEC can
> >> use a PHY attached to any MDIO bus if it is described in the device
> >> tree.
> >
> > AFAIK, Gianfar and UCC Geth drivers can do this too, so I'm assuming
> > that this isn't the cause for these major changes.
>
> Certainly the mpc5200-fec driver's original phy code certainly wasn't
> as robust as the ucc_geth and gianfar phy handling.
>
> ucc_geth open codes a solution to decode the phy_device name from
> several nodes in the device tree and doesn't handle the case where the
> ucc_geth is initialized before the phy_device is registered. gianfar
> open codes the same thing. This solution uses common code to locate
> the phy_device, and it works regardless of what order devices are
> registered in.
>
> That being said, the 5200 driver originally using probe() time to
> connect to the phy. If I change it to be connected at open time, then
> does the registration order issue become irrelevant?
Yup. `ifconfig ethX up' calls ->open(). If it fails (and prints
nice error), you can try `ifconfig ethX up' again later.
> Regardless, I
> think all the drivers should be using common code for obtaining the
> phy_device from the device tree.
Not necessary `struct phy_device'. All we need is some common
routine for translating PHY's "mdio_node->full_name + phy id" to
phy_bus_id.
That is, you can just factor out this code from the gianfar driver,
void gfar_mdio_bus_name(char *name, struct device_node *np)
{
const u32 *reg;
reg = of_get_property(np, "reg", NULL);
snprintf(name, MII_BUS_ID_SIZE, "%s@%x", np->name, reg ? *reg : 0);
}
...
gfar_mdio_bus_name(bus_name, mdio);
snprintf(priv->phy_bus_id, sizeof(priv->phy_bus_id), "%s:%02x",
bus_name, *id);
...
And make sure FEC MDIO driver does mdio_bus->parent = &ofdev->dev;
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
WARNING: multiple messages have this Message-ID (diff)
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: afleming@freescale.com, linuxppc-dev@ozlabs.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
jgarzik@pobox.com
Subject: Re: [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure
Date: Tue, 10 Mar 2009 23:29:49 +0300 [thread overview]
Message-ID: <20090310202949.GA17184@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <fa686aa40903101248n33ee35c2o32c4b24b349b14fc@mail.gmail.com>
On Tue, Mar 10, 2009 at 01:48:26PM -0600, Grant Likely wrote:
[...]
> >> eliminates the assumption that the PHY for the FEC is always
> >> attached to the FEC's own MDIO bus. With this patch, the FEC can
> >> use a PHY attached to any MDIO bus if it is described in the device
> >> tree.
> >
> > AFAIK, Gianfar and UCC Geth drivers can do this too, so I'm assuming
> > that this isn't the cause for these major changes.
>
> Certainly the mpc5200-fec driver's original phy code certainly wasn't
> as robust as the ucc_geth and gianfar phy handling.
>
> ucc_geth open codes a solution to decode the phy_device name from
> several nodes in the device tree and doesn't handle the case where the
> ucc_geth is initialized before the phy_device is registered. gianfar
> open codes the same thing. This solution uses common code to locate
> the phy_device, and it works regardless of what order devices are
> registered in.
>
> That being said, the 5200 driver originally using probe() time to
> connect to the phy. If I change it to be connected at open time, then
> does the registration order issue become irrelevant?
Yup. `ifconfig ethX up' calls ->open(). If it fails (and prints
nice error), you can try `ifconfig ethX up' again later.
> Regardless, I
> think all the drivers should be using common code for obtaining the
> phy_device from the device tree.
Not necessary `struct phy_device'. All we need is some common
routine for translating PHY's "mdio_node->full_name + phy id" to
phy_bus_id.
That is, you can just factor out this code from the gianfar driver,
void gfar_mdio_bus_name(char *name, struct device_node *np)
{
const u32 *reg;
reg = of_get_property(np, "reg", NULL);
snprintf(name, MII_BUS_ID_SIZE, "%s@%x", np->name, reg ? *reg : 0);
}
...
gfar_mdio_bus_name(bus_name, mdio);
snprintf(priv->phy_bus_id, sizeof(priv->phy_bus_id), "%s:%02x",
bus_name, *id);
...
And make sure FEC MDIO driver does mdio_bus->parent = &ofdev->dev;
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
next prev parent reply other threads:[~2009-03-10 20:29 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-10 15:21 [PATCH 0/5] Retrieving Ethernet PHY wireup from the OF device tree Grant Likely
2009-03-10 15:22 ` [PATCH 1/5] drivers/base: Add bus_register_notifier_alldev() variant Grant Likely
2009-03-10 15:22 ` [PATCH 2/5] phylib: rework to prepare for OF registration of PHYs Grant Likely
2009-03-10 15:22 ` [PATCH 3/5] phylib: add *_direct() variants of phy_connect and phy_attach functions Grant Likely
2009-03-10 15:22 ` [PATCH 4/5] openfirmware: Add OF phylib support code Grant Likely
2009-03-10 15:22 ` [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure Grant Likely
2009-03-10 19:16 ` Anton Vorontsov
2009-03-10 19:16 ` Anton Vorontsov
2009-03-10 19:48 ` Grant Likely
2009-03-10 19:48 ` Grant Likely
2009-03-10 19:48 ` Grant Likely
2009-03-10 20:29 ` Anton Vorontsov [this message]
2009-03-10 20:29 ` Anton Vorontsov
2009-03-19 4:35 ` Grant Likely
2009-03-19 4:35 ` Grant Likely
2009-03-19 4:35 ` Grant Likely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090310202949.GA17184@oksana.dev.rtsoft.ru \
--to=avorontsov@ru.mvista.com \
--cc=afleming@freescale.com \
--cc=grant.likely@secretlab.ca \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.