[PATCH 1/1] ide: ide-generic, add another device exception

[PATCH 1/1] ide: ide-generic, add another device exception

[Jiri Slaby]

ide-generic, add another device exception

This device is char device and is grabbed by generic ide driver:
00:0b.0 Class ffff: National Semiconductor Corporation 87410 IDE (rev ff) (prog-if ff)
	Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-

Disallow generic driver to grab it by adding next condition. Also
consolidate exceptions to one bigger 'switch (dev->vendor)'.

Cc: Ingo Korb <ml@akana.de>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 879404085754892c27ff2d1c73d310582b872a98
tree abb7f3b7522c4076da230125afa9d34cbf48ff91
parent f6d5d1877b0f7ac82f5e1fec2ead0429eeb069c8
author Jiri Slaby <jirislaby@gmail.com> Mon, 04 Jun 2007 11:19:39 +0200
committer Jiri Slaby <jirislaby@gmail.com> Mon, 04 Jun 2007 11:19:39 +0200

 drivers/ide/pci/generic.c |   37 +++++++++++++++++++++++--------------
 1 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/pci/generic.c b/drivers/ide/pci/generic.c
index f2c5a14..0d51a11 100644
--- a/drivers/ide/pci/generic.c
+++ b/drivers/ide/pci/generic.c
@@ -198,32 +198,41 @@ static ide_pci_device_t generic_chipsets[] __devinitdata = {
 static int __devinit generic_init_one(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	ide_pci_device_t *d = &generic_chipsets[id->driver_data];
-	u16 command;
 	int ret = -ENODEV;
 
 	/* Don't use the generic entry unless instructed to do so */
 	if (id->driver_data == 0 && ide_generic_all == 0)
 			goto out;
 
-	if (dev->vendor == PCI_VENDOR_ID_UMC &&
-	    dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
-	    (!(PCI_FUNC(dev->devfn) & 1)))
-		goto out; /* UM8886A/BF pair */
-
-	if (dev->vendor == PCI_VENDOR_ID_OPTI &&
-	    dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
-	    (!(PCI_FUNC(dev->devfn) & 1)))
-		goto out;
-
-	if (dev->vendor == PCI_VENDOR_ID_JMICRON) {
-		if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 && PCI_FUNC(dev->devfn) != 1)
+	switch (dev->vendor) {
+	case PCI_VENDOR_ID_UMC:
+		if (dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
+				!(PCI_FUNC(dev->devfn) & 1))
+			goto out; /* UM8886A/BF pair */
+		break;
+	case PCI_VENDOR_ID_OPTI:
+		if (dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
+				!(PCI_FUNC(dev->devfn) & 1))
+			goto out;
+		break;
+	case PCI_VENDOR_ID_JMICRON:
+		if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
+				PCI_FUNC(dev->devfn) != 1)
+			goto out;
+		break;
+	case PCI_VENDOR_ID_NS:
+		if (dev->device == PCI_DEVICE_ID_NS_87410 &&
+				(dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
 			goto out;
+		break;
 	}
 
 	if (dev->vendor != PCI_VENDOR_ID_JMICRON) {
+		u16 command;
 		pci_read_config_word(dev, PCI_COMMAND, &command);
 		if (!(command & PCI_COMMAND_IO)) {
-			printk(KERN_INFO "Skipping disabled %s IDE controller.\n", d->name);
+			printk(KERN_INFO "Skipping disabled %s IDE "
+					"controller.\n", d->name);
 			goto out;
 		}
 	}

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Sergei Shtylyov]

Hello.

Jiri Slaby wrote:

> diff --git a/drivers/ide/pci/generic.c b/drivers/ide/pci/generic.c
> index f2c5a14..0d51a11 100644
> --- a/drivers/ide/pci/generic.c
> +++ b/drivers/ide/pci/generic.c
> @@ -198,32 +198,41 @@ static ide_pci_device_t generic_chipsets[] __devinitdata = {
>  static int __devinit generic_init_one(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>  	ide_pci_device_t *d = &generic_chipsets[id->driver_data];
> -	u16 command;
>  	int ret = -ENODEV;
>  
>  	/* Don't use the generic entry unless instructed to do so */
>  	if (id->driver_data == 0 && ide_generic_all == 0)
>  			goto out;
>  
> -	if (dev->vendor == PCI_VENDOR_ID_UMC &&
> -	    dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
> -	    (!(PCI_FUNC(dev->devfn) & 1)))
> -		goto out; /* UM8886A/BF pair */
> -
> -	if (dev->vendor == PCI_VENDOR_ID_OPTI &&
> -	    dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
> -	    (!(PCI_FUNC(dev->devfn) & 1)))
> -		goto out;
> -
> -	if (dev->vendor == PCI_VENDOR_ID_JMICRON) {
> -		if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 && PCI_FUNC(dev->devfn) != 1)
> +	switch (dev->vendor) {
> +	case PCI_VENDOR_ID_UMC:
> +		if (dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
> +				!(PCI_FUNC(dev->devfn) & 1))
> +			goto out; /* UM8886A/BF pair */
> +		break;
> +	case PCI_VENDOR_ID_OPTI:
> +		if (dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
> +				!(PCI_FUNC(dev->devfn) & 1))
> +			goto out;
> +		break;
> +	case PCI_VENDOR_ID_JMICRON:
> +		if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
> +				PCI_FUNC(dev->devfn) != 1)
> +			goto out;
> +		break;
> +	case PCI_VENDOR_ID_NS:
> +		if (dev->device == PCI_DEVICE_ID_NS_87410 &&
> +				(dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
>  			goto out;
> +		break;
>  	}

   Could you intend the if() conditions carried to another line the way they 
were intended before?

MBR, Sergei

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Jiri Slaby]

Sergei Shtylyov napsal(a):
> Hello.
> 
> Jiri Slaby wrote:
> 
>> diff --git a/drivers/ide/pci/generic.c b/drivers/ide/pci/generic.c
>> index f2c5a14..0d51a11 100644
>> --- a/drivers/ide/pci/generic.c
>> +++ b/drivers/ide/pci/generic.c
>> @@ -198,32 +198,41 @@ static ide_pci_device_t generic_chipsets[]
>> __devinitdata = {
>>  static int __devinit generic_init_one(struct pci_dev *dev, const
>> struct pci_device_id *id)
>>  {
>>      ide_pci_device_t *d = &generic_chipsets[id->driver_data];
>> -    u16 command;
>>      int ret = -ENODEV;
>>  
>>      /* Don't use the generic entry unless instructed to do so */
>>      if (id->driver_data == 0 && ide_generic_all == 0)
>>              goto out;
>>  
>> -    if (dev->vendor == PCI_VENDOR_ID_UMC &&
>> -        dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
>> -        (!(PCI_FUNC(dev->devfn) & 1)))
>> -        goto out; /* UM8886A/BF pair */
>> -
>> -    if (dev->vendor == PCI_VENDOR_ID_OPTI &&
>> -        dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
>> -        (!(PCI_FUNC(dev->devfn) & 1)))
>> -        goto out;
>> -
>> -    if (dev->vendor == PCI_VENDOR_ID_JMICRON) {
>> -        if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
>> PCI_FUNC(dev->devfn) != 1)
>> +    switch (dev->vendor) {
>> +    case PCI_VENDOR_ID_UMC:
>> +        if (dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
>> +                !(PCI_FUNC(dev->devfn) & 1))
>> +            goto out; /* UM8886A/BF pair */
>> +        break;
>> +    case PCI_VENDOR_ID_OPTI:
>> +        if (dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
>> +                !(PCI_FUNC(dev->devfn) & 1))
>> +            goto out;
>> +        break;
>> +    case PCI_VENDOR_ID_JMICRON:
>> +        if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
>> +                PCI_FUNC(dev->devfn) != 1)
>> +            goto out;
>> +        break;
>> +    case PCI_VENDOR_ID_NS:
>> +        if (dev->device == PCI_DEVICE_ID_NS_87410 &&
>> +                (dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
>>              goto out;
>> +        break;
>>      }
> 
>   Could you intend the if() conditions carried to another line the way
> they were intended before?

I could, if you tell me the reason.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Sergei Shtylyov]

Hello.

Jiri Slaby wrote:

>>>diff --git a/drivers/ide/pci/generic.c b/drivers/ide/pci/generic.c
>>>index f2c5a14..0d51a11 100644
>>>--- a/drivers/ide/pci/generic.c
>>>+++ b/drivers/ide/pci/generic.c
>>>@@ -198,32 +198,41 @@ static ide_pci_device_t generic_chipsets[]
>>>__devinitdata = {
>>> static int __devinit generic_init_one(struct pci_dev *dev, const
>>>struct pci_device_id *id)
>>> {
>>>     ide_pci_device_t *d = &generic_chipsets[id->driver_data];
>>>-    u16 command;
>>>     int ret = -ENODEV;
>>> 
>>>     /* Don't use the generic entry unless instructed to do so */
>>>     if (id->driver_data == 0 && ide_generic_all == 0)
>>>             goto out;
>>> 
>>>-    if (dev->vendor == PCI_VENDOR_ID_UMC &&
>>>-        dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
>>>-        (!(PCI_FUNC(dev->devfn) & 1)))
>>>-        goto out; /* UM8886A/BF pair */
>>>-
>>>-    if (dev->vendor == PCI_VENDOR_ID_OPTI &&
>>>-        dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
>>>-        (!(PCI_FUNC(dev->devfn) & 1)))
>>>-        goto out;
>>>-
>>>-    if (dev->vendor == PCI_VENDOR_ID_JMICRON) {
>>>-        if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
>>>PCI_FUNC(dev->devfn) != 1)
>>>+    switch (dev->vendor) {
>>>+    case PCI_VENDOR_ID_UMC:
>>>+        if (dev->device == PCI_DEVICE_ID_UMC_UM8886A &&
>>>+                !(PCI_FUNC(dev->devfn) & 1))
>>>+            goto out; /* UM8886A/BF pair */
>>>+        break;
>>>+    case PCI_VENDOR_ID_OPTI:
>>>+        if (dev->device == PCI_DEVICE_ID_OPTI_82C558 &&
>>>+                !(PCI_FUNC(dev->devfn) & 1))
>>>+            goto out;
>>>+        break;
>>>+    case PCI_VENDOR_ID_JMICRON:
>>>+        if (dev->device != PCI_DEVICE_ID_JMICRON_JMB368 &&
>>>+                PCI_FUNC(dev->devfn) != 1)
>>>+            goto out;
>>>+        break;
>>>+    case PCI_VENDOR_ID_NS:
>>>+        if (dev->device == PCI_DEVICE_ID_NS_87410 &&
>>>+                (dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
>>>             goto out;
>>>+        break;
>>>     }

>>  Could you intend the if() conditions carried to another line the way
>>they were intended before?

> I could, if you tell me the reason.

    Sorry -- I thought it was a part of kernel style, but it appeared to be my 
aesthetical preference only. :-)

> regards,

MBR, Sergei

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Jiri Slaby]

Sergei Shtylyov napsal(a):
> Hello.

Hi.

>    Sorry -- I thought it was a part of kernel style, but it appeared to
> be my aesthetical preference only. :-)

Ok, anyway thanks for reviewing,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Bartlomiej Zolnierkiewicz]

On Monday 04 June 2007, Jiri Slaby wrote:
> ide-generic, add another device exception

ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
(a generic IDE PCI driver) - fixed patch description to avoid confusion.

[ Yes, both drivers need a rename - patches are welcomed. ]

> This device is char device and is grabbed by generic ide driver:
> 00:0b.0 Class ffff: National Semiconductor Corporation 87410 IDE (rev ff) (prog-if ff)
> 	Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
> 
> Disallow generic driver to grab it by adding next condition. Also
> consolidate exceptions to one bigger 'switch (dev->vendor)'.
> 
> Cc: Ingo Korb <ml@akana.de>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

applied, thanks.

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Jeff Garzik]

On Fri, Jun 08, 2007 at 02:18:55PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Monday 04 June 2007, Jiri Slaby wrote:
> > ide-generic, add another device exception
> 
> ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
> (a generic IDE PCI driver) - fixed patch description to avoid confusion.
> 
> [ Yes, both drivers need a rename - patches are welcomed. ]

Probably best to do a rename from your side, in git.

	Jeff

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Bartlomiej Zolnierkiewicz]

Hi,

On Friday 08 June 2007, Jeff Garzik wrote:
> On Fri, Jun 08, 2007 at 02:18:55PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Monday 04 June 2007, Jiri Slaby wrote:
> > > ide-generic, add another device exception
> > 
> > ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
> > (a generic IDE PCI driver) - fixed patch description to avoid confusion.
> > 
> > [ Yes, both drivers need a rename - patches are welcomed. ]
> 
> Probably best to do a rename from your side, in git.

AFAIR git-quiltimport of quilt patches should handle this just fine?

My previous description of the issue was a bit too cryptic: besides
changing ide/ide-generic.c and ide/pci/generic.c filenames, ide/Kconfig
(names of config options) and ide/Makefile should be updated accordingly.
Also enhancing Kconfig entries for both drivers to be more verbose would
be more than appropriate.

Since I'm quite time constrained and I'm working on other IDE issues I
would prefer that somebody else handle this task and just send me patches.

8)

Thanks,
Bart

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Alan Cox]

On Fri, 8 Jun 2007 14:18:55 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> On Monday 04 June 2007, Jiri Slaby wrote:
> > ide-generic, add another device exception
> 
> ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
> (a generic IDE PCI driver) - fixed patch description to avoid confusion.

Nope ide-generic is a generic I/O based ST506 interface. It covers a bit
more than just ISA bus.

> 
> [ Yes, both drivers need a rename - patches are welcomed. ]

Good way to break everyones module settings

Alan

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Alan Cox]

On Sat, 9 Jun 2007 00:42:50 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> On Friday 08 June 2007, Alan Cox wrote:
> > On Fri, 8 Jun 2007 14:18:55 +0200
> > Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > 
> > > On Monday 04 June 2007, Jiri Slaby wrote:
> > > > ide-generic, add another device exception
> > > 
> > > ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
> > > (a generic IDE PCI driver) - fixed patch description to avoid confusion.
> > 
> > Nope ide-generic is a generic I/O based ST506 interface. It covers a bit
> > more than just ISA bus.
> 
> If so then can we finally get rid of hd.c?

Ok ST-506 register set. ST-506 MFM command set is a bit different and
does need hd.c (SFF only deals with PCI controllers)

> > > [ Yes, both drivers need a rename - patches are welcomed. ]
> > 
> > Good way to break everyones module settings
> 
> MODULE_ALIAS() should cover this up nicely.

No problem with that. Just need to be sure it gets done.

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Bartlomiej Zolnierkiewicz]

