2008-08-03 15:57:51

by James Bottomley

[permalink] [raw]
Subject: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Right at the moment, we have two separate subsystems for running IDE
type devices: driver/ide and drivers/ata. The claim I've seen is that
drivers/ata can do everything drivers/ide can do plus it does sata. I
also note that no major distribution seems to enable anything in
drivers/ide anymore, so given this is it time to deprecate drivers/ide?

A counter argument to the above is that not all drivers (particularly
the older ones where hw is scarce) are converted to drivers/ata, so
drivers/ide seems to be needed for some legacy systems (in which case it
can be deprecated but not removed). I've also noted that some embedded
distributions seem to be using drivers/ide, but I'm not really sure
whether this is inertia or some overriding need.

The proposal is to discuss the future of these two subsystems and arrive
at a consensus what's happening to each going forwards.

James


2008-08-03 16:57:15

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, 03 Aug 2008 10:57:35 -0500
James Bottomley <[email protected]> wrote:

> Right at the moment, we have two separate subsystems for running IDE
> type devices: driver/ide and drivers/ata. The claim I've seen is that
> drivers/ata can do everything drivers/ide can do plus it does sata. I
> also note that no major distribution seems to enable anything in
> drivers/ide anymore, so given this is it time to deprecate drivers/ide?
>
> A counter argument to the above is that not all drivers (particularly
> the older ones where hw is scarce) are converted to drivers/ata, so

That statement would be false. In fact for old and obscure hw the libata
coverage is probably better, not that this in fact is the slighest bit
relevant to the real world!

The current situation is something like this

- For PC class hardware libata covers everything in old IDE and a lot
more.
- For the older Mac hardware libata does not have pre PCI Macintosh
support although it is in progress
- For certain embedded PPC boards there are some things to sort out with
IRQ routing on non standards sane configurations using pata_ali in
particular.

There is a trend in new hardware to interfaces that simply won't fit old
IDE so that process of libata only drivers is likely to continue.

> drivers/ide seems to be needed for some legacy systems (in which case it
> can be deprecated but not removed). I've also noted that some embedded
> distributions seem to be using drivers/ide, but I'm not really sure
> whether this is inertia or some overriding need.

Actually what a material number of embedded systems need is a single
dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of
midlayer code. I'm just not sure that trend will continue as CF is giving
way to other smaller media.

> The proposal is to discuss the future of these two subsystems and arrive
> at a consensus what's happening to each going forwards.

Won't be at KS and I suspect that is true of most of the folks hacking on
both sets so the right place for discussion would be the net. Right now
the PPC bits need fixing, and the lack of libata Macio support for the
moment makes the debate premature.

Alan

2008-08-03 17:10:49

by James Bottomley

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, 2008-08-03 at 17:39 +0100, Alan Cox wrote:
> On Sun, 03 Aug 2008 10:57:35 -0500
> James Bottomley <[email protected]> wrote:
>
> > Right at the moment, we have two separate subsystems for running IDE
> > type devices: driver/ide and drivers/ata. The claim I've seen is that
> > drivers/ata can do everything drivers/ide can do plus it does sata. I
> > also note that no major distribution seems to enable anything in
> > drivers/ide anymore, so given this is it time to deprecate drivers/ide?
> >
> > A counter argument to the above is that not all drivers (particularly
> > the older ones where hw is scarce) are converted to drivers/ata, so
>
> That statement would be false. In fact for old and obscure hw the libata
> coverage is probably better, not that this in fact is the slighest bit
> relevant to the real world!
>
> The current situation is something like this

Hmm, OK, but I did see (as you note below) that mac ide doesn't work
with drivers/ata.

> - For PC class hardware libata covers everything in old IDE and a lot
> more.
> - For the older Mac hardware libata does not have pre PCI Macintosh
> support although it is in progress
> - For certain embedded PPC boards there are some things to sort out with
> IRQ routing on non standards sane configurations using pata_ali in
> particular.
>
> There is a trend in new hardware to interfaces that simply won't fit old
> IDE so that process of libata only drivers is likely to continue.

Yes, that's SATA ... but legacy remains for a long time.

> > drivers/ide seems to be needed for some legacy systems (in which case it
> > can be deprecated but not removed). I've also noted that some embedded
> > distributions seem to be using drivers/ide, but I'm not really sure
> > whether this is inertia or some overriding need.
>
> Actually what a material number of embedded systems need is a single
> dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of
> midlayer code. I'm just not sure that trend will continue as CF is giving
> way to other smaller media.

Yes, I understand this ... if that means drivers/ata can't possibly be
used by embedded until its dependence on SCSI is broken, then so be
it ... but I think we should at least investigate what the issues and
potential solutions are.

> > The proposal is to discuss the future of these two subsystems and arrive
> > at a consensus what's happening to each going forwards.
>
> Won't be at KS and I suspect that is true of most of the folks hacking on
> both sets so the right place for discussion would be the net. Right now
> the PPC bits need fixing, and the lack of libata Macio support for the
> moment makes the debate premature.

Well, I did notice from the lists that both drivers/ide and drivers/ata
would be represented. I've also observed that there's been a singular
lack of discussion of this on the lists; plus it's the type of emotive
issue that in-person discussions help to extract the heat from.

James

2008-08-03 17:34:24

by Willy Tarreau

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, Aug 03, 2008 at 05:39:41PM +0100, Alan Cox wrote:
> Actually what a material number of embedded systems need is a single
> dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of
> midlayer code. I'm just not sure that trend will continue as CF is giving
> way to other smaller media.

I bet we'll see CF for a long time, because it is the only cheap
removable media to support IDE emulation and be bootable from an
unmodified PC BIOS. SD is cheaper and will soon provide larger
capacities, but it still requires a separate controller and is
not bootable with standard BIOSes.

Anyway, in my experience, libata supports CF on embedded controllers
pretty well. So maybe we should start marking IDE drivers deprecated
to encourage people to try libata instead and report breakage if any.

