diff mbox

[3/6] mv643xx.c: Add basic device tree support.

Message ID 1343661359-10150-4-git-send-email-ian.molton@codethink.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Molton July 30, 2012, 3:15 p.m. UTC
This patch adds basic device tree support to the mv643xx ethernet driver.

It should be enough for most current users of the device, and should allow
a fairly painless migration once proper support for clk devices is available
to those platforms.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c |  111 ++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 14 deletions(-)

Comments

Andrew Lunn July 30, 2012, 3:39 p.m. UTC | #1
On Mon, Jul 30, 2012 at 04:15:56PM +0100, Ian Molton wrote:
> This patch adds basic device tree support to the mv643xx ethernet driver.
> 
> It should be enough for most current users of the device, and should allow
> a fairly painless migration once proper support for clk devices is available
> to those platforms.
> 
> Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c |  111 ++++++++++++++++++++++++----
>  1 file changed, 97 insertions(+), 14 deletions(-)

Hi Ian

Please document the DT binding in Documentation/devicetree/binding/....

Also, this should also be CC: to the netdev mailing. I got into
trouble for not doing this and breaking the PPC build :-(

	Andrew
Amar Nath July 30, 2012, 4:19 p.m. UTC | #2
Hi Ian,

On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton <ian.molton@codethink.co.uk>wrote:

> This patch adds basic device tree support to the mv643xx ethernet driver.
>
> It should be enough for most current users of the device, and should allow
> a fairly painless migration once proper support for clk devices is
> available
> to those platforms.
>
> Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c |  111
> ++++++++++++++++++++++++----
>  1 file changed, 97 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
> b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 92497eb..579692f 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -48,6 +48,9 @@
>  #include <linux/ethtool.h>
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
>  #include <linux/kernel.h>
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
> @@ -2601,11 +2604,11 @@ static void infer_hw_params(struct
> mv643xx_eth_shared_private *msp)
>  static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  {
>         static int mv643xx_eth_version_printed;
> -       struct mv643xx_eth_shared_platform_data *pd =
> pdev->dev.platform_data;
> +       struct mv643xx_eth_shared_platform_data *pd;
>         struct mv643xx_eth_shared_private *msp;
>         const struct mbus_dram_target_info *dram;
>         struct resource *res;
> -       int ret;
> +       int ret, irq = -1;
>
>         if (!mv643xx_eth_version_printed++)
>                 pr_notice("MV-643xx 10/100/1000 ethernet driver version
> %s\n",
> @@ -2625,6 +2628,23 @@ static int mv643xx_eth_shared_probe(struct
> platform_device *pdev)
>         if (msp->base == NULL)
>                 goto out_free;
>
> +       if (pdev->dev.of_node) {
> +               struct device_node *np = NULL;
> +
> +               /* when all users of this driver use FDT, we can remove
> this */
> +               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
> +               if (!pd) {
> +                       dev_dbg(&pdev->dev, "Could not allocate platform
> data\n");
> +                       goto out_free;
> +               }
> +
> +               np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
> +               if (np)
> +                       pd->shared_smi = of_find_device_by_node(np);
> +
> +       } else {
> +               pd = pdev->dev.platform_data;
> +       }
>         /*
>          * Set up and register SMI bus.
>          */
> @@ -2654,15 +2674,22 @@ static int mv643xx_eth_shared_probe(struct
> platform_device *pdev)
>         /*
>          * Check whether the error interrupt is hooked up.
>          */
> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -       if (res != NULL) {
> +       if (pdev->dev.of_node) {
> +               irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +       } else {
> +               res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +               if (res)
> +                       irq = res->start;
> +       }
> +
> +       if (irq != -1) {
>                 int err;
>
> -               err = request_irq(res->start, mv643xx_eth_err_irq,
> +               err = request_irq(irq, mv643xx_eth_err_irq,
>                                   IRQF_SHARED, "mv643xx_eth", msp);
>                 if (!err) {
>                         writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
> -                       msp->err_interrupt = res->start;
> +                       msp->err_interrupt = irq;
>                 }
>         }
>
> @@ -2675,6 +2702,10 @@ static int mv643xx_eth_shared_probe(struct
> platform_device *pdev)
>
>         msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
>                                         pd->tx_csum_limit : 9 * 1024;
> +
> +       if (pdev->dev.of_node)
> +               kfree(pd);  /* If we created a fake pd, free it now */
> +
>         infer_hw_params(msp);
>
>         platform_set_drvdata(pdev, msp);
> @@ -2708,12 +2739,21 @@ static int mv643xx_eth_shared_remove(struct
> platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
> +       { .compatible = "marvell,mdio-mv643xx", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
> +#endif
> +
>  static struct platform_driver mv643xx_eth_shared_driver = {
>         .probe          = mv643xx_eth_shared_probe,
>         .remove         = mv643xx_eth_shared_remove,
>         .driver = {
>                 .name   = MV643XX_ETH_SHARED_NAME,
>                 .owner  = THIS_MODULE,
> +               .of_match_table = of_match_ptr(mv_mdio_dt_ids),
>         },
>  };
>
> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device
> *pdev)
>         struct resource *res;
>         int err;
>
> -       pd = pdev->dev.platform_data;
> +       if (pdev->dev.of_node) {
> +               struct device_node *np = NULL;
> +
> +               /* when all users of this driver use FDT, we can remove
> this */
> +               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
> +               if (!pd) {
> +                       dev_dbg(&pdev->dev, "Could not allocate platform
> data\n");
> +                       return -ENOMEM;
> +               }
> +
> +               of_property_read_u32(pdev->dev.of_node,
> +                       "port_number", &pd->port_number);
> +               of_property_read_u32(pdev->dev.of_node,
> +                       "phy_addr", &pd->phy_addr);
> +               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
> +               if (np) {
> +                       pd->shared = of_find_device_by_node(np);
> +               } else {
> +                       kfree(pd);
> +                       return -ENODEV;
> +               }
> +       } else {
> +               pd = pdev->dev.platform_data;
> +       }
> +
>         if (pd == NULL) {
>                 dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
>                 return -ENODEV;
>

Can this check for pd be moved in the else part above as well, as for the
pd allocation with kzalloc,
we have already made a check for memory allocation failure?



> @@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct
> platform_device *pdev)
>
>         if (pd->shared == NULL) {
>                 dev_err(&pdev->dev, "no
> mv643xx_eth_platform_data->shared\n");
> -               return -ENODEV;
> +               err = -ENODEV;
> +               goto out_free_pd;
>         }
>
>         dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
> -       if (!dev)
> -               return -ENOMEM;
> +       if (!dev) {
> +               err = -ENOMEM;
> +               goto out_free_pd;
> +       }
>
>         mp = netdev_priv(dev);
>         platform_set_drvdata(pdev, mp);
> @@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device
> *pdev)
>
>         init_pscr(mp, pd->speed, pd->duplex);
>
> +       if (pdev->dev.of_node)
> +               kfree(pd); /* If we created a fake pd, free it now */
>
>         mib_counters_clear(mp);
>
> @@ -2942,10 +3011,13 @@ static int mv643xx_eth_probe(struct
> platform_device *pdev)
>         mp->rx_oom.data = (unsigned long)mp;
>         mp->rx_oom.function = oom_timer_wrapper;
>
> -
> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -       BUG_ON(!res);
> -       dev->irq = res->start;
> +       if (pdev->dev.of_node) {
> +               dev->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +       } else {
> +               res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +               BUG_ON(!res);
> +               dev->irq = res->start;
> +       }
>
>         dev->netdev_ops = &mv643xx_eth_netdev_ops;
>
> @@ -2991,6 +3063,8 @@ out:
>         }
>  #endif
>         free_netdev(dev);
> +out_free_pd:
> +       kfree(pd);
>
>         return err;
>  }
> @@ -3030,6 +3104,14 @@ static void mv643xx_eth_shutdown(struct
> platform_device *pdev)
>                 port_reset(mp);
>  }
>
> +#ifdef CONFIG_OF
> +static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
> +       { .compatible = "marvell,mv643xx", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
> +#endif
> +
>  static struct platform_driver mv643xx_eth_driver = {
>         .probe          = mv643xx_eth_probe,
>         .remove         = mv643xx_eth_remove,
> @@ -3037,6 +3119,7 @@ static struct platform_driver mv643xx_eth_driver = {
>         .driver = {
>                 .name   = MV643XX_ETH_NAME,
>                 .owner  = THIS_MODULE,
> +               .of_match_table = of_match_ptr(mv_eth_dt_ids),
>         },
>  };
>
> --
> 1.7.9.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



Regards,
-Amar
Ian Molton July 30, 2012, 4:32 p.m. UTC | #3
On 30/07/12 17:19, Amar Nath wrote:
> Hi Ian,
>
> On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton <ian.molton@codethink.co.uk>wrote:
>
>> -       pd = pdev->dev.platform_data;
>> +       if (pdev->dev.of_node) {
>> +               struct device_node *np = NULL;
>> +
>> +               /* when all users of this driver use FDT, we can remove
>> this */
>> +               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
>> +               if (!pd) {
>> +                       dev_dbg(&pdev->dev, "Could not allocate platform
>> data\n");
>> +                       return -ENOMEM;
>> +               }
>> +
>> +               of_property_read_u32(pdev->dev.of_node,
>> +                       "port_number", &pd->port_number);
>> +               of_property_read_u32(pdev->dev.of_node,
>> +                       "phy_addr", &pd->phy_addr);
>> +               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
>> +               if (np) {
>> +                       pd->shared = of_find_device_by_node(np);
>> +               } else {
>> +                       kfree(pd);
>> +                       return -ENODEV;
>> +               }
>> +       } else {
>> +               pd = pdev->dev.platform_data;
>> +       }
>> +
>>         if (pd == NULL) {
>>                 dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
>>                 return -ENODEV;
>>
> Can this check for pd be moved in the else part above as well, as for the
> pd allocation with kzalloc,
> we have already made a check for memory allocation failure?

Yes, Folded in for v2.

-Ian
Ian Molton July 30, 2012, 4:49 p.m. UTC | #4
On 30/07/12 16:39, Andrew Lunn wrote:
> On Mon, Jul 30, 2012 at 04:15:56PM +0100, Ian Molton wrote:
>> This patch adds basic device tree support to the mv643xx ethernet
>> driver.
>>
>> It should be enough for most current users of the device, and
>> should allow a fairly painless migration once proper support for
>> clk devices is available to those platforms.
>>
>> Signed-off-by: Ian Molton <ian.molton@codethink.co.uk> ---
>> drivers/net/ethernet/marvell/mv643xx_eth.c | 111
>> ++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 14
>> deletions(-)
>
> Hi Ian
>
> Please document the DT binding in
> Documentation/devicetree/binding/....

Done.

> Also, this should also be CC: to the netdev mailing. I got into
> trouble for not doing this and breaking the PPC build :-(

Will do so for v2, thanks.

-Ian
Arnaud Patard (Rtp) July 31, 2012, 7:14 a.m. UTC | #5
Ian Molton <ian.molton@codethink.co.uk> writes:
Hi,

[...]

> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int err;
>  
> -	pd = pdev->dev.platform_data;
> +	if (pdev->dev.of_node) {
> +		struct device_node *np = NULL;
> +
> +		/* when all users of this driver use FDT, we can remove this */
> +		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
> +		if (!pd) {
> +			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
> +			return -ENOMEM;
> +		}
> +
> +		of_property_read_u32(pdev->dev.of_node,
> +			"port_number", &pd->port_number);
> +		of_property_read_u32(pdev->dev.of_node,
> +			"phy_addr", &pd->phy_addr);

I guess we need something for tx_csum_limit in the device tree too. It's
important for kirkwood and dove.

Arnaud
Ian Molton July 31, 2012, 8:19 a.m. UTC | #6
On 31/07/12 08:14, Arnaud Patard (Rtp) wrote:
> Ian Molton <ian.molton@codethink.co.uk> writes:
> Hi,
>
> [...]
>
>> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	int err;
>>  
>> -	pd = pdev->dev.platform_data;
>> +	if (pdev->dev.of_node) {
>> +		struct device_node *np = NULL;
>> +
>> +		/* when all users of this driver use FDT, we can remove this */
>> +		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
>> +		if (!pd) {
>> +			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
>> +			return -ENOMEM;
>> +		}
>> +
>> +		of_property_read_u32(pdev->dev.of_node,
>> +			"port_number", &pd->port_number);
>> +		of_property_read_u32(pdev->dev.of_node,
>> +			"phy_addr", &pd->phy_addr);
> I guess we need something for tx_csum_limit in the device tree too. It's
> important for kirkwood and dove.

It was my intention to keep the patch as non-invasive as possible - none
of the platforms using D-T at present require this.

Furthermore, git grep shows nothing in mach-kirkwood or mach-dove using
at all...

Are you sure this is required? It can always be added if anything
actually uses it :)

-Ian
Andrew Lunn July 31, 2012, 8:27 a.m. UTC | #7
On Tue, Jul 31, 2012 at 09:19:52AM +0100, Ian Molton wrote:
> On 31/07/12 08:14, Arnaud Patard (Rtp) wrote:
> > Ian Molton <ian.molton@codethink.co.uk> writes:
> > Hi,
> >
> > [...]
> >
> >> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
> >>  	struct resource *res;
> >>  	int err;
> >>  
> >> -	pd = pdev->dev.platform_data;
> >> +	if (pdev->dev.of_node) {
> >> +		struct device_node *np = NULL;
> >> +
> >> +		/* when all users of this driver use FDT, we can remove this */
> >> +		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
> >> +		if (!pd) {
> >> +			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
> >> +			return -ENOMEM;
> >> +		}
> >> +
> >> +		of_property_read_u32(pdev->dev.of_node,
> >> +			"port_number", &pd->port_number);
> >> +		of_property_read_u32(pdev->dev.of_node,
> >> +			"phy_addr", &pd->phy_addr);
> > I guess we need something for tx_csum_limit in the device tree too. It's
> > important for kirkwood and dove.
> 
> It was my intention to keep the patch as non-invasive as possible - none
> of the platforms using D-T at present require this.
> 
> Furthermore, git grep shows nothing in mach-kirkwood or mach-dove using
> at all...
> 
> Are you sure this is required? It can always be added if anything
> actually uses it :)

Hi Ian

Try using jumbo packets on kirkwood.

http://comments.gmane.org/gmane.linux.ports.arm.kernel/178785
http://www.spinics.net/lists/arm-kernel/msg186152.html

	Andrew
Ian Molton July 31, 2012, 8:45 a.m. UTC | #8
On 31/07/12 09:27, Andrew Lunn wrote:
> On Tue, Jul 31, 2012 at 09:19:52AM +0100, Ian Molton wrote:
>> Are you sure this is required? It can always be added if anything
>> actually uses it :)
> Hi Ian
>
> Try using jumbo packets on kirkwood.
>
> http://comments.gmane.org/gmane.linux.ports.arm.kernel/178785
> http://www.spinics.net/lists/arm-kernel/msg186152.html

Ah, this postdates the D-T branches on which I am working being forked.

I'll fold this into my patchset.

-Ian
Arnaud Patard (Rtp) July 31, 2012, 8:58 a.m. UTC | #9
Ian Molton <ian.molton@codethink.co.uk> writes:

> On 31/07/12 09:27, Andrew Lunn wrote:
>> On Tue, Jul 31, 2012 at 09:19:52AM +0100, Ian Molton wrote:
>>> Are you sure this is required? It can always be added if anything
>>> actually uses it :)
>> Hi Ian
>>
>> Try using jumbo packets on kirkwood.
>>
>> http://comments.gmane.org/gmane.linux.ports.arm.kernel/178785
>> http://www.spinics.net/lists/arm-kernel/msg186152.html
>
> Ah, this postdates the D-T branches on which I am working being forked.

the original patch was from years ago but I noticed only recently it was
still not merged and sent this patch. I asked about it on DT because I
don't want it to get lost again, even if it looks like not a lot of
people are using jumbo frames on kirkwood or dove.

>
> I'll fold this into my patchset.

Thanks,
Arnaud
Ian Molton July 31, 2012, 9:14 a.m. UTC | #10
On 31/07/12 09:58, Arnaud Patard (Rtp) wrote:
>> Ah, this postdates the D-T branches on which I am working being forked.
> the original patch was from years ago but I noticed only recently it was
> still not merged and sent this patch. I asked about it on DT because I
> don't want it to get lost again, even if it looks like not a lot of
> people are using jumbo frames on kirkwood or dove.

No problem :) actually it works really nicely in DT, just drop it in
the toplevel includes!

-Ian
Ben Dooks July 31, 2012, 2:30 p.m. UTC | #11
On 31/07/12 08:14, Arnaud Patard (Rtp) wrote:
> Ian Molton<ian.molton@codethink.co.uk>  writes:
> Hi,
>
> [...]
>
>> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>>   	struct resource *res;
>>   	int err;
>>
>> -	pd = pdev->dev.platform_data;
>> +	if (pdev->dev.of_node) {
>> +		struct device_node *np = NULL;
>> +
>> +		/* when all users of this driver use FDT, we can remove this */
>> +		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
>> +		if (!pd) {
>> +			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
>> +			return -ENOMEM;
>> +		}
>> +
>> +		of_property_read_u32(pdev->dev.of_node,
>> +			"port_number",&pd->port_number);
>> +		of_property_read_u32(pdev->dev.of_node,
>> +			"phy_addr",&pd->phy_addr);
>
> I guess we need something for tx_csum_limit in the device tree too. It's
> important for kirkwood and dove.

Is this something for all kirkwood devices? I think the best way of
doing this is to give the driver a new device-tree name and having
it set this field in the init sequence.

However we could also do with some of these as dt parameters just in
case people want to try and alter them for their own systems.
Arnaud Patard (Rtp) July 31, 2012, 3:04 p.m. UTC | #12
Ben Dooks <ben.dooks@codethink.co.uk> writes:

> On 31/07/12 08:14, Arnaud Patard (Rtp) wrote:
>> Ian Molton<ian.molton@codethink.co.uk>  writes:
>> Hi,
>>
>> [...]
>>
>>> @@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>>>   	struct resource *res;
>>>   	int err;
>>>
>>> -	pd = pdev->dev.platform_data;
>>> +	if (pdev->dev.of_node) {
>>> +		struct device_node *np = NULL;
>>> +
>>> +		/* when all users of this driver use FDT, we can remove this */
>>> +		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
>>> +		if (!pd) {
>>> +			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
>>> +			return -ENOMEM;
>>> +		}
>>> +
>>> +		of_property_read_u32(pdev->dev.of_node,
>>> +			"port_number",&pd->port_number);
>>> +		of_property_read_u32(pdev->dev.of_node,
>>> +			"phy_addr",&pd->phy_addr);
>>
>> I guess we need something for tx_csum_limit in the device tree too. It's
>> important for kirkwood and dove.
>
> Is this something for all kirkwood devices? I think the best way of
> doing this is to give the driver a new device-tree name and having
> it set this field in the init sequence.
>
> However we could also do with some of these as dt parameters just in
> case people want to try and alter them for their own systems.

It's for _all_ kirkwood and dove devices. The kirkwood and dove fifos
are smaller than the orion/mv78xxx ones (no idea about armada xp/370).

Arnaud
Florian Fainelli July 31, 2012, 3:12 p.m. UTC | #13
Hello Ian,

On Monday 30 July 2012 17:32:39 Ian Molton wrote:
> On 30/07/12 17:19, Amar Nath wrote:
> > Hi Ian,
> >
> > On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton 
<ian.molton@codethink.co.uk>wrote:
> >
> >> -       pd = pdev->dev.platform_data;
> >> +       if (pdev->dev.of_node) {
> >> +               struct device_node *np = NULL;
> >> +
> >> +               /* when all users of this driver use FDT, we can remove
> >> this */
> >> +               pd = kzalloc(sizeof(*pd), GFP_ATOMIC);

Do you really need an allocation in atomic context here? Is not GFP_KERNEL 
sufficient?

There was a second place like this where you used an atomic allocation that 
needs fixing.

> >> +               if (!pd) {
> >> +                       dev_dbg(&pdev->dev, "Could not allocate platform
> >> data\n");
> >> +                       return -ENOMEM;
> >> +               }
> >> +
> >> +               of_property_read_u32(pdev->dev.of_node,
> >> +                       "port_number", &pd->port_number);
> >> +               of_property_read_u32(pdev->dev.of_node,
> >> +                       "phy_addr", &pd->phy_addr);
> >> +               np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
> >> +               if (np) {
> >> +                       pd->shared = of_find_device_by_node(np);
> >> +               } else {
> >> +                       kfree(pd);
> >> +                       return -ENODEV;
> >> +               }
> >> +       } else {
> >> +               pd = pdev->dev.platform_data;
> >> +       }
> >> +
> >>         if (pd == NULL) {
> >>                 dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
> >>                 return -ENODEV;
> >>
> > Can this check for pd be moved in the else part above as well, as for the
> > pd allocation with kzalloc,
> > we have already made a check for memory allocation failure?
> 
> Yes, Folded in for v2.
> 
> -Ian
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 92497eb..579692f 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -48,6 +48,9 @@ 
 #include <linux/ethtool.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -2601,11 +2604,11 @@  static void infer_hw_params(struct mv643xx_eth_shared_private *msp)
 static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 {
 	static int mv643xx_eth_version_printed;
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
+	struct mv643xx_eth_shared_platform_data *pd;
 	struct mv643xx_eth_shared_private *msp;
 	const struct mbus_dram_target_info *dram;
 	struct resource *res;
-	int ret;
+	int ret, irq = -1;
 
 	if (!mv643xx_eth_version_printed++)
 		pr_notice("MV-643xx 10/100/1000 ethernet driver version %s\n",
@@ -2625,6 +2628,23 @@  static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	if (msp->base == NULL)
 		goto out_free;
 
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			goto out_free;
+		}
+
+		np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
+		if (np)
+			pd->shared_smi = of_find_device_by_node(np);
+
+	} else {
+		pd = pdev->dev.platform_data;
+	}
 	/*
 	 * Set up and register SMI bus.
 	 */
@@ -2654,15 +2674,22 @@  static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	/*
 	 * Check whether the error interrupt is hooked up.
 	 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res != NULL) {
+	if (pdev->dev.of_node) {
+		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	} else {
+		res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		if (res)
+			irq = res->start;
+	}
+
+	if (irq != -1) {
 		int err;
 
-		err = request_irq(res->start, mv643xx_eth_err_irq,
+		err = request_irq(irq, mv643xx_eth_err_irq,
 				  IRQF_SHARED, "mv643xx_eth", msp);
 		if (!err) {
 			writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
-			msp->err_interrupt = res->start;
+			msp->err_interrupt = irq;
 		}
 	}
 
@@ -2675,6 +2702,10 @@  static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
 					pd->tx_csum_limit : 9 * 1024;
+
+	if (pdev->dev.of_node)
+		kfree(pd);  /* If we created a fake pd, free it now */
+
 	infer_hw_params(msp);
 
 	platform_set_drvdata(pdev, msp);
@@ -2708,12 +2739,21 @@  static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mdio-mv643xx", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_shared_driver = {
 	.probe		= mv643xx_eth_shared_probe,
 	.remove		= mv643xx_eth_shared_remove,
 	.driver = {
 		.name	= MV643XX_ETH_SHARED_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_mdio_dt_ids),
 	},
 };
 
@@ -2873,7 +2913,31 @@  static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;
+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number", &pd->port_number);
+		of_property_read_u32(pdev->dev.of_node,
+			"phy_addr", &pd->phy_addr);
+		np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+		if (np) {
+			pd->shared = of_find_device_by_node(np);
+		} else {
+			kfree(pd);
+			return -ENODEV;
+		}
+	} else {
+		pd = pdev->dev.platform_data;
+	}
+
 	if (pd == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
 		return -ENODEV;
@@ -2881,12 +2945,15 @@  static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	if (pd->shared == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data->shared\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto out_free_pd;
 	}
 
 	dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
-	if (!dev)
-		return -ENOMEM;
+	if (!dev) {
+		err = -ENOMEM;
+		goto out_free_pd;
+	}
 
 	mp = netdev_priv(dev);
 	platform_set_drvdata(pdev, mp);
@@ -2923,6 +2990,8 @@  static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	init_pscr(mp, pd->speed, pd->duplex);
 
+	if (pdev->dev.of_node)
+		kfree(pd); /* If we created a fake pd, free it now */
 
 	mib_counters_clear(mp);
 
@@ -2942,10 +3011,13 @@  static int mv643xx_eth_probe(struct platform_device *pdev)
 	mp->rx_oom.data = (unsigned long)mp;
 	mp->rx_oom.function = oom_timer_wrapper;
 
-
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	BUG_ON(!res);
-	dev->irq = res->start;
+	if (pdev->dev.of_node) {
+		dev->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	} else {
+		res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		BUG_ON(!res);
+		dev->irq = res->start;
+	}
 
 	dev->netdev_ops = &mv643xx_eth_netdev_ops;
 
@@ -2991,6 +3063,8 @@  out:
 	}
 #endif
 	free_netdev(dev);
+out_free_pd:
+	kfree(pd);
 
 	return err;
 }
@@ -3030,6 +3104,14 @@  static void mv643xx_eth_shutdown(struct platform_device *pdev)
 		port_reset(mp);
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mv643xx", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_driver = {
 	.probe		= mv643xx_eth_probe,
 	.remove		= mv643xx_eth_remove,
@@ -3037,6 +3119,7 @@  static struct platform_driver mv643xx_eth_driver = {
 	.driver = {
 		.name	= MV643XX_ETH_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_eth_dt_ids),
 	},
 };