On Friday 08 June 2007, Alan Cox wrote:
> On Fri, 8 Jun 2007 14:18:55 +0200
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > On Monday 04 June 2007, Jiri Slaby wrote:
> > > ide-generic, add another device exception
> > 
> > ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
> > (a generic IDE PCI driver) - fixed patch description to avoid confusion.
> 
> Nope ide-generic is a generic I/O based ST506 interface. It covers a bit
> more than just ISA bus.

If so then can we finally get rid of hd.c?

> > [ Yes, both drivers need a rename - patches are welcomed. ]
> 
> Good way to break everyones module settings

MODULE_ALIAS() should cover this up nicely.

Thanks,
Bart

Re: [PATCH 1/1] ide: ide-generic, add another device exception

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> On Friday 08 June 2007, Jeff Garzik wrote:
>> On Fri, Jun 08, 2007 at 02:18:55PM +0200, Bartlomiej Zolnierkiewicz wrote:
>>> On Monday 04 June 2007, Jiri Slaby wrote:
>>>> ide-generic, add another device exception
>>> ide-generic is a generic ISA IDE driver, this one is drivers/ide/pci/generic
>>> (a generic IDE PCI driver) - fixed patch description to avoid confusion.
>>>
>>> [ Yes, both drivers need a rename - patches are welcomed. ]
>> Probably best to do a rename from your side, in git.
> 
> AFAIR git-quiltimport of quilt patches should handle this just fine?

'git mv old_name new_name'