The real issue on embedded systems is that the devices names change.
On the one hand, it increases portability across models, because you
don't have to care anymore about hda/hdc/hdd, your CF is always on
sda (major reason why I'm considering a switch BTW). However, for
systems which were used to always see system disk on hdX and data
on sd* (USB, SATA or SCSI), it might take more time to switch due
to automatic numbering.

Regards,
Willy

2008-08-03 17:42:32

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sunday, 3 of August 2008, Willy Tarreau wrote:
> On Sun, Aug 03, 2008 at 05:39:41PM +0100, Alan Cox wrote:
> > Actually what a material number of embedded systems need is a single
> > dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of
> > midlayer code. I'm just not sure that trend will continue as CF is giving
> > way to other smaller media.
>
> I bet we'll see CF for a long time, because it is the only cheap
> removable media to support IDE emulation and be bootable from an
> unmodified PC BIOS. SD is cheaper and will soon provide larger
> capacities, but it still requires a separate controller and is
> not bootable with standard BIOSes.
>
> Anyway, in my experience, libata supports CF on embedded controllers
> pretty well. So maybe we should start marking IDE drivers deprecated
> to encourage people to try libata instead and report breakage if any.

Well, I know of at least one case of PC hardware in which an old IDE driver
basically works while the libata/pata poeple have no idea why their driver
doesn't:

http://bugzilla.kernel.org/show_bug.cgi?id=9157

Thanks,
Rafael

2008-08-03 17:57:39

by Willy Tarreau

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, Aug 03, 2008 at 07:45:03PM +0200, Rafael J. Wysocki wrote:
> > Anyway, in my experience, libata supports CF on embedded controllers
> > pretty well. So maybe we should start marking IDE drivers deprecated
> > to encourage people to try libata instead and report breakage if any.
>
> Well, I know of at least one case of PC hardware in which an old IDE driver
> basically works while the libata/pata poeple have no idea why their driver
> doesn't:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=9157

Interesting. I have a board with such a SIS controller somewhere here.
I'd have to shuffle the dust out of it and see if it still works, maybe
I can help on this one.

Regards,
Willy

2008-08-03 19:02:26

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> > There is a trend in new hardware to interfaces that simply won't fit old
> > IDE so that process of libata only drivers is likely to continue.
>
> Yes, that's SATA ... but legacy remains for a long time.

It isn't that simple. The newer PATA controllers are diverging from the
old style interfaces as well.

> Yes, I understand this ... if that means drivers/ata can't possibly be
> used by embedded until its dependence on SCSI is broken, then so be
> it ... but I think we should at least investigate what the issues and
> potential solutions are.

The simpler embedded wants a pure CF driver - no scsi, no libata, no
drivers/ide overhead, just about 4K of CF only pio driver.

> Well, I did notice from the lists that both drivers/ide and drivers/ata
> would be represented. I've also observed that there's been a singular
> lack of discussion of this on the lists; plus it's the type of emotive
> issue that in-person discussions help to extract the heat from.

We've certainly discussed it in the past and I'm not sure its gotten that
personal - many people contribute to both trees for example, or are
directly cc'ing stuff between trees to ensure they stay in sync and any
useful info gets in both.

Anyway with my libata hat on I would say its far too early to worry about
this. Right now we have platforms that need old IDE and it is still
proving very useful to be able to ask people to try both. I don't quite
know why Bart is putting so much effort into polishing up the old IDE but
if he's enjoying it I don't really care 8)

Alan

2008-08-03 19:21:38

by James Bottomley

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, 2008-08-03 at 19:45 +0100, Alan Cox wrote:
> > > There is a trend in new hardware to interfaces that simply won't fit old
> > > IDE so that process of libata only drivers is likely to continue.
> >
> > Yes, that's SATA ... but legacy remains for a long time.
>
> It isn't that simple. The newer PATA controllers are diverging from the
> old style interfaces as well.
>
> > Yes, I understand this ... if that means drivers/ata can't possibly be
> > used by embedded until its dependence on SCSI is broken, then so be
> > it ... but I think we should at least investigate what the issues and
> > potential solutions are.
>
> The simpler embedded wants a pure CF driver - no scsi, no libata, no
> drivers/ide overhead, just about 4K of CF only pio driver.

I'd be a bit surprised if that's anything other than a niche. PIO is
deadly slow and wasteful of precious CPU cycles ... and embedded devices
seem to be interested in performance as well (which the DMA modes
bring).

> > Well, I did notice from the lists that both drivers/ide and drivers/ata
> > would be represented. I've also observed that there's been a singular
> > lack of discussion of this on the lists; plus it's the type of emotive
> > issue that in-person discussions help to extract the heat from.
>
> We've certainly discussed it in the past and I'm not sure its gotten that
> personal - many people contribute to both trees for example, or are
> directly cc'ing stuff between trees to ensure they stay in sync and any
> useful info gets in both.

My observation was merely that I hadn't seen it discussed.

> Anyway with my libata hat on I would say its far too early to worry about
> this. Right now we have platforms that need old IDE and it is still
> proving very useful to be able to ask people to try both. I don't quite
> know why Bart is putting so much effort into polishing up the old IDE but
> if he's enjoying it I don't really care 8)

Hey, I'd be the last person to discourage anyone from maintaining
obsolete systems ... however, the outside world looking in on this is
interested to know where they should be starting if they have some type
of CF/IDE device to support. What we really want to avoid is someone
spending months on a device driver only to have us turn around and
deprecate the subsystem.

I don't envisage anything in terms of code changes (or removal) coming
out of this. What I would like is a nice roadmap of where to start for
people contemplating writing IDE drivers.

James

2008-08-03 19:34:42

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> I don't envisage anything in terms of code changes (or removal) coming
> out of this. What I would like is a nice roadmap of where to start for
> people contemplating writing IDE drivers.

For the vendors I've talked with the answer is usually "both". A libata
driver for the future and current distributions, and an old IDE driver
for older RHEL and SLES releases. Fortunately the driver specific parts
for those that are capable of running under old IDE are not too hard to
plug into libata.

Alan

Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

