2008-12-30 16:12:36

by Kirill Smelkov

[permalink] [raw]
Subject: [PATCH] ide: motherboard-info based blacklist for ide-dma

From: Dmitry Gryazin <[email protected]>

We need to keep our kernel working on both legacy and modern hardware.

As it turned out, some boards don't have proper IDE DMA support for
CompactFlash (CF) adaptors, because pin connectors which relate to DMA are left
unsoldered (*).

To verify this we even bought external IDE CF adaptor which showed the same
symthoms, and then manually soldered pins that were left dangling, and voila,
DMA started to work!

Again, as they say, lots of manufacturers of not-so-new hardware did CF
soldering by the wrong way with respect to DMA, so we need a workaround.

For this __ide_dma_bad_drive function is enhance to check not only drives
blacklist but motherboards blacklist too.

Please apply before 2.6.29

(*) On IEI PCISA-C3/EDEN the kernel boots _very_ slowly, becuase we have lots
of DMA timer expiry:

<~30s pause>
hdc: dma_timer_epiry: dma_status == 0x21
hdc: DMA timeout error
...
hdc: DMA disabled
ide1: dma reset

<and the whole story repeats:>
<~30s pause>
hdc: dma_timer_epiry: dma_status == 0x21
...

Also see this posts from 2006 where they say this is usually a problem with CF
adapter not fully soldered:

http://lkml.org/lkml/2006/5/23/198
http://lkml.org/lkml/2006/6/20/164

Signed-off-by: Dmitry Gryazin <[email protected]>
Signed-off-by: Kirill Smelkov <[email protected]>
---
drivers/ide/ide-dma.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
index fffd117..adb5d9d 100644
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -33,6 +33,21 @@
#include <linux/ide.h>
#include <linux/scatterlist.h>
#include <linux/dma-mapping.h>
+#include <linux/dmi.h>
+
+/* Internal structure for blacklisted boards */
+struct board_blacklist_entry {
+ const char *board_vendor;
+ const char *board_name;
+ const char *drive_name;
+};
+
+/* Blacklisted boards which have problems with DMA */
+static const struct board_blacklist_entry board_blacklist[] = {
+ /* on IEI PCISA-C3 CF adapter pin connectors for DMA are missing */
+ { "FIC" , "PT-2200" , "hdc" },
+ { NULL , NULL , NULL }
+};

static const struct drive_list_entry drive_whitelist[] = {
{ "Micropolis 2112A" , NULL },
@@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
drive->hwif->dma_ops->dma_host_set(drive, 1);
}

+static int __ide_dma_bad_adaptor(ide_drive_t *drive)
+{
+ const struct board_blacklist_entry *table = board_blacklist;
+
+ const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
+ const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+ if (!board_vendor || !board_name)
+ return 0;
+
+ for ( ; table->board_name ; table++)
+ if ((!strcmp(board_vendor, table->board_vendor)) &&
+ (!strcmp(board_name, table->board_name)) &&
+ (!strcmp(drive->name, table->drive_name))) {
+ printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
+ "(Board %s %s is blacklisted)\n", drive->name,
+ (char *)&drive->id[ATA_ID_PROD], board_vendor,
+ board_name);
+ return 1;
+ }
+
+ return 0;
+}
+
int __ide_dma_bad_drive(ide_drive_t *drive)
{
u16 *id = drive->id;
@@ -217,7 +256,8 @@ int __ide_dma_bad_drive(ide_drive_t *drive)
drive->name, (char *)&id[ATA_ID_PROD]);
return blacklist;
}
- return 0;
+
+ return __ide_dma_bad_adaptor(drive);
}
EXPORT_SYMBOL(__ide_dma_bad_drive);

--
1.6.1.49.g94ee8


2008-12-30 16:30:45

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello.

Kirill Smelkov wrote:

> From: Dmitry Gryazin <[email protected]>

> We need to keep our kernel working on both legacy and modern hardware.

> As it turned out, some boards don't have proper IDE DMA support for
> CompactFlash (CF) adaptors, because pin connectors which relate to DMA are left
> unsoldered (*).

> To verify this we even bought external IDE CF adaptor which showed the same
> symthoms, and then manually soldered pins that were left dangling, and voila,
> DMA started to work!

> Again, as they say, lots of manufacturers of not-so-new hardware did CF
> soldering by the wrong way with respect to DMA, so we need a workaround.

> For this __ide_dma_bad_drive function is enhance to check not only drives
> blacklist but motherboards blacklist too.

> Please apply before 2.6.29

> (*) On IEI PCISA-C3/EDEN the kernel boots _very_ slowly, becuase we have lots
> of DMA timer expiry:

> <~30s pause>
> hdc: dma_timer_epiry: dma_status == 0x21
> hdc: DMA timeout error
> ...
> hdc: DMA disabled
> ide1: dma reset
>
> <and the whole story repeats:>
> <~30s pause>
> hdc: dma_timer_epiry: dma_status == 0x21
> ...

Are you aware of ide_core.nodma= option which allows disabling DMA per
interface/drive? Read Documentation/ide/ide.txt please. I should note that
nodma option has been there for ages in one or other form.

> Also see this posts from 2006 where they say this is usually a problem with CF
> adapter not fully soldered:

> http://lkml.org/lkml/2006/5/23/198
> http://lkml.org/lkml/2006/6/20/164

> Signed-off-by: Dmitry Gryazin <[email protected]>
> Signed-off-by: Kirill Smelkov <[email protected]>

NAK.

> diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
> index fffd117..adb5d9d 100644
> --- a/drivers/ide/ide-dma.c
> +++ b/drivers/ide/ide-dma.c
> @@ -33,6 +33,21 @@
> #include <linux/ide.h>
> #include <linux/scatterlist.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dmi.h>
> +
> +/* Internal structure for blacklisted boards */
> +struct board_blacklist_entry {
> + const char *board_vendor;
> + const char *board_name;
> + const char *drive_name;
> +};
> +
> +/* Blacklisted boards which have problems with DMA */
> +static const struct board_blacklist_entry board_blacklist[] = {
> + /* on IEI PCISA-C3 CF adapter pin connectors for DMA are missing */
> + { "FIC" , "PT-2200" , "hdc" },
> + { NULL , NULL , NULL }
> +};
>
> static const struct drive_list_entry drive_whitelist[] = {
> { "Micropolis 2112A" , NULL },
> @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
> drive->hwif->dma_ops->dma_host_set(drive, 1);
> }
>
> +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
> +{
> + const struct board_blacklist_entry *table = board_blacklist;
> +
> + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> + if (!board_vendor || !board_name)
> + return 0;
> +
> + for ( ; table->board_name ; table++)
> + if ((!strcmp(board_vendor, table->board_vendor)) &&
> + (!strcmp(board_name, table->board_name)) &&
> + (!strcmp(drive->name, table->drive_name))) {
> + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
> + "(Board %s %s is blacklisted)\n", drive->name,
> + (char *)&drive->id[ATA_ID_PROD], board_vendor,
> + board_name);
> + return 1;
> + }
> +
> + return 0;
> +}
> +

This code doesn't anyhow discriminate the case of on-board CF and say
normal IDE PCI card plugged into such board -- with latter having no issues
with DMA whatsoever. E.g. AMD Geode GX2 dev. board (DB2301) has CF slot (with
unsoldered DMA pins, IIRC) and 3 PCI slots where you can plug a normal IDE card.

MBR, Sergei

Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

On Tuesday 30 December 2008, Sergei Shtylyov wrote:
> Hello.
>
> Kirill Smelkov wrote:
>
> > From: Dmitry Gryazin <[email protected]>
>
> > We need to keep our kernel working on both legacy and modern hardware.
>
> > As it turned out, some boards don't have proper IDE DMA support for
> > CompactFlash (CF) adaptors, because pin connectors which relate to DMA are left
> > unsoldered (*).
>
> > To verify this we even bought external IDE CF adaptor which showed the same
> > symthoms, and then manually soldered pins that were left dangling, and voila,
> > DMA started to work!
>
> > Again, as they say, lots of manufacturers of not-so-new hardware did CF
> > soldering by the wrong way with respect to DMA, so we need a workaround.
>
> > For this __ide_dma_bad_drive function is enhance to check not only drives
> > blacklist but motherboards blacklist too.
>
> > Please apply before 2.6.29
>
> > (*) On IEI PCISA-C3/EDEN the kernel boots _very_ slowly, becuase we have lots
> > of DMA timer expiry:
>
> > <~30s pause>
> > hdc: dma_timer_epiry: dma_status == 0x21
> > hdc: DMA timeout error
> > ...
> > hdc: DMA disabled
> > ide1: dma reset
> >
> > <and the whole story repeats:>
> > <~30s pause>
> > hdc: dma_timer_epiry: dma_status == 0x21
> > ...
>
> Are you aware of ide_core.nodma= option which allows disabling DMA per
> interface/drive? Read Documentation/ide/ide.txt please. I should note that
> nodma option has been there for ages in one or other form.

While it can be workarounded manually using "ide_core.nodma" we should really
be aiming at ease of use for the end users and always let the code handle such
issues automatically if possible.

