[PATCH,CFT] dynamic struct mii_bus allocation

[PATCH,CFT] dynamic struct mii_bus allocation

[Lennert Buytenhek]

Hi *,

You're listed as maintainer of one of the network drivers in the tree
that use phylib.  Available at the URL below is a change to the phylib
API (dynamic allocation of struct mii_bus, which is needed for hooking
up mdio buses into the device tree) that needs corresponding mdio bus
driver changes.  I've patched all mdio bus drivers I could find, and
tried not to break anything, but it's possible I might have
inadvertently broken something, so I'd like you to test these changes
and let me know if they work for you or not:

	git://git.marvell.com/phylib.git master

As a side-effect of the last patch, you should end up with a list of
mdio buses in your system in /sys/class/mdio_bus.


thanks,
Lennert


The following changes since commit e69c4e0f1210450841e40716894ba6a877b31d52:
  Vlad Yasevich (1):
        sctp: correctly save sctp_adaptation from parameter.

are available in the git repository at:

  git://git.marvell.com/phylib.git master

Lennert Buytenhek (5):
      phylib: phy_mii_ioctl() fixes
      phylib: add mdiobus_{read,write}
      phylib: rename mii_bus::dev to mii_bus::parent
      phylib: move to dynamic allocation of struct mii_bus
      phylib: give mdio buses a device tree presence

 arch/powerpc/platforms/82xx/ep8248e.c     |    2 +-
 arch/powerpc/platforms/pasemi/gpio_mdio.c |    6 +-
 drivers/net/au1000_eth.c                  |   43 ++++++---
 drivers/net/au1000_eth.h                  |    2 +-
 drivers/net/bfin_mac.c                    |   31 ++++---
 drivers/net/bfin_mac.h                    |    2 +-
 drivers/net/cpmac.c                       |   51 ++++++----
 drivers/net/fec_mpc52xx_phy.c             |    8 +-
 drivers/net/fs_enet/mii-bitbang.c         |    9 +-
 drivers/net/fs_enet/mii-fec.c             |    8 +-
 drivers/net/gianfar_mii.c                 |    9 +-
 drivers/net/macb.c                        |   49 ++++++----
 drivers/net/macb.h                        |    2 +-
 drivers/net/mv643xx_eth.c                 |   32 ++++---
 drivers/net/phy/fixed.c                   |   29 ++++--
 drivers/net/phy/mdio-bitbang.c            |    4 +-
 drivers/net/phy/mdio-ofgpio.c             |   11 +-
 drivers/net/phy/mdio_bus.c                |  144 ++++++++++++++++++++++++++++-
 drivers/net/phy/phy.c                     |   54 +----------
 drivers/net/sb1250-mac.c                  |   36 +++++---
 drivers/net/sh_eth.c                      |    4 +-
 drivers/net/tc35815.c                     |   45 ++++++----
 drivers/net/tg3.c                         |   66 +++++++-------
 drivers/net/tg3.h                         |    2 +-
 drivers/net/ucc_geth_mii.c                |    9 +-
 include/linux/phy.h                       |   54 ++++++++++--
 26 files changed, 456 insertions(+), 256 deletions(-)

Re: [PATCH,CFT] dynamic struct mii_bus allocation

[Laurent Pinchart]

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

Hi Lennert,

