[PATCH] Fix libata resource conflict for legacy mode

[PATCH] Fix libata resource conflict for legacy mode

[Arnaud Patard]

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

Hi,

Someone reported a bug about not being able to use his box with ICH6
chipset unless booting with PnP disabled on the kernel command-line.

When failing, he had in the logs :

ata_piix 0000:00:1f.2: MAP [ P0 P2 IDE IDE ]
ACPI: PCI Interrupt 0000:00:1f.2[C] -> GSI 20 (level, low) -> IRQ 21
ata: 0x1f0 IDE port busy


After some debugging, it turns out that with PnP activated the resources
are looking like this : 

0100-01fe : pnp 00:09
  0170-0177 : libata
  01f0-01f7 : libata

Thus, with the check done in libata, it finds "pnp 00:09" instead of
"libata" and fails.
The quick fix I made for this is to check wether the resource has a
child or not and then check which child is conflicting.

This patch was tested on 2.6.17 but I rediffed it for 2.6.18-git as the
same bug will likely occur. Of course, if preferred, I can rediff it for
libata#upstream.
Also, I added some printks to print the name of the conflicting resource
but this can also be removed if preferred.


Regards,
Arnaud


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 2618git_libata_vs_pnp.patch --]
[-- Type: text/x-patch, Size: 1739 bytes --]

When the libata is trying to handle legacy ide ports (0x1f0 for instance), it
doesn't take care if the resource has childs or not. 
The result is that this situation :
0100-01fe : pnp 00:09
  0170-0177 : libata
  01f0-01f7 : libata

is seen as conflict, which is wrong. The 'pnp 00:09' part is there due to the 
PnP support. If PnP support is off, this doesn't appear.

The proposed fix is to detect childs and in this case, look at which child is
conflicting.

Signed-off-by: Arnaud Patard <apatard@mandriva.com>
---

Index: linux-2.6/drivers/scsi/libata-bmdma.c
===================================================================
--- linux-2.6.orig/drivers/scsi/libata-bmdma.c
+++ linux-2.6/drivers/scsi/libata-bmdma.c
@@ -1016,10 +1016,13 @@ int ata_pci_init_one (struct pci_dev *pd
 			res.start = 0x1f0;
 			res.end = 0x1f0 + 8 - 1;
 			conflict = ____request_resource(&ioport_resource, &res);
+			if (conflict->child)
+				conflict = ____request_resource(conflict,&res);
 			if (!strcmp(conflict->name, "libata"))
 				legacy_mode |= (1 << 0);
 			else {
 				disable_dev_on_err = 0;
+				printk(KERN_WARNING "ata: conflict with %s\n",conflict->name);
 				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n");
 			}
 		} else
@@ -1030,10 +1033,13 @@ int ata_pci_init_one (struct pci_dev *pd
 			res.start = 0x170;
 			res.end = 0x170 + 8 - 1;
 			conflict = ____request_resource(&ioport_resource, &res);
+			if (conflict->child)
+				conflict = ____request_resource(conflict,&res);
 			if (!strcmp(conflict->name, "libata"))
 				legacy_mode |= (1 << 1);
 			else {
 				disable_dev_on_err = 0;
+				printk(KERN_WARNING "ata: conflict with %s\n",conflict->name);
 				printk(KERN_WARNING "ata: 0x170 IDE port busy\n");
 			}
 		} else

Re: [PATCH] Fix libata resource conflict for legacy mode

[Tejun Heo]

Hello,

Arnaud Patard wrote:
> 0100-01fe : pnp 00:09
>   0170-0177 : libata
>   01f0-01f7 : libata

Arghh...

[--snip--]
> Index: linux-2.6/drivers/scsi/libata-bmdma.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/libata-bmdma.c
> +++ linux-2.6/drivers/scsi/libata-bmdma.c
> @@ -1016,10 +1016,13 @@ int ata_pci_init_one (struct pci_dev *pd
>  			res.start = 0x1f0;
>  			res.end = 0x1f0 + 8 - 1;
>  			conflict = ____request_resource(&ioport_resource, &res);
> +			if (conflict->child)
> +				conflict = ____request_resource(conflict,&res);

IMHO, having something which is slightly more generic would be better - 
e.g. looping till no child.

>  			if (!strcmp(conflict->name, "libata"))
>  				legacy_mode |= (1 << 0);
>  			else {
>  				disable_dev_on_err = 0;
> +				printk(KERN_WARNING "ata: conflict with %s\n",conflict->name);
>  				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n");

Please merge two printks into one.  Other than that, I think printing 
conflict name is nice addition.

>  			}
>  		} else

Thanks.

-- 
tejun