> > Also see this posts from 2006 where they say this is usually a problem with CF
> > adapter not fully soldered:
>
> > http://lkml.org/lkml/2006/5/23/198
> > http://lkml.org/lkml/2006/6/20/164
>
> > Signed-off-by: Dmitry Gryazin <[email protected]>
> > Signed-off-by: Kirill Smelkov <[email protected]>
>
> NAK.
>
> > diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
> > index fffd117..adb5d9d 100644
> > --- a/drivers/ide/ide-dma.c
> > +++ b/drivers/ide/ide-dma.c
> > @@ -33,6 +33,21 @@
> > #include <linux/ide.h>
> > #include <linux/scatterlist.h>
> > #include <linux/dma-mapping.h>
> > +#include <linux/dmi.h>
> > +
> > +/* Internal structure for blacklisted boards */
> > +struct board_blacklist_entry {
> > + const char *board_vendor;
> > + const char *board_name;
> > + const char *drive_name;
> > +};
> > +
> > +/* Blacklisted boards which have problems with DMA */
> > +static const struct board_blacklist_entry board_blacklist[] = {
> > + /* on IEI PCISA-C3 CF adapter pin connectors for DMA are missing */
> > + { "FIC" , "PT-2200" , "hdc" },
> > + { NULL , NULL , NULL }
> > +};
> >
> > static const struct drive_list_entry drive_whitelist[] = {
> > { "Micropolis 2112A" , NULL },
> > @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
> > drive->hwif->dma_ops->dma_host_set(drive, 1);
> > }
> >
> > +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
> > +{
> > + const struct board_blacklist_entry *table = board_blacklist;
> > +
> > + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> > + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> > +
> > + if (!board_vendor || !board_name)
> > + return 0;
> > +
> > + for ( ; table->board_name ; table++)
> > + if ((!strcmp(board_vendor, table->board_vendor)) &&
> > + (!strcmp(board_name, table->board_name)) &&
> > + (!strcmp(drive->name, table->drive_name))) {
> > + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
> > + "(Board %s %s is blacklisted)\n", drive->name,
> > + (char *)&drive->id[ATA_ID_PROD], board_vendor,
> > + board_name);
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> This code doesn't anyhow discriminate the case of on-board CF and say
> normal IDE PCI card plugged into such board -- with latter having no issues
> with DMA whatsoever. E.g. AMD Geode GX2 dev. board (DB2301) has CF slot (with
> unsoldered DMA pins, IIRC) and 3 PCI slots where you can plug a normal IDE card.

True. However it should be possible to handle it correctly by adding the
DMA quirk to the respective host drivers (seems to be via82cxxx.c in case of
IEI PCISA-C3/EDEN).

Kirill, could you please look into adding such quirk to via82cxxx instead?

[ It seems the best place to add it would be via_init_one() as we could just
clear host DMA masks (please also remember to first check if the controller
is an external one -- for VIA it could only be VT6410). BTW it should be
possible to greatly simplify DMI checking by using dmi_check_system() helper
similarly how it's already done for cable detection in via82cxxx. ]

Sergei, care to handle AMD Geode GX2 case?

Thanks,
Bart

2009-01-04 20:33:20

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>> We need to keep our kernel working on both legacy and modern hardware.
>>>
>>> As it turned out, some boards don't have proper IDE DMA support for
>>> CompactFlash (CF) adaptors, because pin connectors which relate to DMA are left
>>> unsoldered (*).
>>>
>>> To verify this we even bought external IDE CF adaptor which showed the same
>>> symthoms, and then manually soldered pins that were left dangling, and voila,
>>> DMA started to work!
>>>
>>> Again, as they say, lots of manufacturers of not-so-new hardware did CF
>>> soldering by the wrong way with respect to DMA, so we need a workaround.
>>>
>>> For this __ide_dma_bad_drive function is enhance to check not only drives
>>> blacklist but motherboards blacklist too.
>>>
>>> Please apply before 2.6.29
>>>
>>> (*) On IEI PCISA-C3/EDEN the kernel boots _very_ slowly, becuase we have lots
>>> of DMA timer expiry:
>>>
>>> <~30s pause>
>>> hdc: dma_timer_epiry: dma_status == 0x21
>>> hdc: DMA timeout error
>>> ...
>>> hdc: DMA disabled
>>> ide1: dma reset
>>>
>>> <and the whole story repeats:>
>>> <~30s pause>
>>> hdc: dma_timer_epiry: dma_status == 0x21
>>> ...
>>>
>> Are you aware of ide_core.nodma= option which allows disabling DMA per
>> interface/drive? Read Documentation/ide/ide.txt please. I should note that
>> nodma option has been there for ages in one or other form.
>>
>
> While it can be workarounded manually using "ide_core.nodma" we should really
> be aiming at ease of use for the end users and always let the code handle such
> issues automatically if possible.
>

I'd agree. The patch didn't seem to provide an accpetable form of
such workaround though... although looks like I wasn't looking
attentively enough.

>>> diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
>>> index fffd117..adb5d9d 100644
>>> --- a/drivers/ide/ide-dma.c
>>> +++ b/drivers/ide/ide-dma.c
>>> @@ -33,6 +33,21 @@
>>> #include <linux/ide.h>
>>> #include <linux/scatterlist.h>
>>> #include <linux/dma-mapping.h>
>>> +#include <linux/dmi.h>
>>> +
>>> +/* Internal structure for blacklisted boards */
>>> +struct board_blacklist_entry {
>>> + const char *board_vendor;
>>> + const char *board_name;
>>> + const char *drive_name;
>>> +};
>>> +
>>> +/* Blacklisted boards which have problems with DMA */
>>> +static const struct board_blacklist_entry board_blacklist[] = {
>>> + /* on IEI PCISA-C3 CF adapter pin connectors for DMA are missing */
>>> + { "FIC" , "PT-2200" , "hdc" },
>>>

I don't get it... FIC PT-2200 was Pentium class motherboard with
Intel's 430HX chipset not having CF slot at all (as googling for its
name has revealed). How it is connected to the (relatively modern)
PCISA-C3/EPIC board?! What are you trying to smuggle into kernel? :-) Or
this board just reports the bogus DMI IDs?

>>> @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
>>> drive->hwif->dma_ops->dma_host_set(drive, 1);
>>> }
>>>
>>> +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
>>> +{
>>> + const struct board_blacklist_entry *table = board_blacklist;
>>> +
>>> + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
>>> + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
>>> +
>>> + if (!board_vendor || !board_name)
>>> + return 0;
>>> +
>>> + for ( ; table->board_name ; table++)
>>> + if ((!strcmp(board_vendor, table->board_vendor)) &&
>>> + (!strcmp(board_name, table->board_name)) &&
>>> + (!strcmp(drive->name, table->drive_name))) {
>>> + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
>>> + "(Board %s %s is blacklisted)\n", drive->name,
>>> + (char *)&drive->id[ATA_ID_PROD], board_vendor,
>>> + board_name);
>>> + return 1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>>
>> This code doesn't anyhow discriminate the case of on-board CF and say
>>

Hm, previously I failed to notice that the patch tries to
discriminate this case based on applying the workaround only to the
drive with certain name ("hdc"). However, it still seems wrong to place
this workaround in __ide_dma_bad_drive() as it's not actually connected
to the deficiency of a specific drive but to the definciency of the CF
slot itself. Moreover, depending on the PCI cards plugged, the drive's
name may change...

>> normal IDE PCI card plugged into such board -- with latter having no issues
>>

And the PT-2200 board did have PCI and even ISA slots... oh wait,
that's really another board. :-) That EPIC board is PICMG however, so
can have a backplane with PCI slots.

>> with DMA whatsoever. E.g. AMD Geode GX2 dev. board (DB2301) has CF slot (with
>> unsoldered DMA pins, IIRC) and 3 PCI slots where you can plug a normal IDE card.
>
> True. However it should be possible to handle it correctly by adding the
> DMA quirk to the respective host drivers (seems to be via82cxxx.c in case of
> IEI PCISA-C3/EDEN).

Yeah, this seems a viable approach...

> Kirill, could you please look into adding such quirk to via82cxxx instead?
>
> [ It seems the best place to add it would be via_init_one() as we could just
>

No, not really -- the issue is not at all as simple as this patch
tried to present it. Looking at its "Quick Startup Reference"
(http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
board has *two* normal IDE connectors in addition to the CF slot
(connected to the secondary port -- and it seems possible that a hard
drive can be connected to the same port as CF), so the right place seems
to rather be in [mu]dma_filter() methods -- and the decision should be
strictly based on the drive type indicating CF, i.e. by calling
ata_id_is_cfa().

> Sergei, care to handle AMD Geode GX2 case?
>

Eh... the DB2301 board I had on my desk for a long time died in my
hands 12/31, that same day when I NAKed this patch -- sort of NY
present/ :-) Well, actually it stopped to boot the BIOS several weeks
before that, then it just stopped to power up. We probably have another
one (not sure if it's alive still) but all MV's development work for
Geode GX2 stoped like 3 years ago. And now I'm not even sure that CF DMA
issue really existed there -- I wasn't directly involved with CF related
issues we had at the time of the active development, so my memory might
be wrong. What I can say for sure is that the CF slot on DB2301 board
coexisted with the normal IDE connector on CS5535's single channel and
both CF and hard disk worked simultaneously (if you were lucky :-). I'll
see if I have the board schematics from which I may be able to tell if
the issue really existed.

> Thanks,
> Bart
>
MBR, Sergei

2009-01-04 21:13:36

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello, I wrote:

>>>> drive->hwif->dma_ops->dma_host_set(drive, 1);
>>>> }
>>>>
>>>> +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
>>>> +{
>>>> + const struct board_blacklist_entry *table = board_blacklist;
>>>> +
>>>> + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
>>>> + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
>>>> +
>>>> + if (!board_vendor || !board_name)
>>>> + return 0;
>>>> +
>>>> + for ( ; table->board_name ; table++)
>>>> + if ((!strcmp(board_vendor, table->board_vendor)) &&
>>>> + (!strcmp(board_name, table->board_name)) &&
>>>> + (!strcmp(drive->name, table->drive_name))) {
>>>> + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
>>>> + "(Board %s %s is blacklisted)\n", drive->name,
>>>> + (char *)&drive->id[ATA_ID_PROD], board_vendor,
>>>> + board_name);
>>>> + return 1;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>>
>>> This code doesn't anyhow discriminate the case of on-board CF
>>> and say
> @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
>
> Hm, previously I failed to notice that the patch tries to
> discriminate this case based on applying the workaround only to the
> drive with certain name ("hdc"). However, it still seems wrong to
> place this workaround in __ide_dma_bad_drive() as it's not actually
> connected to the deficiency of a specific drive but to the definciency
> of the CF slot itself. Moreover, depending on the PCI cards plugged,
> the drive's name may change...

Oh, and the master/slave mode of the CF drive is selectable by
on-board switch -- via the -CSEL signal I guess. :-)

MBR, Sergei

2009-01-04 23:34:55

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello, I wrote:

>>>> As it turned out, some boards don't have proper IDE DMA
>>>> support for
>>>> CompactFlash (CF) adaptors, because pin connectors which relate to
>>>> DMA are left
>>>> unsoldered (*).
>>>> To verify this we even bought external IDE CF adaptor which
>>>> showed the same
>>>> symthoms, and then manually soldered pins that were left dangling,
>>>> and voila,
>>>> DMA started to work!
>>>> Again, as they say, lots of manufacturers of not-so-new
>>>> hardware did CF
>>>> soldering by the wrong way with respect to DMA, so we need a
>>>> workaround.
>>>> For this __ide_dma_bad_drive function is enhance to check not
>>>> only drives
>>>> blacklist but motherboards blacklist too.
>>>> Please apply before 2.6.29
>>>> (*) On IEI PCISA-C3/EDEN the kernel boots _very_ slowly,
>>>> becuase we have lots
>>>> of DMA timer expiry:
>>>> <~30s pause>
>>>> hdc: dma_timer_epiry: dma_status == 0x21
>>>> hdc: DMA timeout error
>>>> ...
>>>> hdc: DMA disabled
>>>> ide1: dma reset
>>>>
>>>> <and the whole story repeats:>
>>>> <~30s pause>
>>>> hdc: dma_timer_epiry: dma_status == 0x21
>>>> ...
>>>>
>>> Are you aware of ide_core.nodma= option which allows disabling
>>> DMA per interface/drive? Read Documentation/ide/ide.txt please. I
>>> should note that nodma option has been there for ages in one or
>>> other form.
>>>
>>
>> While it can be workarounded manually using "ide_core.nodma" we
>> should really
>> be aiming at ease of use for the end users and always let the code
>> handle such
>> issues automatically if possible.
>>
>
> I'd agree. The patch didn't seem to provide an accpetable form of
> such workaround though... although looks like I wasn't looking
> attentively enough.
>
>>>> diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
>>>> index fffd117..adb5d9d 100644
>>>> --- a/drivers/ide/ide-dma.c
>>>> +++ b/drivers/ide/ide-dma.c
>>>> @@ -33,6 +33,21 @@
>>>> #include <linux/ide.h>
>>>> #include <linux/scatterlist.h>
>>>> #include <linux/dma-mapping.h>
>>>> +#include <linux/dmi.h>
>>>> +
>>>> +/* Internal structure for blacklisted boards */
>>>> +struct board_blacklist_entry {
>>>> + const char *board_vendor;
>>>> + const char *board_name;
>>>> + const char *drive_name;
>>>> +};
>>>> +
>>>> +/* Blacklisted boards which have problems with DMA */
>>>> +static const struct board_blacklist_entry board_blacklist[] = {
>>>> + /* on IEI PCISA-C3 CF adapter pin connectors for DMA are
>>>> missing */
>>>> + { "FIC" , "PT-2200" , "hdc" },
>>>>
>
> I don't get it... FIC PT-2200 was Pentium class motherboard with
> Intel's 430HX chipset not having CF slot at all (as googling for its
> name has revealed). How it is connected to the (relatively modern)
> PCISA-C3/EPIC board?! What are you trying to smuggle into kernel? :-)
> Or this board just reports the bogus DMI IDs?
>
>>>> @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
>>>> drive->hwif->dma_ops->dma_host_set(drive, 1);
>>>> }
>>>>
>>>> +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
>>>> +{
>>>> + const struct board_blacklist_entry *table = board_blacklist;
>>>> +
>>>> + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
>>>> + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
>>>> +
>>>> + if (!board_vendor || !board_name)
>>>> + return 0;
>>>> +
>>>> + for ( ; table->board_name ; table++)
>>>> + if ((!strcmp(board_vendor, table->board_vendor)) &&
>>>> + (!strcmp(board_name, table->board_name)) &&
>>>> + (!strcmp(drive->name, table->drive_name))) {
>>>> + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
>>>> + "(Board %s %s is blacklisted)\n", drive->name,
>>>> + (char *)&drive->id[ATA_ID_PROD], board_vendor,
>>>> + board_name);
>>>> + return 1;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>>
>>> This code doesn't anyhow discriminate the case of on-board CF
>>> and say
>
> Hm, previously I failed to notice that the patch tries to
> discriminate this case based on applying the workaround only to the
> drive with certain name ("hdc"). However, it still seems wrong to
> place this workaround in __ide_dma_bad_drive() as it's not actually
> connected to the deficiency of a specific drive but to the definciency
> of the CF slot itself. Moreover, depending on the PCI cards plugged,
> the drive's name may change...
>
>>> normal IDE PCI card plugged into such board -- with latter having no
>>> issues
>
> And the PT-2200 board did have PCI and even ISA slots... oh wait,
> that's really another board. :-) That EPIC board is PICMG however, so
> can have a backplane with PCI slots.
>
>>> with DMA whatsoever. E.g. AMD Geode GX2 dev. board (DB2301) has CF
>>> slot (with unsoldered DMA pins, IIRC) and 3 PCI slots where you can
>>> plug a normal IDE card.
>>
>> True. However it should be possible to handle it correctly by adding
>> the
>> DMA quirk to the respective host drivers (seems to be via82cxxx.c in
>> case of
>> IEI PCISA-C3/EDEN).
>
> Yeah, this seems a viable approach...
>
>> Kirill, could you please look into adding such quirk to via82cxxx
>> instead?
>>
>> [ It seems the best place to add it would be via_init_one() as we
>> could just
>>
>
> No, not really -- the issue is not at all as simple as this patch
> tried to present it. Looking at its "Quick Startup Reference"
> (http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
> board has *two* normal IDE connectors in addition to the CF slot
> (connected to the secondary port -- and it seems possible that a hard
> drive can be connected to the same port as CF), so the right place
> seems to rather be in [mu]dma_filter() methods -- and the decision
> should be strictly based on the drive type indicating CF, i.e. by
> calling ata_id_is_cfa().
>
>> Sergei, care to handle AMD Geode GX2 case?
>>
>
> Eh... the DB2301 board I had on my desk for a long time died in my
> hands 12/31, that same day when I NAKed this patch -- sort of NY
> present/ :-) Well, actually it stopped to boot the BIOS several weeks
> before that, then it just stopped to power up. We probably have
> another one (not sure if it's alive still) but all MV's development
> work for Geode GX2 stoped like 3 years ago. And now I'm not even sure
> that CF DMA issue really existed there -- I wasn't directly involved
> with CF related issues we had at the time of the active development,
> so my memory might be wrong. What I can say for sure is that the CF
> slot on DB2301 board coexisted with the normal IDE connector on
> CS5535's single channel and both CF and hard disk worked
> simultaneously (if you were lucky :-). I'll see if I have the board
> schematics from which I may be able to tell if the issue really existed.

Finally found the schematics on AMD developers' site. Yes, the issue
existed on DM2301 -- they didn't wire CF pins 43/44 for DMA. If only my
board was alive... :-)
CCing Jordan, AMD Geode guru (not sure if he's still active). Jordan,
do you by chance know if the Insyde BIOS for Geode GX2 supported DMI at
all so that we can identify it easily?

>> Thanks,
>> Bart

MBR, Sergei

2009-01-15 12:36:54

by Dmitry Gryazin

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello for everybody.

First of all I'd like to say that we are sorry for so looong delay with
replying.

My name is Dmitry Gryazin, and I'm a collegue of Kirill Smelkov. I'll reply to
the issues you've raised inline:

On Monday 05 January 2009 02:34:35 am Sergei Shtylyov wrote:
[...]
> >>> Are you aware of ide_core.nodma= option which allows disabling
> >>> DMA per interface/drive? Read Documentation/ide/ide.txt please. I
> >>> should note that nodma option has been there for ages in one or
> >>> other form.
> >>
> >> While it can be workarounded manually using "ide_core.nodma" we
> >> should really
> >> be aiming at ease of use for the end users and always let the code
> >> handle such
> >> issues automatically if possible.
> >
> > I'd agree. The patch didn't seem to provide an accpetable form of
> > such workaround though... although looks like I wasn't looking
> > attentively enough.

We know about "ide_core.nodma" and we have been using it for PCISA-C3 for
ages. But we have to support three different motherboards (some of them are
with CF problems, some of them not) with one home-made distribution now.

So, the idea was to get it works automatically.

> >>>> diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c
> >>>> index fffd117..adb5d9d 100644
> >>>> --- a/drivers/ide/ide-dma.c
> >>>> +++ b/drivers/ide/ide-dma.c
> >>>> @@ -33,6 +33,21 @@
> >>>> #include <linux/ide.h>
> >>>> #include <linux/scatterlist.h>
> >>>> #include <linux/dma-mapping.h>
> >>>> +#include <linux/dmi.h>
> >>>> +
> >>>> +/* Internal structure for blacklisted boards */
> >>>> +struct board_blacklist_entry {
> >>>> + const char *board_vendor;
> >>>> + const char *board_name;
> >>>> + const char *drive_name;
> >>>> +};
> >>>> +
> >>>> +/* Blacklisted boards which have problems with DMA */
> >>>> +static const struct board_blacklist_entry board_blacklist[] = {
> >>>> + /* on IEI PCISA-C3 CF adapter pin connectors for DMA are
> >>>> missing */
> >>>> + { "FIC" , "PT-2200" , "hdc" },
> >
> > I don't get it... FIC PT-2200 was Pentium class motherboard with
> > Intel's 430HX chipset not having CF slot at all (as googling for its
> > name has revealed). How it is connected to the (relatively modern)
> > PCISA-C3/EPIC board?! What are you trying to smuggle into kernel? :-)
> > Or this board just reports the bogus DMI IDs?

Yes, PCISA-C3 (with BIOS v1.3) says vendor="FIC" and name="PT-2200".
It's a pity that vendors don't care to correctly assign DMI IDs, so that we
can't fully rely on them.

As Bartlomiej suggests, one of the way to solve this problem is to use IDE
controller as additional criterion to decide on which port/drive DMA should
be off.

> >>>> @@ -207,6 +222,30 @@ void ide_dma_on(ide_drive_t *drive)
> >>>> drive->hwif->dma_ops->dma_host_set(drive, 1);
> >>>> }
> >>>>
> >>>> +static int __ide_dma_bad_adaptor(ide_drive_t *drive)
> >>>> +{
> >>>> + const struct board_blacklist_entry *table = board_blacklist;
> >>>> +
> >>>> + const char *board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> >>>> + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> >>>> +
> >>>> + if (!board_vendor || !board_name)
> >>>> + return 0;
> >>>> +
> >>>> + for ( ; table->board_name ; table++)
> >>>> + if ((!strcmp(board_vendor, table->board_vendor)) &&
> >>>> + (!strcmp(board_name, table->board_name)) &&
> >>>> + (!strcmp(drive->name, table->drive_name))) {
> >>>> + printk(KERN_WARNING "%s: Disabling (U)DMA for %s "
> >>>> + "(Board %s %s is blacklisted)\n", drive->name,
> >>>> + (char *)&drive->id[ATA_ID_PROD], board_vendor,
> >>>> + board_name);
> >>>> + return 1;
> >>>> + }
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>
> >>> This code doesn't anyhow discriminate the case of on-board CF
> >>> and say
> >
> > Hm, previously I failed to notice that the patch tries to
> > discriminate this case based on applying the workaround only to the
> > drive with certain name ("hdc"). However, it still seems wrong to
> > place this workaround in __ide_dma_bad_drive() as it's not actually
> > connected to the deficiency of a specific drive but to the definciency
> > of the CF slot itself. Moreover, depending on the PCI cards plugged,
> > the drive's name may change...
> >
> >>> normal IDE PCI card plugged into such board -- with latter having no
> >>> issues

Yes, I've to agree that using "hdc" is not correct.

> >> True. However it should be possible to handle it correctly by adding
> >> the
> >> DMA quirk to the respective host drivers (seems to be via82cxxx.c in
> >> case of
> >> IEI PCISA-C3/EDEN).
> >
> > Yeah, this seems a viable approach...
> >
> >> Kirill, could you please look into adding such quirk to via82cxxx
> >> instead?
> >>
> >> [ It seems the best place to add it would be via_init_one() as we
> >> could just
> >
> > No, not really -- the issue is not at all as simple as this patch
> > tried to present it. Looking at its "Quick Startup Reference"
> > (http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
> > board has *two* normal IDE connectors in addition to the CF slot
> > (connected to the secondary port -- and it seems possible that a hard
> > drive can be connected to the same port as CF), so the right place
> > seems to rather be in [mu]dma_filter() methods -- and the decision
> > should be strictly based on the drive type indicating CF, i.e. by
> > calling ata_id_is_cfa().

As you suggest, we could use the following criterions:

Board vendor="FIC" &&
Board name="PT-2200" &&
VIA-family IDE controller &&
drive type=CF

It should work good enough.

To test it I also tried to install external IDE CF adaptor to the secondary
IDE controller (both master and slave). And it's also identified as CF drive
type. And we turn off DMA for it. But since external CF adaptor could be
wired right, turning DMA off would be incorrect in some cases.

Yes, _that_ particular external IDE/CF adaptor that we bought, has the same
problem, so turning DMA off on it should be correct. We even decided to
verify ourselves that wrong soldering is the cause of problems, and wired
manually necessary pins, and, voila, DMA works. So the question is:

Is it right to implement the above-mentioned criterion, which would work
correctly in 99 cases of 100, but will pessimistically turn DMA off in
rare cases (e.g. rightly wired external IDE/CF adaptor attached to
PCISA/C3), or is the whole problem unsolvable?

We could not come up with the right criterion which selects exactly on-board
CF slot, so if there is no satisfactory 100% reliable solution maybe it
doesn't make sense to continue...

We ask for help and advice what to do now.

Thanks beforehand,
Dmitry, Kirill.

2009-01-16 12:36:04

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Hello.

Dmitry Gryazin wrote:

>>>> True. However it should be possible to handle it correctly by adding
>>>> the
>>>> DMA quirk to the respective host drivers (seems to be via82cxxx.c in
>>>> case of
>>>> IEI PCISA-C3/EDEN).
>>>>
>>> Yeah, this seems a viable approach...
>>>
>>>> Kirill, could you please look into adding such quirk to via82cxxx
>>>> instead?
>>>>
>>>> [ It seems the best place to add it would be via_init_one() as we
>>>> could just
>>>>
>>> No, not really -- the issue is not at all as simple as this patch
>>> tried to present it. Looking at its "Quick Startup Reference"
>>> (http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
>>> board has *two* normal IDE connectors in addition to the CF slot
>>> (connected to the secondary port -- and it seems possible that a hard
>>> drive can be connected to the same port as CF), so the right place
>>> seems to rather be in [mu]dma_filter() methods -- and the decision
>>> should be strictly based on the drive type indicating CF, i.e. by
>>> calling ata_id_is_cfa().
>>>
>
> As you suggest, we could use the following criterions:
>
> Board vendor="FIC" &&
> Board name="PT-2200" &&
> VIA-family IDE controller &&
> drive type=CF
>

Yes, but in almost exactly opposite order (well one check shoud be added)

VT82C686 (or whatever) and
secondary channel and
CFA device and
DMI ID is FIC PT2200

> It should work good enough.
>
> To test it I also tried to install external IDE CF adaptor to the secondary
> IDE controller (both master and slave).

You mean that this adaptor is directly connected to IDE slot with the
cable... hm, haven't seen those.

> And it's also identified as CF drive
> type. And we turn off DMA for it. But since external CF adaptor could be
> wired right, turning DMA off would be incorrect in some cases.
>
> Yes, _that_ particular external IDE/CF adaptor that we bought, has the same
> problem, so turning DMA off on it should be correct. We even decided to
> verify ourselves that wrong soldering is the cause of problems, and wired
> manually necessary pins, and, voila, DMA works. So the question is:
>
> Is it right to implement the above-mentioned criterion, which would work
> correctly in 99 cases of 100, but will pessimistically turn DMA off in
> rare cases (e.g. rightly wired external IDE/CF adaptor attached to
> PCISA/C3), or is the whole problem unsolvable?
>

Good question. However, with the on-board CF adapter already present
on the seconary channel it seems improbable that the user will need to
connect another (external) CF adapter. Even if he'd need it, he'd still
be able to use the primary port.

> We could not come up with the right criterion which selects exactly on-board
> CF slot, so if there is no satisfactory 100% reliable solution maybe it
> doesn't make sense to continue...
>

I think it still does make sense.

MBR, Sergei

2009-01-22 12:48:27

by Dmitry Gryazin

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

On Friday 16 January 2009 03:35:44 pm Sergei Shtylyov wrote:
> Hello.
>
> Dmitry Gryazin wrote:
> >>>> True. However it should be possible to handle it correctly by adding
> >>>> the
> >>>> DMA quirk to the respective host drivers (seems to be via82cxxx.c in
> >>>> case of
> >>>> IEI PCISA-C3/EDEN).
> >>>
> >>> Yeah, this seems a viable approach...
> >>>
> >>>> Kirill, could you please look into adding such quirk to via82cxxx
> >>>> instead?
> >>>>
> >>>> [ It seems the best place to add it would be via_init_one() as we
> >>>> could just
> >>>
> >>> No, not really -- the issue is not at all as simple as this patch
> >>> tried to present it. Looking at its "Quick Startup Reference"
> >>> (http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
> >>> board has *two* normal IDE connectors in addition to the CF slot
> >>> (connected to the secondary port -- and it seems possible that a hard
> >>> drive can be connected to the same port as CF), so the right place
> >>> seems to rather be in [mu]dma_filter() methods -- and the decision
> >>> should be strictly based on the drive type indicating CF, i.e. by
> >>> calling ata_id_is_cfa().

I have tried my old Trancend 64Mb, RamStar 521Mb and NCP 64Mb cards. My old
cards returned right id[ATA_ID_CONFIG] = 0x848A.

But I have to use Kingston CF Card 1Gb 2008.
ata_id_is_cfa() returns 0 for it and
id[ATA_ID_MAJOR_VER] = 0
id[ATA_ID_CONFIG] = 0x044A

I have only CF+ specification revision 2.0, but I've found in wiki:
(http://en.wikipedia.org/wiki/CompactFlash#CF.2B_specification_revisions)
"... While the current revision 4.1 from 2004 works only in ATA mode, ..."

So I have reached an impasse. How to identify modern CF cards?

Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

On Thursday 22 January 2009, Dmitry Gryazin wrote:
> On Friday 16 January 2009 03:35:44 pm Sergei Shtylyov wrote:
> > Hello.
> >
> > Dmitry Gryazin wrote:
> > >>>> True. However it should be possible to handle it correctly by adding
> > >>>> the
> > >>>> DMA quirk to the respective host drivers (seems to be via82cxxx.c in
> > >>>> case of
> > >>>> IEI PCISA-C3/EDEN).
> > >>>
> > >>> Yeah, this seems a viable approach...
> > >>>
> > >>>> Kirill, could you please look into adding such quirk to via82cxxx
> > >>>> instead?
> > >>>>
> > >>>> [ It seems the best place to add it would be via_init_one() as we
> > >>>> could just
> > >>>
> > >>> No, not really -- the issue is not at all as simple as this patch
> > >>> tried to present it. Looking at its "Quick Startup Reference"
> > >>> (http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
> > >>> board has *two* normal IDE connectors in addition to the CF slot
> > >>> (connected to the secondary port -- and it seems possible that a hard
> > >>> drive can be connected to the same port as CF), so the right place
> > >>> seems to rather be in [mu]dma_filter() methods -- and the decision
> > >>> should be strictly based on the drive type indicating CF, i.e. by
> > >>> calling ata_id_is_cfa().
>
> I have tried my old Trancend 64Mb, RamStar 521Mb and NCP 64Mb cards. My old
> cards returned right id[ATA_ID_CONFIG] = 0x848A.
>
> But I have to use Kingston CF Card 1Gb 2008.
> ata_id_is_cfa() returns 0 for it and
> id[ATA_ID_MAJOR_VER] = 0
> id[ATA_ID_CONFIG] = 0x044A
>
> I have only CF+ specification revision 2.0, but I've found in wiki:
> (http://en.wikipedia.org/wiki/CompactFlash#CF.2B_specification_revisions)
> "... While the current revision 4.1 from 2004 works only in ATA mode, ..."
>
> So I have reached an impasse. How to identify modern CF cards?

Sorry that I missed it before but if indeed normal IDE devices/connectors can
be used with IDE2 then I see no sane/reliable way to detect CF devices using
buggy on-board slot... unless this slot is hardwired to be master (or slave)?

Thanks,
Bart

2009-01-22 15:53:16

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] ide: motherboard-info based blacklist for ide-dma

Bartlomiej Zolnierkiewicz wrote:

>>>>>>>True. However it should be possible to handle it correctly by adding
>>>>>>>the
>>>>>>>DMA quirk to the respective host drivers (seems to be via82cxxx.c in
>>>>>>>case of
>>>>>>>IEI PCISA-C3/EDEN).

>>>>>> Yeah, this seems a viable approach...

>>>>>>>Kirill, could you please look into adding such quirk to via82cxxx
>>>>>>>instead?

>>>>>>>[ It seems the best place to add it would be via_init_one() as we
>>>>>>>could just

>>>>>> No, not really -- the issue is not at all as simple as this patch
>>>>>>tried to present it. Looking at its "Quick Startup Reference"
>>>>>>(http://f.ipc2u.ru/files/add/doc/496/M_PCISA-C800EV_ENG.pdf), the EPIC
>>>>>>board has *two* normal IDE connectors in addition to the CF slot
>>>>>>(connected to the secondary port -- and it seems possible that a hard
>>>>>>drive can be connected to the same port as CF), so the right place
>>>>>>seems to rather be in [mu]dma_filter() methods -- and the decision
>>>>>>should be strictly based on the drive type indicating CF, i.e. by
>>>>>>calling ata_id_is_cfa().

>>I have tried my old Trancend 64Mb, RamStar 521Mb and NCP 64Mb cards. My old
>>cards returned right id[ATA_ID_CONFIG] = 0x848A.

>>But I have to use Kingston CF Card 1Gb 2008.
>>ata_id_is_cfa() returns 0 for it and
>>id[ATA_ID_MAJOR_VER] = 0
>>id[ATA_ID_CONFIG] = 0x044A

>>I have only CF+ specification revision 2.0, but I've found in wiki:
>>(http://en.wikipedia.org/wiki/CompactFlash#CF.2B_specification_revisions)
>>"... While the current revision 4.1 from 2004 works only in ATA mode, ..."

>>So I have reached an impasse. How to identify modern CF cards?

> Sorry that I missed it before but if indeed normal IDE devices/connectors can
> be used with IDE2 then I see no sane/reliable way to detect CF devices using
> buggy on-board slot... unless this slot is hardwired to be master (or slave)?

Selectable via switch. However, it's not very probable that user will plug
in CF adapter into the secondary channel (and it will have the option of using
on on the primary anyway).

> Thanks,
> Bart

MBR, Sergei