(adding cc:linuxppc-...@ozlabs.org)
On Mon, Jul 20, 2009 at 6:20 AM, Mark Ware<mw...@elphinstone.net> wrote:
> Changes to the fs_enet driver (aa73832c5a80d6c52c69b18af858d88fa595dd3c)
> cause kernel crashes when using the mdio-ofgpio driver.
>
> The following patch is a fairly naive attempt to replicate similar changes
> made to the fs_enet mii-bitbang drivers.
For a naive attempt, it's really quite good. However, I'd do it
slightly differently.
You should refactor mdio_gpio_bus_init() to not call
mdiobus_register() at all, and instead just return a pointer to the
unregistered new_bus. Then mdio_gpio_probe() and mdio_ofgpio_probe()
can call the correct register variant directly. Fewer ugly #ifdefs
this way. It also eliminates the need to cast the void* pointer.
Thanks for catching this.
g.
> drivers/net/phy/mdio-gpio.c | 39 +++++++++++++--------------------------
> 1 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index 33984b7..6568176 100644
> --- a/drivers/net/phy/mdio-gpio.c
> +++ b/drivers/net/phy/mdio-gpio.c
> @@ -30,6 +30,7 @@
>
> #ifdef CONFIG_OF_GPIO
> #include <linux/of_gpio.h>
> +#include <linux/of_mdio.h>
> #include <linux/of_platform.h>
> #endif
>
> @@ -83,7 +84,8 @@ static struct mdiobb_ops mdio_gpio_ops = {
>
> static int __devinit mdio_gpio_bus_init(struct device *dev,
> struct mdio_gpio_platform_data
> *pdata,
> - int bus_id)
> + int bus_id,
> + void *ofdev)
> {
> struct mii_bus *new_bus;
> struct mdio_gpio_info *bitbang;
> @@ -129,7 +131,14 @@ static int __devinit mdio_gpio_bus_init(struct device
> *dev,
>
> dev_set_drvdata(dev, new_bus);
>
> - ret = mdiobus_register(new_bus);
> +#ifdef CONFIG_OF_GPIO
> + if (ofdev)
> + ret = of_mdiobus_register(new_bus, ((struct of_device *)
> ofdev)->node);
> + else
> +#else
> + ret = mdiobus_register(new_bus);
> +#endif
> +
> if (ret)
> goto out_free_all;
>
> @@ -168,7 +177,7 @@ static int __devinit mdio_gpio_probe(struct
> platform_device *pdev)
> if (!pdata)
> return -ENODEV;
>
> - return mdio_gpio_bus_init(&pdev->dev, pdata, pdev->id);
> + return mdio_gpio_bus_init(&pdev->dev, pdata, pdev->id, NULL);
> }
>
> static int __devexit mdio_gpio_remove(struct platform_device *pdev)
> @@ -179,28 +188,10 @@ static int __devexit mdio_gpio_remove(struct
> platform_device *pdev)
> }
>
> #ifdef CONFIG_OF_GPIO
> -static void __devinit add_phy(struct mdio_gpio_platform_data *pdata,
> - struct device_node *np)
> -{
> - const u32 *data;
> - int len, id, irq;
> -
> - data = of_get_property(np, "reg", &len);
> - if (!data || len != 4)
> - return;
> -
> - id = *data;
> - pdata->phy_mask &= ~(1 << id);
> -
> - irq = of_irq_to_resource(np, 0, NULL);
> - if (irq)
> - pdata->irqs[id] = irq;
> -}
>
> static int __devinit mdio_ofgpio_probe(struct of_device *ofdev,
> const struct of_device_id *match)
> {
> - struct device_node *np = NULL;
> struct mdio_gpio_platform_data *pdata;
> int ret;
>
> @@ -218,11 +209,7 @@ static int __devinit mdio_ofgpio_probe(struct of_device
> *ofdev,
> goto out_free;
> pdata->mdio = ret;
>
> - while ((np = of_get_next_child(ofdev->node, np)))
> - if (!strcmp(np->type, "ethernet-phy"))
> - add_phy(pdata, np);
> -
> - return mdio_gpio_bus_init(&ofdev->dev, pdata, pdata->mdc);
> + return mdio_gpio_bus_init(&ofdev->dev, pdata, pdata->mdc, ofdev);
>
> out_free:
> kfree(pdata);
> --
> 1.5.6.5
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