Re: [PATCH] Fix libata resource conflict for legacy mode

[Arnaud Patard]

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

Tejun Heo <htejun@gmail.com> writes:

> Hello,

Hi,


>> Index: linux-2.6/drivers/scsi/libata-bmdma.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/scsi/libata-bmdma.c
>> +++ linux-2.6/drivers/scsi/libata-bmdma.c
>> @@ -1016,10 +1016,13 @@ int ata_pci_init_one (struct pci_dev *pd
>>  			res.start = 0x1f0;
>>  			res.end = 0x1f0 + 8 - 1;
>>  			conflict = ____request_resource(&ioport_resource, &res);
>> +			if (conflict->child)
>> +				conflict = ____request_resource(conflict,&res);
>
> IMHO, having something which is slightly more generic would be better - 
> e.g. looping till no child.

Ok. Done in the attached patch.

>
>>  			if (!strcmp(conflict->name, "libata"))
>>  				legacy_mode |= (1 << 0);
>>  			else {
>>  				disable_dev_on_err = 0;
>> +				printk(KERN_WARNING "ata: conflict with %s\n",conflict->name);
>>  				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n");
>
> Please merge two printks into one.  Other than that, I think printing 

Fixed too.


Regards,
Arnaud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 2618git_libata_vs_pnp.patch --]
[-- Type: text/x-patch, Size: 1790 bytes --]

When the libata is trying to handle legacy ide ports (0x1f0 for instance), it
doesn't take care if the resource has childs or not. 
The result is that this situation :
0100-01fe : pnp 00:09
  0170-0177 : libata
  01f0-01f7 : libata

is seen as conflict, which is wrong.
The proposed fix is to detect childs and in this case, look at which child is
conflicting.

Signed-off-by: Arnaud Patard <apatard@mandriva.com>
---
Index: linux-2.6/drivers/scsi/libata-bmdma.c
===================================================================
--- linux-2.6.orig/drivers/scsi/libata-bmdma.c
+++ linux-2.6/drivers/scsi/libata-bmdma.c
@@ -1016,11 +1016,14 @@ int ata_pci_init_one (struct pci_dev *pd
 			res.start = 0x1f0;
 			res.end = 0x1f0 + 8 - 1;
 			conflict = ____request_resource(&ioport_resource, &res);
+			while (conflict->child)
+				conflict = ____request_resource(conflict, &res);
 			if (!strcmp(conflict->name, "libata"))
 				legacy_mode |= (1 << 0);
 			else {
 				disable_dev_on_err = 0;
-				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n");
+				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n" \
+						    "ata: conflict with %s\n", conflict->name);
 			}
 		} else
 			legacy_mode |= (1 << 0);
@@ -1030,11 +1033,14 @@ int ata_pci_init_one (struct pci_dev *pd
 			res.start = 0x170;
 			res.end = 0x170 + 8 - 1;
 			conflict = ____request_resource(&ioport_resource, &res);
+			while (conflict->child)
+				conflict = ____request_resource(conflict, &res);
 			if (!strcmp(conflict->name, "libata"))
 				legacy_mode |= (1 << 1);
 			else {
 				disable_dev_on_err = 0;
-				printk(KERN_WARNING "ata: 0x170 IDE port busy\n");
+				printk(KERN_WARNING "ata: 0x170 IDE port busy\n" \
+						    "ata: conflict with %s\n", conflict->name);
 			}
 		} else
 			legacy_mode |= (1 << 1);

Re: [PATCH] Fix libata resource conflict for legacy mode

[Tejun Heo]

Hello, again.

Arnaud Patard wrote:
> -				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n");
> +				printk(KERN_WARNING "ata: 0x1f0 IDE port busy\n" \
> +						    "ata: conflict with %s\n", conflict->name);

I was thinking something like

"ata: 0x1f0 IDE port busy (conflict with %s)\n"

Other than that,

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun

Re: [PATCH] Fix libata resource conflict for legacy mode

[Jeff Garzik]

Arnaud Patard wrote:
> When the libata is trying to handle legacy ide ports (0x1f0 for instance), it
> doesn't take care if the resource has childs or not. 
> The result is that this situation :
> 0100-01fe : pnp 00:09
>   0170-0177 : libata
>   01f0-01f7 : libata
> 
> is seen as conflict, which is wrong.
> The proposed fix is to detect childs and in this case, look at which child is
> conflicting.
> 
> Signed-off-by: Arnaud Patard <apatard@mandriva.com>


Applied.  For the future, please inline both the patch description, and 
the patch itself.  I had to apply this manually, rather than use the 
usual scripts.

For more info read:
Documentation/SubmittingPatches
http://linux.yyz.us/patch-format.html

	Jeff