[PATCH] pcmcia: ensure correct logging in do_io_probe [v2]

[PATCH] pcmcia: ensure correct logging in do_io_probe [v2]

[Frans Pop]

I've added parens around the conditions. Maybe not strictly needed,
but better readable and consistent with similar usage in cs.c.

Cheers,
FJP

---
From: Frans Pop <elendil@planet.nl>

pcmcia: ensure correct logging in do_io_probe
    
During early probing of the parent PCI bridge I/O window no socket
has been allocated yet.
In that case print &s->cb_dev->dev instead of &s->dev as device.
    
Also properly close previous dev_printk if kzalloc fails.

Signed-off-by: Frans Pop <elendil@planet.nl>
CC: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/drivers/pcmcia/rsrc_nonstatic.c 
b/drivers/pcmcia/rsrc_nonstatic.c
index 17f4ecf..2a6a3d1 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -200,14 +200,17 @@ static void do_io_probe(struct pcmcia_socket *s, 
unsigned int base,
     int any;
     u_char *b, hole, most;
 
-    dev_printk(KERN_INFO, &s->dev, "cs: IO port probe %#x-%#x:",
-	       base, base+num-1);
+    dev_printk(KERN_INFO,
+	       (s->state & SOCKET_PRESENT) ? &s->dev : &s->cb_dev->dev,
+	       "cs: IO port probe %#x-%#x:", base, base+num-1);
 
     /* First, what does a floating port look like? */
     b = kzalloc(256, GFP_KERNEL);
     if (!b) {
-	    dev_printk(KERN_ERR, &s->dev,
-		   "do_io_probe: unable to kmalloc 256 bytes");
+	    printk("\n");
+	    dev_printk(KERN_ERR,
+		   (s->state & SOCKET_PRESENT) ? &s->dev : &s->cb_dev->dev,
+		   "cs: do_io_probe: unable to kmalloc 256 bytes");
             return;
     }
     for (i = base, most = 0; i < base+num; i += 8) {

Re: [PATCH] pcmcia: ensure correct logging in do_io_probe [v2]

[Dominik Brodowski]

Hi,

On Fri, Nov 07, 2008 at 03:22:42PM +0100, Frans Pop wrote:
> I've added parens around the conditions. Maybe not strictly needed,
> but better readable and consistent with similar usage in cs.c.
>
> From: Frans Pop <elendil@planet.nl>
> 
> pcmcia: ensure correct logging in do_io_probe
>     
> During early probing of the parent PCI bridge I/O window no socket
> has been allocated yet.
> In that case print &s->cb_dev->dev instead of &s->dev as device.

similar to the other patch, this won't work: &skt->cb_dev is only available
and set if the parent device is a PCI device; therefore, &skt->dev is the
only thing we can use.

An alternative approach would be to move this initialization code only after
we've set dev_name. Could you test whether this approach works for you,
please?

Thanks,
	Dominik


>From b1769450da0eeae2d95aae5496acbdf4c6ba89b2 Mon Sep 17 00:00:00 2001
From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Sun, 9 Nov 2008 21:47:47 +0100
Subject: [PATCH] pcmcia: ensure correct logging in do_io_probe

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c
index 5d0e60e..0660ad1 100644
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -186,12 +186,6 @@ int pcmcia_register_socket(struct pcmcia_socket *socket)
 
 	spin_lock_init(&socket->lock);
 
-	if (socket->resource_ops->init) {
-		ret = socket->resource_ops->init(socket);
-		if (ret)
-			return (ret);
-	}
-
 	/* try to obtain a socket number [yes, it gets ugly if we
 	 * register more than 2^sizeof(unsigned int) pcmcia
 	 * sockets... but the socket number is deprecated
@@ -239,6 +233,12 @@ int pcmcia_register_socket(struct pcmcia_socket *socket)
 	mutex_init(&socket->skt_mutex);
 	spin_lock_init(&socket->thread_lock);
 
+	if (socket->resource_ops->init) {
+		ret = socket->resource_ops->init(socket);
+		if (ret)
+			goto err;
+	}
+
 	tsk = kthread_run(pccardd, socket, "pccardd");
 	if (IS_ERR(tsk)) {
 		ret = PTR_ERR(tsk);

Re: [PATCH] pcmcia: ensure correct logging in do_io_probe [v2]

[Frans Pop]

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

On Sunday 09 November 2008, Dominik Brodowski wrote:
> On Fri, Nov 07, 2008 at 03:22:42PM +0100, Frans Pop wrote:
> > During early probing of the parent PCI bridge I/O window no socket
> > has been allocated yet.
> > In that case print &s->cb_dev->dev instead of &s->dev as device.
>
> similar to the other patch, this won't work: &skt->cb_dev is only
> available and set if the parent device is a PCI device; therefore,
> &skt->dev is the only thing we can use.

OK.

I still feel I'm missing something in the debug logging. Shouldn't there 
be a line saying something like the following?
yenta_cardbus 0000:02:06.0: Assigned pcmcia socket 0

> An alternative approach would be to move this initialization code only
> after we've set dev_name. Could you test whether this approach works
> for you, please?

Tested on two different systems: Toshiba Satellite A40 running i386 (with 
atheros wlan card already inserted during boot) and HP 2510p running 
x86_64 (with same card inserted after boot).
Both with patch applied against current mainline git. I've not seen any 
problems and wireless worked in both cases.

There are some differences in the logs (see attachments). I only see the 
IO probes on the Toshiba, not on the HP. I'd guess that's expected.

Tested-by: Frans Pop <elendil@planet.nl>


This does leave one minor issue from my previous patch. Please consider 
applying the attached patch.

Cheers,
FJP


[-- Attachment #2: pcmcia_close-prink.patch --]
[-- Type: text/x-diff, Size: 665 bytes --]

From: Frans Pop <elendil@planet.nl>

pcmcia: properly close previous dev_printk if kzalloc fails in do_io_probe

Signed-off-by: Frans Pop <elendil@planet.nl>

diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index 17f4ecf..da95252 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -206,6 +206,7 @@ static void do_io_probe(struct pcmcia_socket *s, unsigned int base,
     /* First, what does a floating port look like? */
     b = kzalloc(256, GFP_KERNEL);
     if (!b) {
+	    printk("\n");
 	    dev_printk(KERN_ERR, &s->dev,
 		   "do_io_probe: unable to kmalloc 256 bytes");
             return;

[-- Attachment #3: pcmcia_hp2510p.log --]
[-- Type: text/x-log, Size: 591 bytes --]

yenta_cardbus 0000:02:06.0: CardBus bridge found [103c:30c9]
yenta_cardbus 0000:02:06.0: ISA IRQ mask 0x0cb8, PCI irq 18
yenta_cardbus 0000:02:06.0: Socket status: 30000006
pci_bus 0000:02: Raising subordinate bus# of parent bus (#02) from #03 to #06
yenta_cardbus 0000:02:06.0: pcmcia: parent PCI bridge I/O window: 0x3000 - 0x3fff
yenta_cardbus 0000:02:06.0: pcmcia: parent PCI bridge Memory window: 0xe0100000 - 0xe03fffff
yenta_cardbus 0000:02:06.0: pcmcia: parent PCI bridge Memory window: 0x80000000 - 0x83ffffff
pcmcia_socket pcmcia_socket0: pccard: CardBus card inserted into slot 0

[-- Attachment #4: pcmcia_toshiba.log --]
[-- Type: text/x-log, Size: 923 bytes --]

yenta_cardbus 0000:01:0b.0: CardBus bridge found [1179:0001]
yenta_cardbus 0000:01:0b.0: ISA IRQ mask 0x0c38, PCI irq 18
yenta_cardbus 0000:01:0b.0: Socket status: 30000020
yenta_cardbus 0000:01:0b.0: pcmcia: parent PCI bridge I/O window: 0xc000 - 0xcfff
pcmcia_socket pcmcia_socket0: cs: IO port probe 0xc000-0xcfff: clean.
yenta_cardbus 0000:01:0b.0: pcmcia: parent PCI bridge Memory window: 0xcff00000 - 0xcfffffff
yenta_cardbus 0000:01:0b.0: pcmcia: parent PCI bridge Memory window: 0x28000000 - 0x2bffffff
pcmcia_socket pcmcia_socket0: pccard: CardBus card inserted into slot 0
pcmcia_socket pcmcia_socket0: cs: IO port probe 0x100-0x3af: clean.
pcmcia_socket pcmcia_socket0: cs: IO port probe 0x3e0-0x4ff: clean.
pcmcia_socket pcmcia_socket0: cs: IO port probe 0x820-0x8ff: clean.
pcmcia_socket pcmcia_socket0: cs: IO port probe 0xc00-0xcf7: clean.
pcmcia_socket pcmcia_socket0: cs: IO port probe 0xa00-0xaff: clean.