[PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Sven Luther]

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

Add Marvell gigabit ethernet hotplug through the northbridge pci id support
for the Pegasos machines.

Signed-off-by: Nicolas DET <det.nicolas@free.fr>
Signed-off-by: Sven Luther <sl@bplan-gmbh.de>

---
This patch is not optimal, as the proper way to handling this is to bring full
hotplug support to the platform drivers, but this would be much more invasive
and far reaching a project. This approach matches the marvell discovery
northbridge pci id to load the driver for the builtin gigabit ethernet
function, and is similar of what is done already using the discover module
loading technique. With more and more distributions moving to using hotplug
only to probe for modules to load, this patch becomes necessary.

Friendly,

Sven Luther

[-- Attachment #2: mv643xx_eth_pcidev_2.6.14-rc5.diff --]
[-- Type: text/plain, Size: 854 bytes --]

diff -Naur a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
--- a/drivers/net/mv643xx_eth.c	2005-10-25 09:20:50.604386000 +0200
+++ b/drivers/net/mv643xx_eth.c	2005-10-25 09:20:14.596386000 +0200
@@ -36,6 +36,10 @@
 #include <linux/udp.h>
 #include <linux/etherdevice.h>
 
+#ifdef CONFIG_PPC_MULTIPLATFORM
+#include <linux/pci.h>
+#endif
+
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/ethtool.h>
@@ -1640,6 +1644,16 @@
 		" and Dale Farnsworth");
 MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
 
+#ifdef CONFIG_PPC_MULTIPLATFORM
+static struct pci_device_id pci_marvell_mv64360[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, pci_marvell_mv64360);
+#endif
+
+
 /*
  * The second part is the low level driver of the gigE ethernet ports.
  */

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Dale Farnsworth]

On Tue, Oct 25, 2005 at 07:51:59AM +0000, Sven Luther wrote:
> Add Marvell gigabit ethernet hotplug through the northbridge pci id support
> for the Pegasos machines.
> 
> Signed-off-by: Nicolas DET <det.nicolas@free.fr>
> Signed-off-by: Sven Luther <sl@bplan-gmbh.de>
> 
> ---
> This patch is not optimal, as the proper way to handling this is to bring full
> hotplug support to the platform drivers, but this would be much more invasive
> and far reaching a project. This approach matches the marvell discovery
> northbridge pci id to load the driver for the builtin gigabit ethernet
> function, and is similar of what is done already using the discover module
> loading technique. With more and more distributions moving to using hotplug
> only to probe for modules to load, this patch becomes necessary.
> 
> Friendly,
> 
> Sven Luther

> diff -Naur a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> --- a/drivers/net/mv643xx_eth.c	2005-10-25 09:20:50.604386000 +0200
> +++ b/drivers/net/mv643xx_eth.c	2005-10-25 09:20:14.596386000 +0200
> @@ -36,6 +36,10 @@
>  #include <linux/udp.h>
>  #include <linux/etherdevice.h>
>  
> +#ifdef CONFIG_PPC_MULTIPLATFORM
> +#include <linux/pci.h>
> +#endif
> +
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/ethtool.h>
> @@ -1640,6 +1644,16 @@
>  		" and Dale Farnsworth");
>  MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
>  
> +#ifdef CONFIG_PPC_MULTIPLATFORM
> +static struct pci_device_id pci_marvell_mv64360[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, pci_marvell_mv64360);
> +#endif
> +
> +
>  /*
>   * The second part is the low level driver of the gigE ethernet ports.
>   */

I don't think this is really dependent on PPC_MULTIPLATFORM,
and the driver also supports 64460.

How about the patch below.
-Dale

Index: linux-2.6.14-rc2-mm1-mv643xx-enet/drivers/net/mv643xx_eth.c
===================================================================
--- linux-2.6.14-rc2-mm1-mv643xx-enet.orig/drivers/net/mv643xx_eth.c
+++ linux-2.6.14-rc2-mm1-mv643xx-enet/drivers/net/mv643xx_eth.c
@@ -39,6 +39,7 @@
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/ethtool.h>
+#include <linux/pci.h>
 #include <asm/io.h>
 #include <asm/types.h>
 #include <asm/pgtable.h>
@@ -1597,6 +1598,14 @@ MODULE_AUTHOR(	"Rabeeh Khoury, Assaf Hof
 		" and Dale Farnsworth");
 MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
 
+static struct pci_device_id pci_mv64xxx_enet[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64460) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, pci_mv64xxx_enet);
+
 /*
  * The second part is the low level driver of the gigE ethernet ports.
  */

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Sven Luther]

On Tue, Oct 25, 2005 at 11:24:34AM -0700, Dale Farnsworth wrote:
> On Tue, Oct 25, 2005 at 07:51:59AM +0000, Sven Luther wrote:
> > Add Marvell gigabit ethernet hotplug through the northbridge pci id support
> > for the Pegasos machines.
> > 
> > Signed-off-by: Nicolas DET <det.nicolas@free.fr>
> > Signed-off-by: Sven Luther <sl@bplan-gmbh.de>
> > 
> > ---
> > This patch is not optimal, as the proper way to handling this is to bring full
> > hotplug support to the platform drivers, but this would be much more invasive
> > and far reaching a project. This approach matches the marvell discovery
> > northbridge pci id to load the driver for the builtin gigabit ethernet
> > function, and is similar of what is done already using the discover module
> > loading technique. With more and more distributions moving to using hotplug
> > only to probe for modules to load, this patch becomes necessary.
> > 
> > Friendly,
> > 
> > Sven Luther
> 
> > diff -Naur a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> > --- a/drivers/net/mv643xx_eth.c	2005-10-25 09:20:50.604386000 +0200
> > +++ b/drivers/net/mv643xx_eth.c	2005-10-25 09:20:14.596386000 +0200
> > @@ -36,6 +36,10 @@
> >  #include <linux/udp.h>
> >  #include <linux/etherdevice.h>
> >  
> > +#ifdef CONFIG_PPC_MULTIPLATFORM
> > +#include <linux/pci.h>
> > +#endif
> > +
> >  #include <linux/bitops.h>
> >  #include <linux/delay.h>
> >  #include <linux/ethtool.h>
> > @@ -1640,6 +1644,16 @@
> >  		" and Dale Farnsworth");
> >  MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
> >  
> > +#ifdef CONFIG_PPC_MULTIPLATFORM
> > +static struct pci_device_id pci_marvell_mv64360[] = {
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
> > +	{ }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(pci, pci_marvell_mv64360);
> > +#endif
> > +
> > +
> >  /*
> >   * The second part is the low level driver of the gigE ethernet ports.
> >   */
> 
> I don't think this is really dependent on PPC_MULTIPLATFORM,
> and the driver also supports 64460.

Ah, well, the PPC_MULTIPLATFORM was in order to not break potentially other
plateforms i know nothing about, and yes, the below patch would be perfect
too.

Friendly,

Sven Luther

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Christoph Hellwig]

> I don't think this is really dependent on PPC_MULTIPLATFORM,
> and the driver also supports 64460.

This is not a PCI driver so adding a device table is wrong.

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Sven Luther]

On Thu, Oct 27, 2005 at 09:26:55PM +0200, Christoph Hellwig wrote:
> > I don't think this is really dependent on PPC_MULTIPLATFORM,
> > and the driver also supports 64460.
> 
> This is not a PCI driver so adding a device table is wrong.

Sure, please provide us with a patch to add hotplug support to those platform
drivers who are not pci :)

In the meantime, this is no worse than how this was done previously, namely
having discover match the northbridge pci id.

Friendly,

Sven Luther

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Nicolas DET]

Le jeudi 27 octobre 2005 à 21:26 +0200, Christoph Hellwig a écrit :
> > I don't think this is really dependent on PPC_MULTIPLATFORM,
> > and the driver also supports 64460.
> 
> This is not a PCI driver so adding a device table is wrong.
> 

I think we all agree ;-).

The point is to have a 'quick' way to get this module automagicly loaded
until we get a proper super duper OFW devices hotplug or whatsoeever
support.

Regards

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Paul Mackerras]

Sven Luther writes:

> +static struct pci_device_id pci_marvell_mv64360[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },

Are there other devices in this northbridge whose drivers might also
want to bind to this PCI id?

What other devices does this northbridge contain?

Paul.

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Sven Luther]

On Sat, Oct 29, 2005 at 02:56:41PM +1000, Paul Mackerras wrote:
> Sven Luther writes:
> 
> > +static struct pci_device_id pci_marvell_mv64360[] = {
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
> 
> Are there other devices in this northbridge whose drivers might also
> want to bind to this PCI id?
> 
> What other devices does this northbridge contain?

Yep, Dale proposed to add : 

+       { PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64460) },

Which i was about to resubmit the patch when i saw the commit, ... Err, no i
misunderstood your question.

Other devices, well, there is the interrupt controller, some dma engine and
the builtin sram, but nothing to my knowledge which would benefit from being
bound to the pci id.

Friendly,

Sven Luther

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Mark A. Greer]

On Sat, Oct 29, 2005 at 02:56:41PM +1000, Paul Mackerras wrote:
> Sven Luther writes:
> 
> > +static struct pci_device_id pci_marvell_mv64360[] = {
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
> 
> Are there other devices in this northbridge whose drivers might also
> want to bind to this PCI id?
> 
> What other devices does this northbridge contain?

It does have a PIC, DMA engine, and SRAM that Sven mentioned but there
are also a serial controller (MPSC), an i2c controller, and a watchdog
timer.  I would guess that the MPSC and i2c drivers, at least, should
use MODULE_DEVICE_TABLE.  If you agree, I'll add it to the list.

Mark

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Sven Luther]

On Mon, Oct 31, 2005 at 12:05:24PM -0700, Mark A. Greer wrote:
> On Sat, Oct 29, 2005 at 02:56:41PM +1000, Paul Mackerras wrote:
> > Sven Luther writes:
> > 
> > > +static struct pci_device_id pci_marvell_mv64360[] = {
> > > +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_MV64360) },
> > 
> > Are there other devices in this northbridge whose drivers might also
> > want to bind to this PCI id?
> > 
> > What other devices does this northbridge contain?
> 
> It does have a PIC, DMA engine, and SRAM that Sven mentioned but there
> are also a serial controller (MPSC), an i2c controller, and a watchdog
> timer.  I would guess that the MPSC and i2c drivers, at least, should
> use MODULE_DEVICE_TABLE.  If you agree, I'll add it to the list.

Yeah, forgot those, we don't use them on the pegasos though. 

Friendly,

Sven Luther

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Paul Mackerras]

Mark A. Greer writes:

> It does have a PIC, DMA engine, and SRAM that Sven mentioned but there
> are also a serial controller (MPSC), an i2c controller, and a watchdog
> timer.  I would guess that the MPSC and i2c drivers, at least, should
> use MODULE_DEVICE_TABLE.  If you agree, I'll add it to the list.

But the point is that if one driver has claimed the device (because
its ID is in its module device table), the other drivers won't be able
to.  You need something like the macio driver that BenH did, which
claims the macio device and creates a macio bus structure and allows
individual macio device drivers to claim the pieces of it.

Paul.

Re: [PATCH 2.6.14-rc5 1/1] : mv643xx_eth_pcidev - implements hotplug for the marvell gige functionality by probing the northbridge pci id.

[Mark A. Greer]

On Tue, Nov 01, 2005 at 08:25:24PM +1100, Paul Mackerras wrote:
> Mark A. Greer writes:
> 
> > It does have a PIC, DMA engine, and SRAM that Sven mentioned but there
> > are also a serial controller (MPSC), an i2c controller, and a watchdog
> > timer.  I would guess that the MPSC and i2c drivers, at least, should
> > use MODULE_DEVICE_TABLE.  If you agree, I'll add it to the list.
> 
> But the point is that if one driver has claimed the device (because
> its ID is in its module device table), the other drivers won't be able
> to.

Okay.  TBH, I still have to look at what the MODULE_DEVICE_TABLE does so
it was ignorance + laziness on my part.

> You need something like the macio driver that BenH did, which
> claims the macio device and creates a macio bus structure and allows
> individual macio device drivers to claim the pieces of it.

I'll take a look.

Mark