[PATCH] net: fix OF fixed-link property handling on Freescale network device drivers
Anton Vorontsov
avorontsov at ru.mvista.com
Wed Jul 8 00:35:02 EST 2009
On Fri, Jul 03, 2009 at 04:20:19PM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely at secretlab.ca>
>
> The MDIO rework patches broke the handling of fixed MII links.
[...]
> Signed-off-by: Grant Likely <grant.likely at secretlab.ca>
> ---
> Anton, can you please review, comment and test? I've tested it on an
> mpc8349 board, but that is the only hardware that I have. I've also
> probably made mistakes and it needs to be split up into separate patches,
> but this is probably a sufficient form for first review. I'll also give
> it another once over tomorrow when after I've had a decent night sleep.
Did some tests for ucc geth...
Will get to fs_enet a bit later.
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 40c6eba..c216cd5 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -1443,6 +1443,53 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
> return 0;
> }
>
> +static void ugeth_set_link(struct net_device *dev)
> +{
> + struct ucc_geth_private *ugeth = netdev_priv(dev);
> + struct phy_device *phydev = ugeth->phydev;
> + struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
> + struct ucc_fast __iomem *uf_regs = ugeth->uccf->uf_regs;
In fixed-link case you'll call set_link() before ucc_struct_init,
so these *_regs are NULL, following oops pops up:
Unable to handle kernel paging request for data at address 0x00000004
Faulting instruction address: 0xc01d6228
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01d6228] ugeth_set_link+0x20/0x158
LR [c01d723c] init_phy+0xa8/0xdc
Call Trace:
[cf831e40] [84042028] 0x84042028 (unreliable)
[cf831e60] [c01d723c] init_phy+0xa8/0xdc
[cf831e80] [c01d8fb8] ucc_geth_open+0x2c/0x2b0
[cf831ea0] [c02040a8] dev_open+0xfc/0x134
[cf831ec0] [c0202670] dev_change_flags+0x84/0x1ac
[cf831ee0] [c03b0cfc] ic_open_devs+0x168/0x2cc
[cf831f20] [c03b20f8] ip_auto_config+0x90/0x2a4
[cf831f60] [c0003894] do_one_initcall+0x34/0x1a8
[cf831fd0] [c03922f8] do_initcalls+0x38/0x58
[cf831fe0] [c0392388] kernel_init+0x38/0x98
[cf831ff0] [c0011b78] kernel_thread+0x4c/0x68
> + u32 tempval = in_be32(&ug_regs->maccfg2);
> + u32 upsmr = in_be32(&uf_regs->upsmr);
[...]
> + default:
> + if (netif_msg_link(ugeth))
> + ugeth_warn("%s: Ack! Speed (%d) is not 10/100/1000!",
> + dev->name, phydev->speed);
You may dereference NULL here.
> + break;
> + }
> +
> + out_be32(&ug_regs->maccfg2, tempval);
(while you're at it, maybe s/tempval/maccfg2/ ?)
> + out_be32(&uf_regs->upsmr, upsmr);
> +}
> +
[...]
> @@ -3708,15 +3710,19 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
>
> ug_info->uf_info.regs = res.start;
> ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
> - fixed_link = of_get_property(np, "fixed-link", NULL);
> - if (fixed_link) {
> - phy = NULL;
> - } else {
> - phy = of_parse_phandle(np, "phy-handle", 0);
> - if (phy == NULL)
> - return -ENODEV;
> +
> + /* Setup the initial link state */
> + prop = of_get_property(np, "fixed-link", &prop_sz);
> + if (prop && prop_sz >= sizeof(*prop) * 3) {
> + ugeth->oldlink = 1;
'ugeth' is NULL at this point, causes this:
Unable to handle kernel paging request for data at address 0x000002c4
Faulting instruction address: 0xc01da0d8
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01da0d8] ucc_geth_probe+0x218/0x530
LR [c01da0c0] ucc_geth_probe+0x200/0x530
Call Trace:
[cf831e00] [c01da0c0] ucc_geth_probe+0x200/0x530 (unreliable)
[cf831e60] [c01eddc4] of_platform_device_probe+0x5c/0x84
[cf831e80] [c019bfd4] really_probe+0x78/0x1a0
[cf831ea0] [c019c1c4] __driver_attach+0xa4/0xa8
[cf831ec0] [c019b3ec] bus_for_each_dev+0x60/0x9c
[cf831ef0] [c019be18] driver_attach+0x24/0x34
[cf831f00] [c019badc] bus_add_driver+0xb4/0x1c4
> + ugeth->oldduplex = prop[1];
> + ugeth->oldspeed = prop[2];
> }
> - ug_info->phy_node = phy;
> +
> + /* Find the phy. Bail if there is no phy and no initial link speed */
> + ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
> + if (!ug_info->phy_node && !ugeth->oldlink)
> + return -ENODEV;
>
> /* Find the TBI PHY node. If it's not there, we don't support SGMII */
> ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
> @@ -3725,7 +3731,7 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
> prop = of_get_property(np, "phy-connection-type", NULL);
> if (!prop) {
> /* handle interface property present in old trees */
> - prop = of_get_property(phy, "interface", NULL);
> + prop = of_get_property(ug_info->phy_node, "interface", NULL);
> if (prop != NULL) {
> phy_interface = enet_to_phy_interface[*prop];
> max_speed = enet_to_speed[*prop];
>
There is another oops, if interface is down:
# ethtool -S eth1
Unable to handle kernel paging request for data at address 0x00000180
Faulting instruction address: 0xc01da7f4
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01da7f4] uec_get_ethtool_stats+0x3c/0x14c
LR [c0207484] ethtool_get_stats+0xfc/0x23c
Call Trace:
[cfb2ddf0] [c0207464] ethtool_get_stats+0xdc/0x23c (unreliable)
[cfb2de30] [c0207b24] dev_ethtool+0x324/0x5a8
[cfb2de60] [c0204ad4] dev_ioctl+0x290/0x320
[cfb2deb0] [c01ef9ac] sock_ioctl+0x80/0x2f4
[cfb2ded0] [c0090814] vfs_ioctl+0x34/0x98
[cfb2dee0] [c009103c] do_vfs_ioctl+0x84/0x2cc
[cfb2df10] [c00912c4] sys_ioctl+0x40/0x74
[cfb2df40] [c0011d54] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff64820
LR = 0xffec848
Instruction dump:
80c901b8 70c00001 41820058 8123004c 7cab2b78 39000000 38000000 38e90180
39200012 7d2903a6 5400103a 7c0004ac <7d27002e> 0c090000 4c00012c 7d2a4b78
---[ end trace 7b7ae7cbafe6f2ba ]---
Segmentation fault
Also, now userspace has no chance to know link speed:
# ifconfig eth1 up
# ethtool eth1
Settings for eth1:
Cannot get device settings: No such device
Current message level: 0x0000003f (63)
Link detected: yes
#
(You need to tech *_ethtool.c to report speed/duplex for fixed-link
cases).
Thanks,
--
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2
More information about the Linuxppc-dev
mailing list