lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fa686aa40907211009o473020e2wb0ea79130b6f1d09@mail.gmail.com>
Date:	Tue, 21 Jul 2009 11:09:46 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Mark Ware <mware@...hinstone.net>
Cc:	netdev@...r.kernel.org, linuxppc-dev <linuxppc-dev@...abs.org>,
	David Miller <davem@...emloft.net>
Subject: Re: [PATCH v2] net: Rework mdio-ofgpio driver to use of_mdio 
	infrastructure

On Tue, Jul 21, 2009 at 3:50 AM, Mark Ware<mware@...hinstone.net> wrote:
> Changes to the fs_enet driver (aa73832c5a80d6c52c69b18af858d88fa595dd3c)
> cause kernel crashes when using the mdio-ofgpio driver.
>
> The following patch replicates similar changes made to the fs_enet
> mii-bitbang drivers.

This looks good to me.  I've not tested it though (no hardware that
uses it).  You should specify the testing you've done in the commit
log.  Your signed-off-by: line is also missing.

Acked-by: Grant Likely <grant.likely@...retlab.ca>

David, once the above issues are solved, this one needs to be merged for 2.6.31.

Cheers,
g.

> ---
>
> This version attempts to address Grant's comments below:
>
> Grant Likely wrote:
>>
>> 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.
>>
>
>  drivers/net/phy/mdio-gpio.c |   77
> ++++++++++++++++++++-----------------------
>  1 files changed, 36 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index 33984b7..22cdd45 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
>
> @@ -81,13 +82,12 @@ static struct mdiobb_ops mdio_gpio_ops = {
>        .get_mdio_data = mdio_get,
>  };
>
> -static int __devinit mdio_gpio_bus_init(struct device *dev,
> +static struct mii_bus * __devinit mdio_gpio_bus_init(struct device *dev,
>                                        struct mdio_gpio_platform_data
> *pdata,
>                                        int bus_id)
>  {
>        struct mii_bus *new_bus;
>        struct mdio_gpio_info *bitbang;
> -       int ret = -ENOMEM;
>        int i;
>
>        bitbang = kzalloc(sizeof(*bitbang), GFP_KERNEL);
> @@ -104,8 +104,6 @@ static int __devinit mdio_gpio_bus_init(struct device
> *dev,
>
>        new_bus->name = "GPIO Bitbanged MDIO",
>
> -       ret = -ENODEV;
> -
>        new_bus->phy_mask = pdata->phy_mask;
>        new_bus->irq = pdata->irqs;
>        new_bus->parent = dev;
> @@ -129,15 +127,8 @@ static int __devinit mdio_gpio_bus_init(struct device
> *dev,
>
>        dev_set_drvdata(dev, new_bus);
>
> -       ret = mdiobus_register(new_bus);
> -       if (ret)
> -               goto out_free_all;
> -
> -       return 0;
> +       return new_bus;
>
> -out_free_all:
> -       dev_set_drvdata(dev, NULL);
> -       gpio_free(bitbang->mdio);
>  out_free_mdc:
>        gpio_free(bitbang->mdc);
>  out_free_bus:
> @@ -145,30 +136,47 @@ out_free_bus:
>  out_free_bitbang:
>        kfree(bitbang);
>  out:
> -       return ret;
> +       return NULL;
>  }
>
> -static void __devexit mdio_gpio_bus_destroy(struct device *dev)
> +static void __devinit mdio_gpio_bus_deinit(struct device *dev)
>  {
>        struct mii_bus *bus = dev_get_drvdata(dev);
>        struct mdio_gpio_info *bitbang = bus->priv;
>
> -       mdiobus_unregister(bus);
> -       free_mdio_bitbang(bus);
>        dev_set_drvdata(dev, NULL);
> -       gpio_free(bitbang->mdc);
>        gpio_free(bitbang->mdio);
> +       gpio_free(bitbang->mdc);
> +       free_mdio_bitbang(bus);
>        kfree(bitbang);
>  }
>
> +static void __devexit mdio_gpio_bus_destroy(struct device *dev)
> +{
> +       struct mii_bus *bus = dev_get_drvdata(dev);
> +
> +       mdiobus_unregister(bus);
> +       mdio_gpio_bus_deinit(dev);
> +}
> +
>  static int __devinit mdio_gpio_probe(struct platform_device *pdev)
>  {
>        struct mdio_gpio_platform_data *pdata = pdev->dev.platform_data;
> +       struct mii_bus *new_bus;
> +       int ret;
>
>        if (!pdata)
>                return -ENODEV;
>
> -       return mdio_gpio_bus_init(&pdev->dev, pdata, pdev->id);
> +       new_bus = mdio_gpio_bus_init(&pdev->dev, pdata, pdev->id);
> +       if (!new_bus)
> +               return -ENODEV;
> +
> +       ret = mdiobus_register(new_bus);
> +       if (ret)
> +               mdio_gpio_bus_deinit(&pdev->dev);
> +
> +       return ret;
>  }
>
>  static int __devexit mdio_gpio_remove(struct platform_device *pdev)
> @@ -179,29 +187,12 @@ 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;
> +       struct mii_bus *new_bus;
>        int ret;
>
>        pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> @@ -215,14 +206,18 @@ static int __devinit mdio_ofgpio_probe(struct
> of_device *ofdev,
>
>        ret = of_get_gpio(ofdev->node, 1);
>        if (ret < 0)
> -                goto out_free;
> +               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);
> +       new_bus = mdio_gpio_bus_init(&ofdev->dev, pdata, pdata->mdc);
> +       if (!new_bus)
> +               return -ENODEV;
>
> -       return mdio_gpio_bus_init(&ofdev->dev, pdata, pdata->mdc);
> +       ret = of_mdiobus_register(new_bus, ofdev->node);
> +       if (ret)
> +               mdio_gpio_bus_deinit(&ofdev->dev);
> +
> +       return ret;
>
>  out_free:
>        kfree(pdata);
> --
> 1.5.6.5
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