I'm using gmail's interface (I don't have access to my laptop ATM) so
the mail may look a bit weird...

On Sun, Aug 3, 2008 at 5:57 PM, James Bottomley
<[email protected]> wrote:
> Right at the moment, we have two separate subsystems for running IDE
> type devices: driver/ide and drivers/ata. The claim I've seen is that
> drivers/ata can do everything drivers/ide can do plus it does sata. I

This claim doesn't seem to have confirmation in facts:

* There is still hardware that is simply not-supported by libata at all:

- architecture specific hardware (ppc, m68k, mips, arm)

- "difficult" legacy PC-class hardware (i.e. secondary interface on
CY82C693 etc)

* There are still regressions in many libata PCI host drivers dating back
to their rushed introduction.

* There are still corner case in libata core - PIO is dead slow
compared to drivers/ide/,
"serialized" hosts are not supported, some quirks for obsolete
hardware got lost...

> also note that no major distribution seems to enable anything in
> drivers/ide anymore, so given this is it time to deprecate drivers/ide?

Major distributions make their own decisions (I don't remeber anybody from
these distros discussing the conversion on linux-kernel or linux-ide) which
sometimes don't match with what kernel.org kernels are doing.

[ Actually one distro went so far as CONFIG_IDE=n even before support for
all PC-class IDE PCI hardware present in drivers/ide was available
in libata. ]

Also the same major distros that use libata on x86 are using
drivers/ide on non-x86.

> A counter argument to the above is that not all drivers (particularly
> the older ones where hw is scarce) are converted to drivers/ata, so
> drivers/ide seems to be needed for some legacy systems (in which case it
> can be deprecated but not removed). I've also noted that some embedded
> distributions seem to be using drivers/ide, but I'm not really sure
> whether this is inertia or some overriding need.

drivers/ide deprecation would be a premature thing.

> The proposal is to discuss the future of these two subsystems and arrive
> at a consensus what's happening to each going forwards.

Well, I'm looking forward to discuss the future of Linux ATA support.

Thanks,
Bart

Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, Aug 3, 2008 at 9:17 PM, Alan Cox <[email protected]> wrote:
>> I don't envisage anything in terms of code changes (or removal) coming
>> out of this. What I would like is a nice roadmap of where to start for
>> people contemplating writing IDE drivers.
>
> For the vendors I've talked with the answer is usually "both". A libata
> driver for the future and current distributions, and an old IDE driver
> for older RHEL and SLES releases. Fortunately the driver specific parts
> for those that are capable of running under old IDE are not too hard to
> plug into libata.

The contrary is also true, for modern drivers/ide conversion of libata PATA
driver to use drivers/ide should usually be a matter of an hour or two.

I think that the conversion of libata drivers for PATA hardware that is not
supported by drivers/ide currently should be doable by somebody knowing
both subsystems in less than 24h. I'm not doing it only because it will make
me a maintainer of even more host drivers than currently and I don't have a
time for it. I'll happily accept patches though.

Thanks,
Bart

2008-08-03 20:23:43

by David Woodhouse

[permalink] [raw]
Subject: Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, 2008-08-03 at 19:45 +0200, Rafael J. Wysocki wrote:
> Well, I know of at least one case of PC hardware in which an old IDE driver
> basically works while the libata/pata poeple have no idea why their driver
> doesn't:

I have one of those too (pdc202xx craps itself with the new drivers).

--
dwmw2

2008-08-03 22:18:49

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> - "difficult" legacy PC-class hardware (i.e. secondary interface on
> CY82C693 etc)

Yep - probably an hours work but I've simply not been able to get the
relevant hardware to test.

> * There are still regressions in many libata PCI host drivers dating back
> to their rushed introduction.

I don't think we have any that are real straight forward "you
forgot something" regressions now. There are a few "works with A not B"
cases in both directions that we'll probably take ages to figure out.

> * There are still corner case in libata core - PIO is dead slow
> compared to drivers/ide/,

There are two there - libata keeps IRQs blocked for longer in PIO mode as
well which is a factor for realtime that needs looking at, as well as
using 16bit not 32bit I/O for most devices (which is trivial to fix). The
IRQ masking stuff is more complex and old IDE handles it far better for
PIO on non shared IRQ interfaces. That is actually probably the most
complicated thing to address of the stuff you'd want to do if you were
going to kill off old IDE.

> "serialized" hosts are not supported, some quirks for obsolete
> hardware got lost...

Serialized hardware has been supported since 2.6.24 or so. Not sure there
are any quirks for obsolete hardware now missing but its always fun
trying to nail them all. Libata can support per command serializing and
pretty much arbitarily complex serializing rules via qc_defer.

> > The proposal is to discuss the future of these two subsystems and arrive
> > at a consensus what's happening to each going forwards.
>
> Well, I'm looking forward to discuss the future of Linux ATA support.

Have fun, but don't expect me to respect any decisions made off public
lists. I don't really see any point worrying about it - porting m68k to
libata is of minimal value for example and there is little likelyhood of
Amiga AHCI SATA suddenely appearing.

Alan

2008-08-03 22:25:03

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> The contrary is also true, for modern drivers/ide conversion of libata PATA
> driver to use drivers/ide should usually be a matter of an hour or two.

Sounds about right for a typical device based upon the ones I did that
way.

2008-08-03 23:10:40

by Jeff Garzik

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

James Bottomley wrote:
> Right at the moment, we have two separate subsystems for running IDE
> type devices: driver/ide and drivers/ata. The claim I've seen is that
> drivers/ata can do everything drivers/ide can do plus it does sata. I
> also note that no major distribution seems to enable anything in
> drivers/ide anymore, so given this is it time to deprecate drivers/ide?
>
> A counter argument to the above is that not all drivers (particularly
> the older ones where hw is scarce) are converted to drivers/ata, so
> drivers/ide seems to be needed for some legacy systems (in which case it
> can be deprecated but not removed). I've also noted that some embedded
> distributions seem to be using drivers/ide, but I'm not really sure
> whether this is inertia or some overriding need.
>
> The proposal is to discuss the future of these two subsystems and arrive
> at a consensus what's happening to each going forwards.

I'm not in any rush to change the status quo as I see it: don't remove
drivers/ide but encourage new drivers to be under libata.

I am a bit disappointed at all the drivers/ide churn. I had hoped it
would sit around and be a stable alternative, a fallback to libata.

Jeff


2008-08-04 05:35:31

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Sun, 2008-08-03 at 17:39 +0100, Alan Cox wrote:
> On Sun, 03 Aug 2008 10:57:35 -0500
> James Bottomley <[email protected]> wrote:

> - For PC class hardware libata covers everything in old IDE and a lot
> more.
> - For the older Mac hardware libata does not have pre PCI Macintosh
> support although it is in progress
> - For certain embedded PPC boards there are some things to sort out with
> IRQ routing on non standards sane configurations using pata_ali in
> particular.
>
> There is a trend in new hardware to interfaces that simply won't fit old
> IDE so that process of libata only drivers is likely to continue.

Actually, libata is missing anything using the Apple proprietary (I call
it "macio") IDE controller. That includes pretty much all Macs with IDE
not only pre-PCI. IE, it's used for the CD-ROM on the G5's for example
(the disks use sata_svw).

I'm working on it though :-)

> Won't be at KS and I suspect that is true of most of the folks hacking on
> both sets so the right place for discussion would be the net. Right now
> the PPC bits need fixing, and the lack of libata Macio support for the
> moment makes the debate premature.

Hopefully that will be fixed by .28

Cheers,
Ben.

2008-08-04 13:17:19

by Kumar Gala

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE


On Aug 3, 2008, at 11:39 AM, Alan Cox wrote:

> - For certain embedded PPC boards there are some things to sort out
> with
> IRQ routing on non standards sane configurations using pata_ali in
> particular.

Alan, do you have any specifics here?

- k

2008-08-04 13:25:41

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Mon, 4 Aug 2008 08:16:47 -0500
Kumar Gala <[email protected]> wrote:

>
> On Aug 3, 2008, at 11:39 AM, Alan Cox wrote:
>
> > - For certain embedded PPC boards there are some things to sort out
> > with
> > IRQ routing on non standards sane configurations using pata_ali in
> > particular.
>
> Alan, do you have any specifics here?

Some people are running PPC setups which report the IDE controller as in
native mode, the PCI IRQ as zero and in fact are using legacy IRQ lines.

2008-08-04 20:07:21

by Robert Hancock

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Alan Cox wrote:
>> * There are still corner case in libata core - PIO is dead slow
>> compared to drivers/ide/,
>
> There are two there - libata keeps IRQs blocked for longer in PIO mode as
> well which is a factor for realtime that needs looking at, as well as
> using 16bit not 32bit I/O for most devices (which is trivial to fix). The
> IRQ masking stuff is more complex and old IDE handles it far better for
> PIO on non shared IRQ interfaces. That is actually probably the most
> complicated thing to address of the stuff you'd want to do if you were
> going to kill off old IDE.

I was looking into the 32-bit PIO issue a bit yesterday. It looks like
some of the VLB libata drivers are doing this internally already, so it
shouldn't be hard to do this in the core. Only question is how we know
generically if the controller can do it or not? It looks like in old
IDE, a few controllers explicitly disable it, but it appears that it
doesn't default to on for any controller, so it's possible there are
others on which it doesn't work. Presumably anything on an actual 16-bit
bus (ISA, LPC, etc.) wouldn't like it, to start with.

There's also the matter of the identify bit to indicate whether the
drive supports 32-bit transfers, which was reallocated to trusted
computing in ATA-8 so any drive matching that standard will indicate not
supported. I couldn't track down where that bit was actually defined in
the first place, all the way back to ATA-1 it seems to be indicated as
reserved. Actually, I'm not sure why the drive cares in the first place,
it would seem like a pure host controller issue..

2008-08-04 20:12:36

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> I was looking into the 32-bit PIO issue a bit yesterday. It looks like
> some of the VLB libata drivers are doing this internally already, so it
> shouldn't be hard to do this in the core. Only question is how we know
> generically if the controller can do it or not? It looks like in old

You don't. Basically it is controller dependant. Pretty much all the
newer controllers support the 32bit PIO data cycles. Most PCI controllers
it makes no speed difference but host bus controllers (especially
PIIX/ICH) really benefit.

> supported. I couldn't track down where that bit was actually defined in
> the first place, all the way back to ATA-1 it seems to be indicated as
> reserved. Actually, I'm not sure why the drive cares in the first place,
> it would seem like a pure host controller issue..

It goes back before IDE into the depths of the original compaq spec. When
you have a device wired basically directly to the ISA bus (original IDE)
it mattered. I don't believe it is relevant to any of the PCI controllers.

Alan

2008-08-04 20:38:31

by Jeff Garzik

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Robert Hancock wrote:
> Alan Cox wrote:
>>> * There are still corner case in libata core - PIO is dead slow
>>> compared to drivers/ide/,
>>
>> There are two there - libata keeps IRQs blocked for longer in PIO mode as
>> well which is a factor for realtime that needs looking at, as well as
>> using 16bit not 32bit I/O for most devices (which is trivial to fix). The
>> IRQ masking stuff is more complex and old IDE handles it far better for
>> PIO on non shared IRQ interfaces. That is actually probably the most
>> complicated thing to address of the stuff you'd want to do if you were
>> going to kill off old IDE.
>
> I was looking into the 32-bit PIO issue a bit yesterday. It looks like
> some of the VLB libata drivers are doing this internally already, so it
> shouldn't be hard to do this in the core. Only question is how we know
> generically if the controller can do it or not? It looks like in old
> IDE, a few controllers explicitly disable it, but it appears that it
> doesn't default to on for any controller, so it's possible there are
> others on which it doesn't work. Presumably anything on an actual 16-bit
> bus (ISA, LPC, etc.) wouldn't like it, to start with.

FWIW there is already a patch from Willy Terreau (sp?) to add 32-bit I/O.

I queued it for "later" because it had some issues that Alan pointed
out, IIRC. I definitely want to push it in, though.

Jeff


2008-08-04 21:18:12

by Robert Hancock

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Alan Cox wrote:
>> I was looking into the 32-bit PIO issue a bit yesterday. It looks like
>> some of the VLB libata drivers are doing this internally already, so it
>> shouldn't be hard to do this in the core. Only question is how we know
>> generically if the controller can do it or not? It looks like in old
>
> You don't. Basically it is controller dependant. Pretty much all the
> newer controllers support the 32bit PIO data cycles. Most PCI controllers
> it makes no speed difference but host bus controllers (especially
> PIIX/ICH) really benefit.
>
>> supported. I couldn't track down where that bit was actually defined in
>> the first place, all the way back to ATA-1 it seems to be indicated as
>> reserved. Actually, I'm not sure why the drive cares in the first place,
>> it would seem like a pure host controller issue..
>
> It goes back before IDE into the depths of the original compaq spec. When
> you have a device wired basically directly to the ISA bus (original IDE)
> it mattered. I don't believe it is relevant to any of the PCI controllers.

I guess that bit doesn't really make any difference with remotely modern
drives, then.. Could we make that ata_id_has_dword_io check always
return true if ata_id_is_ata returns true and only check word 48 if not?

I saw Willy Tarreau's patch from February for this, I agree that we
should likely use a separate data_xfer method for 32-bit transfer (or if
enough controllers should support 32-bit, then just make it be the
default and make a separate 16-bit only function for those that don't),
rather than punting the decision to the user with hdparm.

You mentioned in the thread for Willy's patch that "some
controllers have quirky rules for 32bit xfers" - any details anywhere?

2008-08-04 21:27:44

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> I guess that bit doesn't really make any difference with remotely modern
> drives, then.. Could we make that ata_id_has_dword_io check always
> return true if ata_id_is_ata returns true and only check word 48 if not?

I suspect (but need to dig further) that ata_id_has_dword_io should only
be called by pata_legacy.

> I saw Willy Tarreau's patch from February for this, I agree that we
> should likely use a separate data_xfer method for 32-bit transfer (or if
> enough controllers should support 32-bit, then just make it be the
> default and make a separate 16-bit only function for those that don't),
> rather than punting the decision to the user with hdparm.

Definitely.

> You mentioned in the thread for Willy's patch that "some
> controllers have quirky rules for 32bit xfers" - any details anywhere?

There are two main ones

- Some controllers only support 32bit I/O for a multiple of 32bit values
[sometimes 'unless the fifo is disabled']. I'd have to go back over the
docs but I think the AMD may be one of those
- Some controllers (VLB generally) require a magic sequence before the
transfer. You'll see that in the pata_legacy bits.

Alan

2008-08-04 21:55:14

by Robert Hancock

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Alan Cox wrote:
>> You mentioned in the thread for Willy's patch that "some
>> controllers have quirky rules for 32bit xfers" - any details anywhere?
>
> There are two main ones
>
> - Some controllers only support 32bit I/O for a multiple of 32bit values
> [sometimes 'unless the fifo is disabled']. I'd have to go back over the
> docs but I think the AMD may be one of those

The AMD-766 doc I have says that when the Secondary Posted Write Buffer
or Primary Posted Write Buffer are enabled, only 32-bit writes are
allowed to the data port. It doesn't say anything about a restriction
with the read prefetch buffer though.

I guess it depends if any other controllers could potentially have this
restriction. I suspect non-multiple-of-32-bit transfers are rare enough
we could just fall back to 16-bit IO always for them, but maybe not.

> - Some controllers (VLB generally) require a magic sequence before the
> transfer. You'll see that in the pata_legacy bits.
>
> Alan
>

2008-08-04 21:57:00

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Hello.

Alan Cox wrote:
>> I was looking into the 32-bit PIO issue a bit yesterday. It looks like
>> some of the VLB libata drivers are doing this internally already, so it
>> shouldn't be hard to do this in the core. Only question is how we know
>> generically if the controller can do it or not? It looks like in old
>>
>
> You don't. Basically it is controller dependant. Pretty much all the
> newer controllers support the 32bit PIO data cycles. Most PCI controllers
> it makes no speed difference but host bus controllers (especially
> PIIX/ICH) really benefit.
>

In what way if there's no speed gain?

>> supported. I couldn't track down where that bit was actually defined in
>> the first place, all the way back to ATA-1 it seems to be indicated as
>> reserved. Actually, I'm not sure why the drive cares in the first place,
>> it would seem like a pure host controller issue..
>>
>
> It goes back before IDE into the depths of the original compaq spec. When
> you have a device wired basically directly to the ISA bus (original IDE)
>

ISA has only 8/16-bit data bus, so it could not have mattered
there... and I don't think that there ever was a direct connection
between the IDE and host bus other than ISA... except maybe EISA.

MBR, Sergei

2008-08-04 22:01:28

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> > newer controllers support the 32bit PIO data cycles. Most PCI controllers
> > it makes no speed difference but host bus controllers (especially
> > PIIX/ICH) really benefit.
> >
>
> In what way if there's no speed gain?

As in the numbers are the same before and after. The FIFO on the
controller is happily hiding the extra latencies I assume.

> >> supported. I couldn't track down where that bit was actually defined in
> >> the first place, all the way back to ATA-1 it seems to be indicated as
> >> reserved. Actually, I'm not sure why the drive cares in the first place,
> >> it would seem like a pure host controller issue..
> >>
> >
> > It goes back before IDE into the depths of the original compaq spec. When
> > you have a device wired basically directly to the ISA bus (original IDE)
> >
>
> ISA has only 8/16-bit data bus, so it could not have mattered
> there...

Depends what a 32bit I/O looks like on the 16bit bus - timing wise.

2008-08-04 22:13:08

by Mark Lord

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Alan Cox wrote:
> You don't. Basically it is controller dependant. Pretty much all the
> newer controllers support the 32bit PIO data cycles. Most PCI controllers
> it makes no speed difference but host bus controllers (especially
> PIIX/ICH) really benefit.
..

By "most PCI controllers", did you really mean "most add-on PCI (card) controllers"?

Nearly all onboard "PCI" controllers (integrated into the host chipset) benefit hugely,
but I suppose that is what was meant by "host bus controllers".

Cheers

2008-08-04 22:17:42

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

On Mon, 04 Aug 2008 18:12:51 -0400
Mark Lord <[email protected]> wrote:

> Alan Cox wrote:
> > You don't. Basically it is controller dependant. Pretty much all the
> > newer controllers support the 32bit PIO data cycles. Most PCI controllers
> > it makes no speed difference but host bus controllers (especially
> > PIIX/ICH) really benefit.
> ..
>
> By "most PCI controllers", did you really mean "most add-on PCI (card) controllers"?

Yes

> Nearly all onboard "PCI" controllers (integrated into the host chipset) benefit hugely,
> but I suppose that is what was meant by "host bus controllers".

Yes

2008-08-04 22:46:52

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Hello.

Alan Cox wrote:
>>> newer controllers support the 32bit PIO data cycles. Most PCI controllers
>>> it makes no speed difference but host bus controllers (especially
>>> PIIX/ICH) really benefit.
>>>

PIIX was a pure PCI controller, IIRC. ICH is also not a "host bus"
controller, it hangs off the I/O hub bus...

>>>
>>>
>> In what way if there's no speed gain?
>>
>
> As in the numbers are the same before and after. The FIFO on the
> controller is happily hiding the extra latencies I assume.
>

Depends on the PIO mode -- in the lower ones, the prefetch reads
might really be slower than successive reads on the host bus...

>>>> supported. I couldn't track down where that bit was actually defined in
>>>> the first place, all the way back to ATA-1 it seems to be indicated as
>>>> reserved. Actually, I'm not sure why the drive cares in the first place,
>>>> it would seem like a pure host controller issue..
>>>>
>>>>
>>> It goes back before IDE into the depths of the original compaq spec. When
>>> you have a device wired basically directly to the ISA bus (original IDE)
>>>
>>>
>> ISA has only 8/16-bit data bus, so it could not have mattered
>> there...
>>
>
> Depends what a 32bit I/O looks like on the 16bit bus - timing wise.
>

Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed
recovery time, IIRC... It's just occured to me that in case of the
16-bit bus it should be how the drive treated the accesses at address
0x1x2 with IOCS16 asserted that could have mattered. If it honored them,
32-bit I/O could have worked even on a dumb ISA "controller", if not --
no way (unless you really had *something* between the ISA and the IDE
cable).

MBR, Sergei

2008-08-04 22:53:31

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Hello, I wrote:
>>>>> supported. I couldn't track down where that bit was actually
>>>>> defined in the first place, all the way back to ATA-1 it seems to
>>>>> be indicated as reserved. Actually, I'm not sure why the drive
>>>>> cares in the first place, it would seem like a pure host
>>>>> controller issue..
>>>>>
>>>> It goes back before IDE into the depths of the original compaq
>>>> spec. When
>>>> you have a device wired basically directly to the ISA bus (original
>>>> IDE)
>>>>
>>> ISA has only 8/16-bit data bus, so it could not have mattered
>>> there...
>>
>> Depends what a 32bit I/O looks like on the 16bit bus - timing wise.
>>
>
> Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed
> recovery time, IIRC... It's just occured to me that in case of the
> 16-bit bus it should be how the drive treated the accesses at address
> 0x1x2 with IOCS16 asserted that could have mattered. If it honored
> them, 32-bit I/O could have worked even on a dumb ISA "controller", if
> not -- no way (unless you really had *something* between the ISA and
> the IDE cable).

Oh, -IOCS16 is driven by device, not host. I give up then. :-)

MBR, Sergei

2008-08-04 23:50:01

by Tejun Heo

[permalink] [raw]
Subject: Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE

Jeff Garzik wrote:
> Robert Hancock wrote:
>> Alan Cox wrote:
>>>> * There are still corner case in libata core - PIO is dead slow
>>>> compared to drivers/ide/,
>>> There are two there - libata keeps IRQs blocked for longer in PIO mode as
>>> well which is a factor for realtime that needs looking at, as well as
>>> using 16bit not 32bit I/O for most devices (which is trivial to fix). The
>>> IRQ masking stuff is more complex and old IDE handles it far better for
>>> PIO on non shared IRQ interfaces. That is actually probably the most
>>> complicated thing to address of the stuff you'd want to do if you were
>>> going to kill off old IDE.
>> I was looking into the 32-bit PIO issue a bit yesterday. It looks like
>> some of the VLB libata drivers are doing this internally already, so it
>> shouldn't be hard to do this in the core. Only question is how we know
>> generically if the controller can do it or not? It looks like in old
>> IDE, a few controllers explicitly disable it, but it appears that it
>> doesn't default to on for any controller, so it's possible there are
>> others on which it doesn't work. Presumably anything on an actual 16-bit
>> bus (ISA, LPC, etc.) wouldn't like it, to start with.
>
> FWIW there is already a patch from Willy Terreau (sp?) to add 32-bit I/O.
>
> I queued it for "later" because it had some issues that Alan pointed
> out, IIRC. I definitely want to push it in, though.

Jeff, have your thoughts about PIO IRQ disable handling changed yet? I
don't really see any better way than doing it like IDE.

Thanks.

--
tejun

2008-08-06 00:23:15

by Robert Hancock

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Alan Cox wrote:
>> I guess that bit doesn't really make any difference with remotely modern
>> drives, then.. Could we make that ata_id_has_dword_io check always
>> return true if ata_id_is_ata returns true and only check word 48 if not?
>
> I suspect (but need to dig further) that ata_id_has_dword_io should only
> be called by pata_legacy.
>
>> I saw Willy Tarreau's patch from February for this, I agree that we
>> should likely use a separate data_xfer method for 32-bit transfer (or if
>> enough controllers should support 32-bit, then just make it be the
>> default and make a separate 16-bit only function for those that don't),
>> rather than punting the decision to the user with hdparm.
>
> Definitely.
>
>> You mentioned in the thread for Willy's patch that "some
>> controllers have quirky rules for 32bit xfers" - any details anywhere?
>
> There are two main ones
>
> - Some controllers only support 32bit I/O for a multiple of 32bit values
> [sometimes 'unless the fifo is disabled']. I'd have to go back over the
> docs but I think the AMD may be one of those
> - Some controllers (VLB generally) require a magic sequence before the
> transfer. You'll see that in the pata_legacy bits.

Here's my first cut at it. Compile tested only. This sets most controllers
to use 32-bit PIO except for those which could potentially be on a real ISA
or other 16-bit bus. It's a bit non-obvious what to do with some of the
drivers, so input is welcome.

This implementation doesn't check the ata_id_has_dword_io at all, since it
would only make a difference on controllers where we don't really want to
use it anyway.

It seems like regardless of whether we do 32-bit by default or not the 32-bit
data_xfer function should be added to libata core as we have several drivers
which duplicate the same code currently..

Signed-off-by: Robert Hancock <[email protected]>

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 304fdc6..acb65a9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -673,13 +673,14 @@ static inline void ata_tf_to_host(struct ata_port *ap,
}

/**
- * ata_sff_data_xfer - Transfer data by PIO
+ * ata_sff_data_xfer_16bit - Transfer data by PIO
* @dev: device to target
* @buf: data buffer
* @buflen: buffer length
* @rw: read/write
*
- * Transfer data from/to the device data register by PIO.
+ * Transfer data from/to the device data register by PIO using only
+ * 16-bit transfers.
*
* LOCKING:
* Inherited from caller.
@@ -687,7 +688,7 @@ static inline void ata_tf_to_host(struct ata_port *ap,
* RETURNS:
* Bytes consumed.
*/
-unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
+unsigned int ata_sff_data_xfer_16bit(struct ata_device *dev, unsigned char *buf,
unsigned int buflen, int rw)
{
struct ata_port *ap = dev->link->ap;
@@ -719,6 +720,51 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
}

/**
+ * ata_sff_data_xfer - Transfer data by PIO
+ * @dev: device to target
+ * @buf: data buffer
+ * @buflen: buffer length
+ * @rw: read/write
+ *
+ * Transfer data from/to the device data register by PIO.
+ *
+ * LOCKING:
+ * Inherited from caller.
+ *
+ * RETURNS:
+ * Bytes consumed.
+ */
+unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
+ unsigned int buflen, int rw)
+{
+ struct ata_port *ap = dev->link->ap;
+ void __iomem *data_addr = ap->ioaddr.data_addr;
+ unsigned int words = buflen >> 2;
+ unsigned int slop = buflen & 3;
+
+ /* Transfer multiple of 4 bytes */
+ if (rw == READ)
+ ioread32_rep(data_addr, buf, words);
+ else
+ iowrite32_rep(data_addr, buf, words);
+
+ /* Transfer trailing 1 byte, if any. */
+ if (unlikely(slop)) {
+ __le32 pad = 0;
+ if (rw == READ) {
+ pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
+ memcpy(buf + buflen - slop, &pad, slop);
+ } else {
+ memcpy(&pad, buf + buflen - slop, slop);
+ iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
+ }
+ buflen += 4 - slop;
+ }
+
+ return buflen;
+}
+
+/**
* ata_sff_data_xfer_noirq - Transfer data by PIO
* @dev: device to target
* @buf: data buffer
@@ -748,6 +794,36 @@ unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf,
}

/**
+ * ata_sff_data_xfer_noirq_16bit - Transfer data by PIO
+ * @dev: device to target
+ * @buf: data buffer
+ * @buflen: buffer length
+ * @rw: read/write
+ *
+ * Transfer data from/to the device data register by PIO. Do the
+ * transfer with interrupts disabled using only 16-bit transfers.
+ *
+ * LOCKING:
+ * Inherited from caller.
+ *
+ * RETURNS:
+ * Bytes consumed.
+ */
+unsigned int ata_sff_data_xfer_noirq_16bit(struct ata_device *dev,
+ unsigned char *buf,
+ unsigned int buflen, int rw)
+{
+ unsigned long flags;
+ unsigned int consumed;
+
+ local_irq_save(flags);
+ consumed = ata_sff_data_xfer_16bit(dev, buf, buflen, rw);
+ local_irq_restore(flags);
+
+ return consumed;
+}
+
+/**
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
*
@@ -2827,7 +2903,9 @@ EXPORT_SYMBOL_GPL(ata_sff_tf_load);
EXPORT_SYMBOL_GPL(ata_sff_tf_read);
EXPORT_SYMBOL_GPL(ata_sff_exec_command);
EXPORT_SYMBOL_GPL(ata_sff_data_xfer);
+EXPORT_SYMBOL_GPL(ata_sff_data_xfer_16bit);
EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq);
+EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq_16bit);
EXPORT_SYMBOL_GPL(ata_sff_irq_on);
EXPORT_SYMBOL_GPL(ata_sff_irq_clear);
EXPORT_SYMBOL_GPL(ata_sff_hsm_move);
diff --git a/drivers/ata/pata_at32.c b/drivers/ata/pata_at32.c
index 82fb6e2..aa90b19 100644
--- a/drivers/ata/pata_at32.c
+++ b/drivers/ata/pata_at32.c
@@ -173,6 +173,7 @@ static struct scsi_host_template at32_sht = {
static struct ata_port_operations at32_port_ops = {
.inherits = &ata_sff_port_ops,
.cable_detect = ata_cable_40wire,
+ .sff_data_xfer = ata_sff_data_xfer_16bit,
.set_piomode = pata_at32_set_piomode,
};

diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index cf9e984..a1f09ca 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -336,7 +336,7 @@ static struct ata_port_operations pata_icside_port_ops = {
.inherits = &ata_sff_port_ops,
/* no need to build any PRD tables for DMA */
.qc_prep = ata_noop_qc_prep,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer_noirq_16bit,
.bmdma_setup = pata_icside_bmdma_setup,
.bmdma_start = pata_icside_bmdma_start,
.bmdma_stop = pata_icside_bmdma_stop,
diff --git a/drivers/ata/pata_isapnp.c b/drivers/ata/pata_isapnp.c
index 6a111ba..6f38f76 100644
--- a/drivers/ata/pata_isapnp.c
+++ b/drivers/ata/pata_isapnp.c
@@ -26,6 +26,7 @@ static struct scsi_host_template isapnp_sht = {
static struct ata_port_operations isapnp_port_ops = {
.inherits = &ata_sff_port_ops,
.cable_detect = ata_cable_40wire,
+ .sff_data_xfer = ata_sff_data_xfer_16bit,
};

/**
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index bc037ff..d919934 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -226,12 +226,12 @@ static const struct ata_port_operations legacy_base_port_ops = {

static struct ata_port_operations simple_port_ops = {
.inherits = &legacy_base_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer_noirq_16bit,
};

static struct ata_port_operations legacy_port_ops = {
.inherits = &legacy_base_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer_noirq_16bit,
.set_mode = legacy_set_mode,
};

@@ -286,39 +286,18 @@ static void pdc20230_set_piomode(struct ata_port *ap, struct ata_device *adev)
static unsigned int pdc_data_xfer_vlb(struct ata_device *dev,
unsigned char *buf, unsigned int buflen, int rw)
{
- if (ata_id_has_dword_io(dev->id)) {
- struct ata_port *ap = dev->link->ap;
- int slop = buflen & 3;
- unsigned long flags;
+ unsigned long flags;

- local_irq_save(flags);
+ local_irq_save(flags);

- /* Perform the 32bit I/O synchronization sequence */
- ioread8(ap->ioaddr.nsect_addr);
- ioread8(ap->ioaddr.nsect_addr);
- ioread8(ap->ioaddr.nsect_addr);
+ /* Perform the 32bit I/O synchronization sequence */
+ ioread8(ap->ioaddr.nsect_addr);
+ ioread8(ap->ioaddr.nsect_addr);
+ ioread8(ap->ioaddr.nsect_addr);

- /* Now the data */
- if (rw == READ)
- ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
- else
- iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
-
- if (unlikely(slop)) {
- __le32 pad;
- if (rw == READ) {
- pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
- memcpy(buf + buflen - slop, &pad, slop);
- } else {
- memcpy(&pad, buf + buflen - slop, slop);
- iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
- }
- buflen += 4 - slop;
- }
- local_irq_restore(flags);
- } else
- buflen = ata_sff_data_xfer_noirq(dev, buf, buflen, rw);
+ buflen = ata_sff_data_xfer(dev, buf, buflen, rw);

+ local_irq_restore(flags);
return buflen;
}

@@ -733,33 +712,6 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc)
return ata_sff_qc_issue(qc);
}

