[PATCH 2.5] : i82365 & platform_bus_type

[PATCH 2.5] : i82365 & platform_bus_type

[Jean Tourrilhes]

	Hi,

	I'm trying to get i82365 to work again, because I need to test
802.11 drivers (hint : most Pcmcia carrier sold alongside 802.11 cards
are 16 bits only).
	Trying to make the driver compile and load in 2.5.63, I've
made the fix bellow. Seeing the obviousness of the bug, it seems that
nobody is using modules anymore ;-)
-------------------------------------------------
diff -u -p linux/drivers/base/platform.c.original linux/drivers/base/platform.c
--- linux/drivers/base/platform.c.original      Mon Mar  3 17:01:17 2003
+++ linux/drivers/base/platform.c       Mon Mar  3 17:02:22 2003
@@ -60,5 +60,6 @@ static int __init platform_bus_init(void
 
 postcore_initcall(platform_bus_init);
 
+EXPORT_SYMBOL(platform_bus_type);
 EXPORT_SYMBOL(platform_device_register);
 EXPORT_SYMBOL(platform_device_unregister);

-------------------------------------------------

	It now loads, but ends up in :
---------------------------------------
Linux Kernel Card Services 3.1.22
  options:  [pci] [cardbus] [pm]
Intel PCIC probe: 
  Vadem VG-469 ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
    host opts [0]: none
    host opts [1]: none
    ISA irqs (scanned) = 4,5 polling interval = 1000 ms
ds: no socket drivers loaded!
---------------------------------------

	<Rant about post-freeze deleted>

	Have fun...

	Jean

[PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Dominik Brodowski]

Hi,

On Mon, Mar 03, 2003 at 05:30:20PM -0800, Jean Tourrilhes wrote:
> 	Hi,
> 
> 	I'm trying to get i82365 to work again, because I need to test
<snip>
> Intel PCIC probe: 
>   Vadem VG-469 ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
>     host opts [0]: none
>     host opts [1]: none
>     ISA irqs (scanned) = 4,5 polling interval = 1000 ms
> ds: no socket drivers loaded!

Sorry about that -- I mixed up the ordering of initializing the class data
and registering the platform device. Here's a bugfix for the three pcmcia
socket drivers that are platform devices.

Please apply,
	Dominik

diff -ruN linux-original/drivers/pcmcia/hd64465_ss.c linux/drivers/pcmcia/hd64465_ss.c
--- linux-original/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:27:06.000000000 +0100
+++ linux/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:30:37.000000000 +0100
@@ -1070,8 +1070,8 @@
 	}
 
 /*	hd64465_io_debug = 0; */
-	platform_device_register(&hd64465_device);
 	hd64465_device.dev.class_data = &hd64465_data;
+	platform_device_register(&hd64465_device);
 
 	return 0;
 }
diff -ruN linux-original/drivers/pcmcia/i82365.c linux/drivers/pcmcia/i82365.c
--- linux-original/drivers/pcmcia/i82365.c	2003-03-04 08:27:06.000000000 +0100
+++ linux/drivers/pcmcia/i82365.c	2003-03-04 08:28:28.000000000 +0100
@@ -1628,11 +1628,11 @@
 	request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
 #endif
     
-    platform_device_register(&i82365_device);
-
     i82365_data.nsock = sockets;
     i82365_device.dev.class_data = &i82365_data;
     
+    platform_device_register(&i82365_device);
+
     /* Finally, schedule a polling interrupt */
     if (poll_interval != 0) {
 	poll_timer.function = pcic_interrupt_wrapper;
diff -ruN linux-original/drivers/pcmcia/tcic.c linux/drivers/pcmcia/tcic.c
--- linux-original/drivers/pcmcia/tcic.c	2003-03-04 08:27:06.000000000 +0100
+++ linux/drivers/pcmcia/tcic.c	2003-03-04 08:30:03.000000000 +0100
@@ -452,8 +452,6 @@
 	sockets++;
     }
 
-    platform_device_register(&tcic_device);
-
     switch (socket_table[0].id) {
     case TCIC_ID_DB86082:
 	printk("DB86082"); break;
@@ -527,6 +525,8 @@
     tcic_data.nsock = sockets;
     tcic_device.dev.class_data = &tcic_data;
 
+    platform_device_register(&tcic_device);
+
     return 0;
     
 } /* init_tcic */

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Dominik Brodowski]