On Thursday 02 October 2008, Lennert Buytenhek wrote:
> Hi *,
> 
> You're listed as maintainer of one of the network drivers in the tree
> that use phylib.  Available at the URL below is a change to the phylib
> API (dynamic allocation of struct mii_bus, which is needed for hooking
> up mdio buses into the device tree) that needs corresponding mdio bus
> driver changes.  I've patched all mdio bus drivers I could find, and
> tried not to break anything, but it's possible I might have
> inadvertently broken something, so I'd like you to test these changes
> and let me know if they work for you or not:
> 
> 	git://git.marvell.com/phylib.git master
> 
> As a side-effect of the last patch, you should end up with a list of
> mdio buses in your system in /sys/class/mdio_bus.
> 
> 
> thanks,
> Lennert
> 
> 
> The following changes since commit e69c4e0f1210450841e40716894ba6a877b31d52:
>   Vlad Yasevich (1):
>         sctp: correctly save sctp_adaptation from parameter.
> 
> are available in the git repository at:
> 
>   git://git.marvell.com/phylib.git master
> 
> Lennert Buytenhek (5):
>       phylib: phy_mii_ioctl() fixes
>       phylib: add mdiobus_{read,write}
>       phylib: rename mii_bus::dev to mii_bus::parent
>       phylib: move to dynamic allocation of struct mii_bus
>       phylib: give mdio buses a device tree presence
> 
>  arch/powerpc/platforms/82xx/ep8248e.c     |    2 +-
>  arch/powerpc/platforms/pasemi/gpio_mdio.c |    6 +-
>  drivers/net/au1000_eth.c                  |   43 ++++++---
>  drivers/net/au1000_eth.h                  |    2 +-
>  drivers/net/bfin_mac.c                    |   31 ++++---
>  drivers/net/bfin_mac.h                    |    2 +-
>  drivers/net/cpmac.c                       |   51 ++++++----
>  drivers/net/fec_mpc52xx_phy.c             |    8 +-
>  drivers/net/fs_enet/mii-bitbang.c         |    9 +-
>  drivers/net/fs_enet/mii-fec.c             |    8 +-
>  drivers/net/gianfar_mii.c                 |    9 +-
>  drivers/net/macb.c                        |   49 ++++++----
>  drivers/net/macb.h                        |    2 +-
>  drivers/net/mv643xx_eth.c                 |   32 ++++---

Just a side note, the patch "phylib: rename mii_bus::dev to mii_bus::parent" seems to do a lot more than just renaming mii_bus::dev to mii_bus::parent in drivers/net/mv643xx_eth.c. You might have inadvertently committed unrelated changes.

>  drivers/net/phy/fixed.c                   |   29 ++++--
>  drivers/net/phy/mdio-bitbang.c            |    4 +-
>  drivers/net/phy/mdio-ofgpio.c             |   11 +-

Works fine for me. For the mdio-ofgpio part:

Acked-by: Laurent Pinchart <laurentp@cse-semaphore.com>

BTW your "phylib: move to dynamic allocation of struct mii_bus" patch fixes a double free in drivers/net/phy/mdio-ofgpio.c. Thanks for catching this.

>  drivers/net/phy/mdio_bus.c                |  144 ++++++++++++++++++++++++++
>  drivers/net/phy/phy.c                     |   54 +----------
>  drivers/net/sb1250-mac.c                  |   36 +++++---
>  drivers/net/sh_eth.c                      |    4 +-
>  drivers/net/tc35815.c                     |   45 ++++++----
>  drivers/net/tg3.c                         |   66 +++++++-------
>  drivers/net/tg3.h                         |    2 +-
>  drivers/net/ucc_geth_mii.c                |    9 +-
>  include/linux/phy.h                       |   54 ++++++++++--
>  26 files changed, 456 insertions(+), 256 deletions(-)

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH,CFT] dynamic struct mii_bus allocation

[Lennert Buytenhek]

On Fri, Oct 03, 2008 at 11:36:01AM +0200, Laurent Pinchart wrote:

> Hi Lennert,

Hi Laurent,