-static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf,
- unsigned int buflen, int rw)
-{
- struct ata_port *ap = adev->link->ap;
- int slop = buflen & 3;
-
- if (ata_id_has_dword_io(adev->id)) {
- if (rw == WRITE)
- iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
- else
- ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
-
- if (unlikely(slop)) {
- __le32 pad;
- if (rw == WRITE) {
- memcpy(&pad, buf + buflen - slop, slop);
- iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
- } else {
- pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
- memcpy(buf + buflen - slop, &pad, slop);
- }
- }
- return (buflen + 3) & ~3;
- } else
- return ata_sff_data_xfer(adev, buf, buflen, rw);
-}
-
static int qdi_port(struct platform_device *dev,
struct legacy_probe *lp, struct legacy_data *ld)
{
@@ -773,19 +725,19 @@ static struct ata_port_operations qdi6500_port_ops = {
.inherits = &legacy_base_port_ops,
.set_piomode = qdi6500_set_piomode,
.qc_issue = qdi_qc_issue,
- .sff_data_xfer = vlb32_data_xfer,
+ .sff_data_xfer = ata_sff_data_xfer,
};

static struct ata_port_operations qdi6580_port_ops = {
.inherits = &legacy_base_port_ops,
.set_piomode = qdi6580_set_piomode,
- .sff_data_xfer = vlb32_data_xfer,
+ .sff_data_xfer = ata_sff_data_xfer,
};

static struct ata_port_operations qdi6580dp_port_ops = {
.inherits = &legacy_base_port_ops,
.set_piomode = qdi6580dp_set_piomode,
- .sff_data_xfer = vlb32_data_xfer,
+ .sff_data_xfer = ata_sff_data_xfer,
};

static DEFINE_SPINLOCK(winbond_lock);
@@ -856,7 +808,7 @@ static int winbond_port(struct platform_device *dev,
static struct ata_port_operations winbond_port_ops = {
.inherits = &legacy_base_port_ops,
.set_piomode = winbond_set_piomode,
- .sff_data_xfer = vlb32_data_xfer,
+ .sff_data_xfer = ata_sff_data_xfer,
};

static struct legacy_controller controllers[] = {
diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
index a9e8273..0ab2c8e 100644
--- a/drivers/ata/pata_mpc52xx.c
+++ b/drivers/ata/pata_mpc52xx.c
@@ -265,6 +265,7 @@ static struct ata_port_operations mpc52xx_ata_port_ops = {
.cable_detect = ata_cable_40wire,
.set_piomode = mpc52xx_ata_set_piomode,
.post_internal_cmd = ATA_OP_NULL,
+ .sff_data_xfer = ata_sff_data_xfer_16bit,
};

static int __devinit
diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index 41b4361..8cb4923 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -133,7 +133,7 @@ static struct scsi_host_template pcmcia_sht = {

static struct ata_port_operations pcmcia_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer_noirq_16bit,
.cable_detect = ata_cable_40wire,
.set_mode = pcmcia_set_mode,
};
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index 8f65ad6..3310eb0 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -52,7 +52,7 @@ static struct scsi_host_template pata_platform_sht = {

static struct ata_port_operations pata_platform_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer_noirq_16bit,
.cable_detect = ata_cable_unknown,
.set_mode = pata_platform_set_mode,
.port_start = ATA_OP_NULL,
diff --git a/drivers/ata/pata_qdi.c b/drivers/ata/pata_qdi.c
index 63b7a1c..36cc778 100644
--- a/drivers/ata/pata_qdi.c
+++ b/drivers/ata/pata_qdi.c
@@ -124,35 +124,6 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc)
return ata_sff_qc_issue(qc);
}

-static unsigned int qdi_data_xfer(struct ata_device *dev, unsigned char *buf,
- unsigned int buflen, int rw)
-{
- if (ata_id_has_dword_io(dev->id)) {
- struct ata_port *ap = dev->link->ap;
- int slop = buflen & 3;
-
- if (rw == READ)
- ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
- else
- iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
-
- if (unlikely(slop)) {
- __le32 pad;
- if (rw == READ) {
- pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
- memcpy(buf + buflen - slop, &pad, slop);
- } else {
- memcpy(&pad, buf + buflen - slop, slop);
- iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
- }
- buflen += 4 - slop;
- }
- } else
- buflen = ata_sff_data_xfer(dev, buf, buflen, rw);
-
- return buflen;
-}
-
static struct scsi_host_template qdi_sht = {
ATA_PIO_SHT(DRV_NAME),
};
@@ -160,7 +131,6 @@ static struct scsi_host_template qdi_sht = {
static struct ata_port_operations qdi6500_port_ops = {
.inherits = &ata_sff_port_ops,
.qc_issue = qdi_qc_issue,
- .sff_data_xfer = qdi_data_xfer,
.cable_detect = ata_cable_40wire,
.set_piomode = qdi6500_set_piomode,
};
diff --git a/drivers/ata/pata_winbond.c b/drivers/ata/pata_winbond.c
index a7606b0..56f5f8e 100644
--- a/drivers/ata/pata_winbond.c
+++ b/drivers/ata/pata_winbond.c
@@ -92,42 +92,12 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
}


-static unsigned int winbond_data_xfer(struct ata_device *dev,
- unsigned char *buf, unsigned int buflen, int rw)
-{
- struct ata_port *ap = dev->link->ap;
- int slop = buflen & 3;
-
- if (ata_id_has_dword_io(dev->id)) {
- if (rw == READ)
- ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
- else
- iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
-
- if (unlikely(slop)) {
- __le32 pad;
- if (rw == READ) {
- pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
- memcpy(buf + buflen - slop, &pad, slop);
- } else {
- memcpy(&pad, buf + buflen - slop, slop);
- iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
- }
- buflen += 4 - slop;
- }
- } else
- buflen = ata_sff_data_xfer(dev, buf, buflen, rw);
-
- return buflen;
-}
-
static struct scsi_host_template winbond_sht = {
ATA_PIO_SHT(DRV_NAME),
};

static struct ata_port_operations winbond_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = winbond_data_xfer,
.cable_detect = ata_cable_40wire,
.set_piomode = winbond_set_piomode,
};
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 06b8033..643328a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1481,8 +1481,12 @@ extern void ata_sff_exec_command(struct ata_port *ap,
const struct ata_taskfile *tf);
extern unsigned int ata_sff_data_xfer(struct ata_device *dev,
unsigned char *buf, unsigned int buflen, int rw);
+extern unsigned int ata_sff_data_xfer_16bit(struct ata_device *dev,
+ unsigned char *buf, unsigned int buflen, int rw);
extern unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev,
unsigned char *buf, unsigned int buflen, int rw);
+extern unsigned int ata_sff_data_xfer_noirq_16bit(struct ata_device *dev,
+ unsigned char *buf, unsigned int buflen, int rw);
extern u8 ata_sff_irq_on(struct ata_port *ap);
extern void ata_sff_irq_clear(struct ata_port *ap);
extern int ata_sff_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,