On Tue, Mar 04, 2003 at 10:54:28AM -0800, Jean Tourrilhes wrote:
> On Tue, Mar 04, 2003 at 11:48:22AM -0600, Patrick Mochel wrote:
> > 
> > Surely you're sore that your code has required some modifications since
> > Dominik has started working on PCMCIA, and I'm sure that no harm was
> > intended. It's had some bumps, but IMO, he's done a great job, and the 
> > result is a vast improvement. The least you could is give the guy some 
> > slack, instead of whining about your own inconveniences. 
> 
> 	I don't mind the changes, changes are usually good. In 2.5.X,
> I had to change my code to accomodate the new PCI interface, the
> removal of global IRQ, the new module interface, the various USB API
> changes and other changes. And actually, your work currently hasn't
> had any impact on the source code I follow (yet).
> 	What I mind is the lack of basic testing. From your patch, the
> initialisation order mixup and the other obvious bug fix I sent you,
> this code had zero chances of working at all and it's obvious that
> nobody bothered to check if it could work or not for at least two
> kernel releases.

The problem is that I only have one yenta-compatible cardbus controller, and
one pcmcia card -- no real "infrastructure" to test the patches.[*] And I
really try to verify that my patches work, but obviously I had a bad day
when I wrote the "let's add a pcmcia socket devices class" patch.
Nonetheless, one point you mention is perfectly valid -- the kernel is in 
a feature freeze.

	Dominik

[*] Needless to say, the patches I sent tend to work on the hardware I
own...

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Jean Tourrilhes]

On Tue, Mar 04, 2003 at 11:48:22AM -0600, Patrick Mochel wrote:
> 
> Surely you're sore that your code has required some modifications since
> Dominik has started working on PCMCIA, and I'm sure that no harm was
> intended. It's had some bumps, but IMO, he's done a great job, and the 
> result is a vast improvement. The least you could is give the guy some 
> slack, instead of whining about your own inconveniences. 

	I don't mind the changes, changes are usually good. In 2.5.X,
I had to change my code to accomodate the new PCI interface, the
removal of global IRQ, the new module interface, the various USB API
changes and other changes. And actually, your work currently hasn't
had any impact on the source code I follow (yet).
	What I mind is the lack of basic testing. From your patch, the
initialisation order mixup and the other obvious bug fix I sent you,
this code had zero chances of working at all and it's obvious that
nobody bothered to check if it could work or not for at least two
kernel releases.
	Yeah, I know that I should not complain because at least the
code did compile (modulo other minor obvious fixes that are already in
Linus BK).

> Thanks,
> 
> 	-pat

	Have fun...

	Jean

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Patrick Mochel]

On Tue, 4 Mar 2003, Jean Tourrilhes wrote:

> On Tue, Mar 04, 2003 at 08:35:05AM -0600, Patrick Mochel wrote:
> > 
> > This patch is completley untested, but it should work. 
> 
> 	I think you are hitting the nail on the head, various people
> are doing many "obvious" changes all over the place without bothering
> to check them, resulting in non functional code.
> 	That's not the way I want to work, and that's why I won't
> commit the new Wireless Extensions stuff until I can really test it.

Whoa, calm down. 

The patch was meant essentially for Dominik only, so he could verify that
it worked with his new PCMCIA code. The only reason the patch is untested
is because I don't have the hardware in question.

The reason the patch was unwritten in the first place is because, as I
mentioned in the previous email, no one has used this feature. Dominik
followed the usage model exactly, and the feature was simple, and
predefined. That's the only reason it was obvious, and sent in the first
place.

Surely you're sore that your code has required some modifications since
Dominik has started working on PCMCIA, and I'm sure that no harm was
intended. It's had some bumps, but IMO, he's done a great job, and the 
result is a vast improvement. The least you could is give the guy some 
slack, instead of whining about your own inconveniences. 

Thanks,

	-pat

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Jean Tourrilhes]

On Tue, Mar 04, 2003 at 08:35:05AM -0600, Patrick Mochel wrote:
> 
> This patch is completley untested, but it should work. 

	I think you are hitting the nail on the head, various people
are doing many "obvious" changes all over the place without bothering
to check them, resulting in non functional code.
	That's not the way I want to work, and that's why I won't
commit the new Wireless Extensions stuff until I can really test it.

> 	-pat

	Thanks.

	Jean

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Patrick Mochel]

On Tue, 4 Mar 2003, Dominik Brodowski wrote:

> Hi Pat,
> 
> How is it supposed to work then? I thought adding a platform_device and
> platform_driver with the same name and bus_id causes the platform_driver to
> be bound to the platform_device?

Erm yes. Color me lazy, I just hadn't implemented that yet.. You've hit 
something else no one had used before. 

This patch is completley untested, but it should work. 

	-pat

===== drivers/base/platform.c 1.5 vs edited =====
--- 1.5/drivers/base/platform.c	Fri Oct 18 13:27:29 2002
+++ edited/drivers/base/platform.c	Tue Mar  4 08:34:10 2003
@@ -41,9 +41,29 @@
 	if (pdev)
 		device_unregister(&pdev->dev);
 }
-	
+
+
+/**
+ *	platform_match - bind platform device to platform driver.
+ *	@dev:	device.
+ *	@drv:	driver.
+ *
+ *	Platform device IDs are assumed to be encoded like this: 
+ *	"<name><instance>", where <name> is a short description of the 
+ *	type of device, like "pci" or "floppy", and <instance> is the 
+ *	enumerated instance of the device, like '0' or '42'.
+ *	Driver IDs are simply "<name>". 
+ *	So, extract the <name> from the device, and compare it against 
+ *	the name of the driver. Return whether they match or not.
+ */
+
 static int platform_match(struct device * dev, struct device_driver * drv)
 {
+	char name[BUS_ID_SIZE];
+
+	if (sscanf(dev->bus_id,"%s",name))
+		return (strcmp(name,drv->name) == 0);
+
 	return 0;
 }
 

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[Dominik Brodowski]

Hi Pat,

How is it supposed to work then? I thought adding a platform_device and
platform_driver with the same name and bus_id causes the platform_driver to
be bound to the platform_device?

Thanks,
	Dominik

On Tue, Mar 04, 2003 at 10:30:31AM +0200, mika.penttila@kolumbus.fi wrote:
> I think the problem is platform_match() :
> 
> static int platform_match(struct device * dev, struct device_driver * drv)
> {
> 	return 0;
> }
> 
> which effectively makes driver binding impossible, pcmcia_socket_class->add_device isn't called.
> 
> --Mika
> 
> 
> > 
> > Lähettäjä: Dominik Brodowski <linux@brodo.de>
> > Päiväys: 2003/03/04 ti AM 09:39:15 GMT+02:00
> > Vastaanottaja: torvalds@transmeta.com,  jt@hpl.hp.com
> > Kopio: Linux kernel mailing list <linux-kernel@vger.kernel.org>, 
> > 	Patrick Mochel <mochel@osdl.org>
> > Aihe: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]
> > 
> > Hi,
> > 
> > On Mon, Mar 03, 2003 at 05:30:20PM -0800, Jean Tourrilhes wrote:
> > > 	Hi,
> > > 
> > > 	I'm trying to get i82365 to work again, because I need to test
> > <snip>
> > > Intel PCIC probe: 
> > >   Vadem VG-469 ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
> > >     host opts [0]: none
> > >     host opts [1]: none
> > >     ISA irqs (scanned) = 4,5 polling interval = 1000 ms
> > > ds: no socket drivers loaded!
> > 
> > Sorry about that -- I mixed up the ordering of initializing the class data
> > and registering the platform device. Here's a bugfix for the three pcmcia
> > socket drivers that are platform devices.
> > 
> > Please apply,
> > 	Dominik
> > 
> > diff -ruN linux-original/drivers/pcmcia/hd64465_ss.c linux/drivers/pcmcia/hd64465_ss.c
> > --- linux-original/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:27:06.000000000 +0100
> > +++ linux/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:30:37.000000000 +0100
> > @@ -1070,8 +1070,8 @@
> >  	}
> >  
> >  /*	hd64465_io_debug = 0; */
> > -	platform_device_register(&hd64465_device);
> >  	hd64465_device.dev.class_data = &hd64465_data;
> > +	platform_device_register(&hd64465_device);
> >  
> >  	return 0;
> >  }
> > diff -ruN linux-original/drivers/pcmcia/i82365.c linux/drivers/pcmcia/i82365.c
> > --- linux-original/drivers/pcmcia/i82365.c	2003-03-04 08:27:06.000000000 +0100
> > +++ linux/drivers/pcmcia/i82365.c	2003-03-04 08:28:28.000000000 +0100
> > @@ -1628,11 +1628,11 @@
> >  	request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
> >  #endif
> >      
> > -    platform_device_register(&i82365_device);
> > -
> >      i82365_data.nsock = sockets;
> >      i82365_device.dev.class_data = &i82365_data;
> >      
> > +    platform_device_register(&i82365_device);
> > +
> >      /* Finally, schedule a polling interrupt */
> >      if (poll_interval != 0) {
> >  	poll_timer.function = pcic_interrupt_wrapper;
> > diff -ruN linux-original/drivers/pcmcia/tcic.c linux/drivers/pcmcia/tcic.c
> > --- linux-original/drivers/pcmcia/tcic.c	2003-03-04 08:27:06.000000000 +0100
> > +++ linux/drivers/pcmcia/tcic.c	2003-03-04 08:30:03.000000000 +0100
> > @@ -452,8 +452,6 @@
> >  	sockets++;
> >      }
> >  
> > -    platform_device_register(&tcic_device);
> > -
> >      switch (socket_table[0].id) {
> >      case TCIC_ID_DB86082:
> >  	printk("DB86082"); break;
> > @@ -527,6 +525,8 @@
> >      tcic_data.nsock = sockets;
> >      tcic_device.dev.class_data = &tcic_data;
> >  
> > +    platform_device_register(&tcic_device);
> > +
> >      return 0;
> >      
> >  } /* init_tcic */
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
> 

Re: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]

[mika.penttila]

I think the problem is platform_match() :

static int platform_match(struct device * dev, struct device_driver * drv)
{
	return 0;
}

which effectively makes driver binding impossible, pcmcia_socket_class->add_device isn't called.

--Mika


> 
> Lähettäjä: Dominik Brodowski <linux@brodo.de>
> Päiväys: 2003/03/04 ti AM 09:39:15 GMT+02:00
> Vastaanottaja: torvalds@transmeta.com,  jt@hpl.hp.com
> Kopio: Linux kernel mailing list <linux-kernel@vger.kernel.org>, 
> 	Patrick Mochel <mochel@osdl.org>
> Aihe: [PATCH] pcmcia: get initialization ordering right [Was: [PATCH 2.5] : i82365 & platform_bus_type]
> 
> Hi,
> 
> On Mon, Mar 03, 2003 at 05:30:20PM -0800, Jean Tourrilhes wrote:
> > 	Hi,
> > 
> > 	I'm trying to get i82365 to work again, because I need to test
> <snip>
> > Intel PCIC probe: 
> >   Vadem VG-469 ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
> >     host opts [0]: none
> >     host opts [1]: none
> >     ISA irqs (scanned) = 4,5 polling interval = 1000 ms
> > ds: no socket drivers loaded!
> 
> Sorry about that -- I mixed up the ordering of initializing the class data
> and registering the platform device. Here's a bugfix for the three pcmcia
> socket drivers that are platform devices.
> 
> Please apply,
> 	Dominik
> 
> diff -ruN linux-original/drivers/pcmcia/hd64465_ss.c linux/drivers/pcmcia/hd64465_ss.c
> --- linux-original/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:27:06.000000000 +0100
> +++ linux/drivers/pcmcia/hd64465_ss.c	2003-03-04 08:30:37.000000000 +0100
> @@ -1070,8 +1070,8 @@
>  	}
>  
>  /*	hd64465_io_debug = 0; */
> -	platform_device_register(&hd64465_device);
>  	hd64465_device.dev.class_data = &hd64465_data;
> +	platform_device_register(&hd64465_device);
>  
>  	return 0;
>  }
> diff -ruN linux-original/drivers/pcmcia/i82365.c linux/drivers/pcmcia/i82365.c
> --- linux-original/drivers/pcmcia/i82365.c	2003-03-04 08:27:06.000000000 +0100
> +++ linux/drivers/pcmcia/i82365.c	2003-03-04 08:28:28.000000000 +0100
> @@ -1628,11 +1628,11 @@
>  	request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
>  #endif
>      
> -    platform_device_register(&i82365_device);
> -
>      i82365_data.nsock = sockets;
>      i82365_device.dev.class_data = &i82365_data;
>      
> +    platform_device_register(&i82365_device);
> +
>      /* Finally, schedule a polling interrupt */
>      if (poll_interval != 0) {
>  	poll_timer.function = pcic_interrupt_wrapper;
> diff -ruN linux-original/drivers/pcmcia/tcic.c linux/drivers/pcmcia/tcic.c
> --- linux-original/drivers/pcmcia/tcic.c	2003-03-04 08:27:06.000000000 +0100
> +++ linux/drivers/pcmcia/tcic.c	2003-03-04 08:30:03.000000000 +0100
> @@ -452,8 +452,6 @@
>  	sockets++;
>      }
>  
> -    platform_device_register(&tcic_device);
> -
>      switch (socket_table[0].id) {
>      case TCIC_ID_DB86082:
>  	printk("DB86082"); break;
> @@ -527,6 +525,8 @@
>      tcic_data.nsock = sockets;
>      tcic_device.dev.class_data = &tcic_data;
>  
> +    platform_device_register(&tcic_device);
> +
>      return 0;
>      
>  } /* init_tcic */
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>