> > You're listed as maintainer of one of the network drivers in the tree
> > that use phylib.  Available at the URL below is a change to the phylib
> > API (dynamic allocation of struct mii_bus, which is needed for hooking
> > up mdio buses into the device tree) that needs corresponding mdio bus
> > driver changes.  I've patched all mdio bus drivers I could find, and
> > tried not to break anything, but it's possible I might have
> > inadvertently broken something, so I'd like you to test these changes
> > and let me know if they work for you or not:
> > 
> > 	git://git.marvell.com/phylib.git master
> > 
> > As a side-effect of the last patch, you should end up with a list of
> > mdio buses in your system in /sys/class/mdio_bus.
> > 
> > 
> > thanks,
> > Lennert
> > 
> > 
> > The following changes since commit e69c4e0f1210450841e40716894ba6a877b31d52:
> >   Vlad Yasevich (1):
> >         sctp: correctly save sctp_adaptation from parameter.
> > 
> > are available in the git repository at:
> > 
> >   git://git.marvell.com/phylib.git master
> > 
> > Lennert Buytenhek (5):
> >       phylib: phy_mii_ioctl() fixes
> >       phylib: add mdiobus_{read,write}
> >       phylib: rename mii_bus::dev to mii_bus::parent
> >       phylib: move to dynamic allocation of struct mii_bus
> >       phylib: give mdio buses a device tree presence
> > 
> >  arch/powerpc/platforms/82xx/ep8248e.c     |    2 +-
> >  arch/powerpc/platforms/pasemi/gpio_mdio.c |    6 +-
> >  drivers/net/au1000_eth.c                  |   43 ++++++---
> >  drivers/net/au1000_eth.h                  |    2 +-
> >  drivers/net/bfin_mac.c                    |   31 ++++---
> >  drivers/net/bfin_mac.h                    |    2 +-
> >  drivers/net/cpmac.c                       |   51 ++++++----
> >  drivers/net/fec_mpc52xx_phy.c             |    8 +-
> >  drivers/net/fs_enet/mii-bitbang.c         |    9 +-
> >  drivers/net/fs_enet/mii-fec.c             |    8 +-
> >  drivers/net/gianfar_mii.c                 |    9 +-
> >  drivers/net/macb.c                        |   49 ++++++----
> >  drivers/net/macb.h                        |    2 +-
> >  drivers/net/mv643xx_eth.c                 |   32 ++++---
> 
> Just a side note, the patch "phylib: rename mii_bus::dev to
> mii_bus::parent" seems to do a lot more than just renaming mii_bus::dev
> to mii_bus::parent in drivers/net/mv643xx_eth.c. You might have
> inadvertently committed unrelated changes.

What commit ID are you looking at?  I only see this:

	commit def8867a8d2a9f474262dd46179770845a420d51
	Author: Lennert Buytenhek <buytenh@wantstofly.org>
	Date:   Tue Sep 23 02:35:17 2008 +0200

	    phylib: rename mii_bus::dev to mii_bus::parent
	    
	    In preparation of giving mii_bus objects a device tree presence of
	    their own, rename struct mii_bus's ->dev argument to ->parent, since
	    having a 'struct device *dev' that points to our parent device
	    conflicts with introducing a 'struct device dev' representing our own
	    device.
	    
	    Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
	    Acked-by: Andy Fleming <afleming@freescale.com>

	[...]

	diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
	index 372811a..6340081 100644
	--- a/drivers/net/mv643xx_eth.c
	+++ b/drivers/net/mv643xx_eth.c
	@@ -2368,7 +2368,7 @@ static int mv643xx_eth_shared_probe(struct platform_device
			msp->smi_bus.read = smi_bus_read;
			msp->smi_bus.write = smi_bus_write,
			snprintf(msp->smi_bus.id, MII_BUS_ID_SIZE, "%d", pdev->id);
	-               msp->smi_bus.dev = &pdev->dev;
	+               msp->smi_bus.parent = &pdev->dev;
			msp->smi_bus.phy_mask = 0xffffffff;
			if (mdiobus_register(&msp->smi_bus) < 0)
				goto out_unmap;



> >  drivers/net/phy/fixed.c                   |   29 ++++--
> >  drivers/net/phy/mdio-bitbang.c            |    4 +-
> >  drivers/net/phy/mdio-ofgpio.c             |   11 +-
> 
> Works fine for me. For the mdio-ofgpio part:
> 
> Acked-by: Laurent Pinchart <laurentp@cse-semaphore.com>

Thanks!


> BTW your "phylib: move to dynamic allocation of struct mii_bus"
> patch fixes a double free in drivers/net/phy/mdio-ofgpio.c. Thanks
> for catching this.

Yeah, sorry for not reporting that separately.  (I'm not sure if it's
worth fixing separately, since deinit probably doesn't happen very
often.)


cheers,
Lennert

Re: [PATCH,CFT] dynamic struct mii_bus allocation

[Laurent Pinchart]

[-- Attachment #1: Type: text/plain, Size: 5139 bytes --]

On Friday 03 October 2008, Lennert Buytenhek wrote:
> On Fri, Oct 03, 2008 at 11:36:01AM +0200, Laurent Pinchart wrote:
> 
> > Hi Lennert,
> 
> Hi Laurent,
> 
> 
> > > You're listed as maintainer of one of the network drivers in the tree
> > > that use phylib.  Available at the URL below is a change to the phylib
> > > API (dynamic allocation of struct mii_bus, which is needed for hooking
> > > up mdio buses into the device tree) that needs corresponding mdio bus
> > > driver changes.  I've patched all mdio bus drivers I could find, and
> > > tried not to break anything, but it's possible I might have
> > > inadvertently broken something, so I'd like you to test these changes
> > > and let me know if they work for you or not:
> > > 
> > > 	git://git.marvell.com/phylib.git master
> > > 
> > > As a side-effect of the last patch, you should end up with a list of
> > > mdio buses in your system in /sys/class/mdio_bus.
> > > 
> > > 
> > > thanks,
> > > Lennert
> > > 
> > > 
> > > The following changes since commit e69c4e0f1210450841e40716894ba6a877b31d52:
> > >   Vlad Yasevich (1):
> > >         sctp: correctly save sctp_adaptation from parameter.
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.marvell.com/phylib.git master
> > > 
> > > Lennert Buytenhek (5):
> > >       phylib: phy_mii_ioctl() fixes
> > >       phylib: add mdiobus_{read,write}
> > >       phylib: rename mii_bus::dev to mii_bus::parent
> > >       phylib: move to dynamic allocation of struct mii_bus
> > >       phylib: give mdio buses a device tree presence
> > > 
> > >  arch/powerpc/platforms/82xx/ep8248e.c     |    2 +-
> > >  arch/powerpc/platforms/pasemi/gpio_mdio.c |    6 +-
> > >  drivers/net/au1000_eth.c                  |   43 ++++++---
> > >  drivers/net/au1000_eth.h                  |    2 +-
> > >  drivers/net/bfin_mac.c                    |   31 ++++---
> > >  drivers/net/bfin_mac.h                    |    2 +-
> > >  drivers/net/cpmac.c                       |   51 ++++++----
> > >  drivers/net/fec_mpc52xx_phy.c             |    8 +-
> > >  drivers/net/fs_enet/mii-bitbang.c         |    9 +-
> > >  drivers/net/fs_enet/mii-fec.c             |    8 +-
> > >  drivers/net/gianfar_mii.c                 |    9 +-
> > >  drivers/net/macb.c                        |   49 ++++++----
> > >  drivers/net/macb.h                        |    2 +-
> > >  drivers/net/mv643xx_eth.c                 |   32 ++++---
> > 
> > Just a side note, the patch "phylib: rename mii_bus::dev to
> > mii_bus::parent" seems to do a lot more than just renaming mii_bus::dev
> > to mii_bus::parent in drivers/net/mv643xx_eth.c. You might have
> > inadvertently committed unrelated changes.
> 
> What commit ID are you looking at?  I only see this:
> 
> 	commit def8867a8d2a9f474262dd46179770845a420d51
> 	Author: Lennert Buytenhek <buytenh@wantstofly.org>
> 	Date:   Tue Sep 23 02:35:17 2008 +0200
> 
> 	    phylib: rename mii_bus::dev to mii_bus::parent
> 	    
> 	    In preparation of giving mii_bus objects a device tree presence of
> 	    their own, rename struct mii_bus's ->dev argument to ->parent, since
> 	    having a 'struct device *dev' that points to our parent device
> 	    conflicts with introducing a 'struct device dev' representing our own
> 	    device.
> 	    
> 	    Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> 	    Acked-by: Andy Fleming <afleming@freescale.com>
> 
> 	[...]
> 
> 	diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> 	index 372811a..6340081 100644
> 	--- a/drivers/net/mv643xx_eth.c
> 	+++ b/drivers/net/mv643xx_eth.c
> 	@@ -2368,7 +2368,7 @@ static int mv643xx_eth_shared_probe(struct platform_device
> 			msp->smi_bus.read = smi_bus_read;
> 			msp->smi_bus.write = smi_bus_write,
> 			snprintf(msp->smi_bus.id, MII_BUS_ID_SIZE, "%d", pdev->id);
> 	-               msp->smi_bus.dev = &pdev->dev;
> 	+               msp->smi_bus.parent = &pdev->dev;
> 			msp->smi_bus.phy_mask = 0xffffffff;
> 			if (mdiobus_register(&msp->smi_bus) < 0)
> 				goto out_unmap;

My bad, I was looking at my local branch which included a merge conflict resolution. Sorry for the noise.

> > >  drivers/net/phy/fixed.c                   |   29 ++++--
> > >  drivers/net/phy/mdio-bitbang.c            |    4 +-
> > >  drivers/net/phy/mdio-ofgpio.c             |   11 +-
> > 
> > Works fine for me. For the mdio-ofgpio part:
> > 
> > Acked-by: Laurent Pinchart <laurentp@cse-semaphore.com>
> 
> Thanks!
> 
> 
> > BTW your "phylib: move to dynamic allocation of struct mii_bus"
> > patch fixes a double free in drivers/net/phy/mdio-ofgpio.c. Thanks
> > for catching this.
> 
> Yeah, sorry for not reporting that separately.  (I'm not sure if it's
> worth fixing separately, since deinit probably doesn't happen very
> often.)

No worries. This is not critical, so I'm happy to let your patch fix the bug.

Cheers,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH,CFT] dynamic struct mii_bus allocation

[Vitaly Bordug]

On Thu, Oct 02, 2008 at 12:15 +0200, Lennert Buytenhek wrote:
> Hi *,
> 
> You're listed as maintainer of one of the network drivers in the tree
> that use phylib.  Available at the URL below is a change to the phylib
> API (dynamic allocation of struct mii_bus, which is needed for hooking
> up mdio buses into the device tree) that needs corresponding mdio bus
> driver changes.  I've patched all mdio bus drivers I could find, and
> tried not to break anything, but it's possible I might have
> inadvertently broken something, so I'd like you to test these changes
> and let me know if they work for you or not:
> 
> 	git://git.marvell.com/phylib.git master
> 
> As a side-effect of the last patch, you should end up with a list of
> mdio buses in your system in /sys/class/mdio_bus.
> 
> 
> thanks,
> Lennert
> 
> 
> The following changes since commit e69c4e0f1210450841e40716894ba6a877b31d52:
>   Vlad Yasevich (1):
>         sctp: correctly save sctp_adaptation from parameter.
> 
> are available in the git repository at:
> 
>   git://git.marvell.com/phylib.git master
> 
> Lennert Buytenhek (5):
>       phylib: phy_mii_ioctl() fixes
>       phylib: add mdiobus_{read,write}
>       phylib: rename mii_bus::dev to mii_bus::parent
>       phylib: move to dynamic allocation of struct mii_bus
>       phylib: give mdio buses a device tree presence

looks good, thanks!

Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>

> 
>  arch/powerpc/platforms/82xx/ep8248e.c     |    2 +-
>  arch/powerpc/platforms/pasemi/gpio_mdio.c |    6 +-
>  drivers/net/au1000_eth.c                  |   43 ++++++---
>  drivers/net/au1000_eth.h                  |    2 +-
>  drivers/net/bfin_mac.c                    |   31 ++++---
>  drivers/net/bfin_mac.h                    |    2 +-
>  drivers/net/cpmac.c                       |   51 ++++++----
>  drivers/net/fec_mpc52xx_phy.c             |    8 +-
>  drivers/net/fs_enet/mii-bitbang.c         |    9 +-
>  drivers/net/fs_enet/mii-fec.c             |    8 +-
>  drivers/net/gianfar_mii.c                 |    9 +-
>  drivers/net/macb.c                        |   49 ++++++----
>  drivers/net/macb.h                        |    2 +-
>  drivers/net/mv643xx_eth.c                 |   32 ++++---
>  drivers/net/phy/fixed.c                   |   29 ++++--
>  drivers/net/phy/mdio-bitbang.c            |    4 +-
>  drivers/net/phy/mdio-ofgpio.c             |   11 +-
>  drivers/net/phy/mdio_bus.c                |  144 ++++++++++++++++++++++++++++-
>  drivers/net/phy/phy.c                     |   54 +----------
>  drivers/net/sb1250-mac.c                  |   36 +++++---
>  drivers/net/sh_eth.c                      |    4 +-
>  drivers/net/tc35815.c                     |   45 ++++++----
>  drivers/net/tg3.c                         |   66 +++++++-------
>  drivers/net/tg3.h                         |    2 +-
>  drivers/net/ucc_geth_mii.c                |    9 +-
>  include/linux/phy.h                       |   54 ++++++++++--
>  26 files changed, 456 insertions(+), 256 deletions(-)