* Re: [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation
2014-03-18 23:25 [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation Laurent Pinchart
@ 2014-03-19 16:19 ` Sergei Shtylyov
2014-03-20 13:42 ` Laurent Pinchart
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-03-19 16:19 UTC (permalink / raw)
To: linux-sh
Hello.
On 03/19/2014 02:25 AM, Laurent Pinchart wrote:
> Memory allocated for the MDIO bus with the devm_kzalloc() API is
> associated with the network device. While this will cause memory to be
> freed at the right time, it doesn't allow allocating memory before the
> network device is initialized.
> Replace the network device with the parent platform device for memory
> allocation to remove that dependency. This also improves consistency
> with the other devm_* calls in the driver that all use the platform
> device.
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index be7211d..b80abb6 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2615,10 +2615,10 @@ static int sh_mdio_init(struct net_device *ndev, int id,
> int ret, i;
> struct bb_info *bitbang;
> struct sh_eth_private *mdp = netdev_priv(ndev);
I'd suggest to also declare:
struct platform_device *pdev = &mdp->pdev;
so that you could simplify 'mdev->pdev->name' derefs in the sprintf() call and
to get rid of the 'id' parameter to sh_mdio_init() by using 'pdev->id'
directly in the same call.
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation
2014-03-18 23:25 [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation Laurent Pinchart
2014-03-19 16:19 ` Sergei Shtylyov
@ 2014-03-20 13:42 ` Laurent Pinchart
2014-03-20 13:43 ` Laurent Pinchart
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2014-03-20 13:42 UTC (permalink / raw)
To: linux-sh
Hi Sergei,
On Wednesday 19 March 2014 20:19:51 Sergei Shtylyov wrote:
> On 03/19/2014 02:25 AM, Laurent Pinchart wrote:
> > Memory allocated for the MDIO bus with the devm_kzalloc() API is
> > associated with the network device. While this will cause memory to be
> > freed at the right time, it doesn't allow allocating memory before the
> > network device is initialized.
> >
> > Replace the network device with the parent platform device for memory
> > allocation to remove that dependency. This also improves consistency
> > with the other devm_* calls in the driver that all use the platform
> > device.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c
> > b/drivers/net/ethernet/renesas/sh_eth.c index be7211d..b80abb6 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > @@ -2615,10 +2615,10 @@ static int sh_mdio_init(struct net_device *ndev,
> > int id,>
> > int ret, i;
> > struct bb_info *bitbang;
> > struct sh_eth_private *mdp = netdev_priv(ndev);
>
> I'd suggest to also declare:
>
> struct platform_device *pdev = &mdp->pdev;
>
> so that you could simplify 'mdev->pdev->name' derefs in the sprintf() call
> and to get rid of the 'id' parameter to sh_mdio_init() by using 'pdev->id'
> directly in the same call.
Good idea. I'll do that in patch 2/5 for v2.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation
2014-03-18 23:25 [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation Laurent Pinchart
2014-03-19 16:19 ` Sergei Shtylyov
2014-03-20 13:42 ` Laurent Pinchart
@ 2014-03-20 13:43 ` Laurent Pinchart
2014-03-20 15:28 ` Sergei Shtylyov
2014-03-21 13:02 ` Laurent Pinchart
4 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2014-03-20 13:43 UTC (permalink / raw)
To: linux-sh
On Thursday 20 March 2014 14:42:30 Laurent Pinchart wrote:
> On Wednesday 19 March 2014 20:19:51 Sergei Shtylyov wrote:
> > On 03/19/2014 02:25 AM, Laurent Pinchart wrote:
> > > Memory allocated for the MDIO bus with the devm_kzalloc() API is
> > > associated with the network device. While this will cause memory to be
> > > freed at the right time, it doesn't allow allocating memory before the
> > > network device is initialized.
> > >
> > > Replace the network device with the parent platform device for memory
> > > allocation to remove that dependency. This also improves consistency
> > > with the other devm_* calls in the driver that all use the platform
> > > device.
> > >
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >
> > > drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/sh_eth.c
> > > b/drivers/net/ethernet/renesas/sh_eth.c index be7211d..b80abb6 100644
> > > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > > @@ -2615,10 +2615,10 @@ static int sh_mdio_init(struct net_device *ndev,
> > > int id,>
> > >
> > > int ret, i;
> > > struct bb_info *bitbang;
> > > struct sh_eth_private *mdp = netdev_priv(ndev);
> >
> > I'd suggest to also declare:
> > struct platform_device *pdev = &mdp->pdev;
> >
> > so that you could simplify 'mdev->pdev->name' derefs in the sprintf() call
> > and to get rid of the 'id' parameter to sh_mdio_init() by using 'pdev->id'
> > directly in the same call.
>
> Good idea. I'll do that in patch 2/5 for v2.
I meant 3/5, sorry.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation
2014-03-18 23:25 [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation Laurent Pinchart
` (2 preceding siblings ...)
2014-03-20 13:43 ` Laurent Pinchart
@ 2014-03-20 15:28 ` Sergei Shtylyov
2014-03-21 13:02 ` Laurent Pinchart
4 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-03-20 15:28 UTC (permalink / raw)
To: linux-sh
Hello.
On 20-03-2014 17:43, Laurent Pinchart wrote:
>>> On 03/19/2014 02:25 AM, Laurent Pinchart wrote:
>>>> Memory allocated for the MDIO bus with the devm_kzalloc() API is
>>>> associated with the network device. While this will cause memory to be
>>>> freed at the right time, it doesn't allow allocating memory before the
>>>> network device is initialized.
>>>> Replace the network device with the parent platform device for memory
>>>> allocation to remove that dependency. This also improves consistency
>>>> with the other devm_* calls in the driver that all use the platform
>>>> device.
>>>> Signed-off-by: Laurent Pinchart
>>>> <laurent.pinchart+renesas@ideasonboard.com>
>>>> ---
>>>>
>>>> drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>>>> b/drivers/net/ethernet/renesas/sh_eth.c index be7211d..b80abb6 100644
>>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>>> @@ -2615,10 +2615,10 @@ static int sh_mdio_init(struct net_device *ndev,
>>>> int id,>
>>>>
>>>> int ret, i;
>>>> struct bb_info *bitbang;
>>>> struct sh_eth_private *mdp = netdev_priv(ndev);
>>> I'd suggest to also declare:
>>> struct platform_device *pdev = &mdp->pdev;
>>> so that you could simplify 'mdev->pdev->name' derefs in the sprintf() call
>>> and to get rid of the 'id' parameter to sh_mdio_init() by using 'pdev->id'
>>> directly in the same call.
>> Good idea. I'll do that in patch 2/5 for v2.
> I meant 3/5, sorry.
In fact, I think it would fit better in either this patch or a separate
patch (at least removal of 'id' parameter probably deserves a separate patch).
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation
2014-03-18 23:25 [PATCH/RFC 1/5] sh_eth: Use the platform device for memory allocation Laurent Pinchart
` (3 preceding siblings ...)
2014-03-20 15:28 ` Sergei Shtylyov
@ 2014-03-21 13:02 ` Laurent Pinchart
4 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2014-03-21 13:02 UTC (permalink / raw)
To: linux-sh
Hi Sergei,
On Thursday 20 March 2014 19:28:44 Sergei Shtylyov wrote:
> On 20-03-2014 17:43, Laurent Pinchart wrote:
> >>> On 03/19/2014 02:25 AM, Laurent Pinchart wrote:
> >>>> Memory allocated for the MDIO bus with the devm_kzalloc() API is
> >>>> associated with the network device. While this will cause memory to be
> >>>> freed at the right time, it doesn't allow allocating memory before the
> >>>> network device is initialized.
> >>>>
> >>>> Replace the network device with the parent platform device for memory
> >>>> allocation to remove that dependency. This also improves consistency
> >>>> with the other devm_* calls in the driver that all use the platform
> >>>> device.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>>
> >>>> drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
> >>>> 1 file changed, 3 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
> >>>> b/drivers/net/ethernet/renesas/sh_eth.c index be7211d..b80abb6 100644
> >>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
> >>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> >>>> @@ -2615,10 +2615,10 @@ static int sh_mdio_init(struct net_device
> >>>> *ndev,
> >>>> int id,>
> >>>>
> >>>> int ret, i;
> >>>> struct bb_info *bitbang;
> >>>> struct sh_eth_private *mdp = netdev_priv(ndev);
> >>>
> >>> I'd suggest to also declare:
> >>> struct platform_device *pdev = &mdp->pdev;
> >>>
> >>> so that you could simplify 'mdev->pdev->name' derefs in the sprintf()
> >>> call
> >>> and to get rid of the 'id' parameter to sh_mdio_init() by using
> >>> 'pdev->id'
> >>> directly in the same call.
> >>
> >> Good idea. I'll do that in patch 2/5 for v2.
> >
> > I meant 3/5, sorry.
>
> In fact, I think it would fit better in either this patch or a separate
> patch (at least removal of 'id' parameter probably deserves a separate
> patch).
David has been faster than me and has already applied the series to his tree,
sorry.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread