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
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 */
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 <[email protected]>
> P?iv?ys: 2003/03/04 ti AM 09:39:15 GMT+02:00
> Vastaanottaja: [email protected], [email protected]
> Kopio: Linux kernel mailing list <[email protected]>,
> Patrick Mochel <[email protected]>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
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, [email protected] 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 <[email protected]>
> > P?iv?ys: 2003/03/04 ti AM 09:39:15 GMT+02:00
> > Vastaanottaja: [email protected], [email protected]
> > Kopio: Linux kernel mailing list <[email protected]>,
> > Patrick Mochel <[email protected]>
> > 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
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;
}
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
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
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
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...
Hi Pat,
On Tue, Mar 04, 2003 at 08:35:05AM -0600, Patrick Mochel wrote:
>
> 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.
...
> +
> + if (sscanf(dev->bus_id,"%s",name))
> + return (strcmp(name,drv->name) == 0);
Unfortunately, this won't work: digits are perfectly valid entries of
strings. However, we have the name without the appending instance still
saved in platform_device pdev->name... so what about this?
diff -ruN linux-original/drivers/base/platform.c linux/drivers/base/platform.c
--- linux-original/drivers/base/platform.c 2003-03-05 07:19:19.000000000 +0100
+++ linux/drivers/base/platform.c 2003-03-05 07:22:31.000000000 +0100
@@ -59,12 +59,9 @@
static int platform_match(struct device * dev, struct device_driver * drv)
{
- char name[BUS_ID_SIZE];
+ struct platform_device *pdev = container_of(dev, struct platform_device, dev);
- if (sscanf(dev->bus_id,"%s",name))
- return (strcmp(name,drv->name) == 0);
-
- return 0;
+ return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
}
struct bus_type platform_bus_type = {
> Unfortunately, this won't work: digits are perfectly valid entries of
> strings. However, we have the name without the appending instance still
> saved in platform_device pdev->name... so what about this?
D'oh. You're completely right.
Thanks,
-pat
On Wed, Mar 05, 2003 at 10:16:46AM -0600, Patrick Mochel wrote:
>
> > Unfortunately, this won't work: digits are perfectly valid entries of
> > strings. However, we have the name without the appending instance still
> > saved in platform_device pdev->name... so what about this?
>
> D'oh. You're completely right.
>
> Thanks,
>
>
> -pat
Hi,
I tried 2.5.64-bk9, and it doesn't work !
You simply forgot to apply the patch I sent you at the
very beggining of this discussion :
----------------------------------------------------
diff -u -p linux/drivers/base/platform.c.original linux/drivers/base/platform.c
--- linux/drivers/base/platform.c.original Fri Mar 14 17:47:16 2003
+++ linux/drivers/base/platform.c Fri Mar 14 17:47:40 2003
@@ -75,5 +75,6 @@ int __init platform_bus_init(void)
return bus_register(&platform_bus_type);
}
+EXPORT_SYMBOL(platform_bus_type);
EXPORT_SYMBOL(platform_device_register);
EXPORT_SYMBOL(platform_device_unregister);
----------------------------------------------------
On the other hand, with the above patch, now everything is
working properly and my Pcmcia card work wonderfully.
Thanks a lot for the great work, and have fun...
Jean