2008-08-06 00:45:27

by Tejun Heo

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Robert Hancock wrote:
> Here's my first cut at it. Compile tested only. This sets most controllers
> to use 32-bit PIO except for those which could potentially be on a real ISA
> or other 16-bit bus. It's a bit non-obvious what to do with some of the
> drivers, so input is welcome.
>
> This implementation doesn't check the ata_id_has_dword_io at all, since it
> would only make a difference on controllers where we don't really want to
> use it anyway.
>
> It seems like regardless of whether we do 32-bit by default or not the 32-bit
> data_xfer function should be added to libata core as we have several drivers
> which duplicate the same code currently..

Great, just some minor nitpicks as I don't have much idea about 16 bit ones.

> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
> + unsigned int buflen, int rw)
> +{
> + struct ata_port *ap = dev->link->ap;
> + void __iomem *data_addr = ap->ioaddr.data_addr;
> + unsigned int words = buflen >> 2;

dwords maybe?

> + unsigned int slop = buflen & 3;
> +
> + /* Transfer multiple of 4 bytes */
> + if (rw == READ)
> + ioread32_rep(data_addr, buf, words);
> + else
> + iowrite32_rep(data_addr, buf, words);
> +
> + /* Transfer trailing 1 byte, if any. */

1byte?

Thanks.

--
tejun

2008-08-06 02:31:19

by Robert Hancock

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Tejun Heo wrote:
> Robert Hancock wrote:
>> Here's my first cut at it. Compile tested only. This sets most controllers
>> to use 32-bit PIO except for those which could potentially be on a real ISA
>> or other 16-bit bus. It's a bit non-obvious what to do with some of the
>> drivers, so input is welcome.
>>
>> This implementation doesn't check the ata_id_has_dword_io at all, since it
>> would only make a difference on controllers where we don't really want to
>> use it anyway.
>>
>> It seems like regardless of whether we do 32-bit by default or not the 32-bit
>> data_xfer function should be added to libata core as we have several drivers
>> which duplicate the same code currently..
>
> Great, just some minor nitpicks as I don't have much idea about 16 bit ones.
>
>> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
>> + unsigned int buflen, int rw)
>> +{
>> + struct ata_port *ap = dev->link->ap;
>> + void __iomem *data_addr = ap->ioaddr.data_addr;
>> + unsigned int words = buflen >> 2;
>
> dwords maybe?
>
>> + unsigned int slop = buflen & 3;
>> +
>> + /* Transfer multiple of 4 bytes */
>> + if (rw == READ)
>> + ioread32_rep(data_addr, buf, words);
>> + else
>> + iowrite32_rep(data_addr, buf, words);
>> +
>> + /* Transfer trailing 1 byte, if any. */
>
> 1byte?

Yeah, those are both leftovers from the 16-bit code. I'll fix them up in
the next version, if the approach looks good..

>
> Thanks.
>

2008-08-06 09:09:56

by Alan

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

> Here's my first cut at it. Compile tested only. This sets most controllers
> to use 32-bit PIO except for those which could potentially be on a real ISA
> or other 16-bit bus. It's a bit non-obvious what to do with some of the
> drivers, so input is welcome.

Only thing I would do differently and definitely want to do differently
is the assumption that we just turn on 32bit and see what occurs. We need
to read/test/change controller by controller over to 32bit not just hope.

Alan

2008-08-06 11:18:19

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Hello, I wrote:

>>>>>> supported. I couldn't track down where that bit was actually
>>>>>> defined in the first place, all the way back to ATA-1 it seems to
>>>>>> be indicated as reserved. Actually, I'm not sure why the drive
>>>>>> cares in the first place, it would seem like a pure host
>>>>>> controller issue..
>>>>>>
>>>>> It goes back before IDE into the depths of the original compaq
>>>>> spec. When
>>>>> you have a device wired basically directly to the ISA bus
>>>>> (original IDE)
>>>>>
>>>> ISA has only 8/16-bit data bus, so it could not have mattered
>>>> there...
>>>
>>> Depends what a 32bit I/O looks like on the 16bit bus - timing wise.
>>>
>>
>> Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed
>> recovery time, IIRC... It's just occured to me that in case of the
>> 16-bit bus it should be how the drive treated the accesses at address
>> 0x1x2 with IOCS16 asserted that could have mattered. If it honored
>> them, 32-bit I/O could have worked even on a dumb ISA "controller",
>> if not -- no way (unless you really had *something* between the ISA
>> and the IDE cable).
>
> Oh, -IOCS16 is driven by device, not host. I give up then. :-)
>

OTOH, it definitely could work if the drive asserted it for the I/O
port 0x1x2 at least for the data transfer phase (and probably even if it
always asserted -IOCS16 for this address).
That pre-historic word indeed could have made sense then...

MBR, Sergei

2008-08-06 11:27:30

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE

Hello.

Tejun Heo wrote:
>> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
>> + unsigned int buflen, int rw)
>> +{
>> + struct ata_port *ap = dev->link->ap;
>> + void __iomem *data_addr = ap->ioaddr.data_addr;
>> + unsigned int words = buflen >> 2;
>>
>
> dwords maybe?
>

Isn't word 32-bit in the 32-bit systems? We're not on 80[12]86
(despite Intel/MS preference of calling 32-bit memory cells DWORDS)...

MBR, Sergei

2008-08-06 13:05:20

by Tejun Heo

[permalink] [raw]
Subject: Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE

Sergei Shtylyov wrote:
> Hello.
>
> Tejun Heo wrote:
>>> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
>>> + unsigned int buflen, int rw)
>>> +{
>>> + struct ata_port *ap = dev->link->ap;
>>> + void __iomem *data_addr = ap->ioaddr.data_addr;
>>> + unsigned int words = buflen >> 2;
>>>
>> dwords maybe?
>>
>
> Isn't word 32-bit in the 32-bit systems? We're not on 80[12]86
> (despite Intel/MS preference of calling 32-bit memory cells DWORDS)...

Well, as most of machines are 64-bit these days, discussing whether a
word means 16 or 32 bits is kind of moot. I usually just think byte,
word, dword, qword and and libata and pci assume that too -
ata_id_has_dword_io() and PCI configuration accessors. It's ultimately
a peripheral issue either way tho.

Thanks.

--
tejun