From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: linux-ide@vger.kernel.org
Subject: Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
Date: Thu, 28 Jun 2007 00:10:34 +0200 [thread overview]
Message-ID: <200706280010.34122.bzolnier@gmail.com> (raw)
In-Reply-To: <4682C87A.2040709@ru.mvista.com>
On Wednesday 27 June 2007, Sergei Shtylyov wrote:
> >>>>Set Transfer Mode subcode of the Set Features command, you're always selecting
> >>>>the flow control mode, i.e. using IORDY. So, the last condition in this if
>
> >> So, what actually would need fixing in *all* the drivers if one was aiming
> >>at ATA-1 compatibility is *not* issuing that subcommand to such drives...
>
> > I was actually thinking about a different way of fixing this:
>
> > - remove 0x08 bit from XFER_PIO_[0,6] defines and add new XFER_PIO_IORDY
> > define (<linux/ata.h>)
>
> Nah, that wouldn't match to the ATA definition of these values.
>
> > - check for speed == XFER_PIO_[0,6] in ide-lib.c::ide_config_drive_speed()
>
> It's in ide-iops.c. ;-)
>
> > and pmac.c::pmac_ide_do_setfeature(), add XFER_PIO_IORDY if needed
>
> And what, just pass the mode thru to the Set Features if there's no IORDY
> support? That would be bogus since there are just *no* subcodes to set the
> specific mode below 0x08 -- the only defined subcodes are 0x00 (set default
> mode), and 0x01 (set default mode w/o IORDY).
Thinko on my side.
Damn, I should have re-check ATA specs before writing this. :)
> I was thinking of checking if the drive really supports IORDY before
> issuing a command to set PIO mode (and just skipping the command if there's no
> IORDY -- well, maybe adding an extra check that the passed mode is acceptable
> to the drive, i.e. <= its default one). Should be quite simple to do.
Sounds fine.
> > This should be done together with fixing these host drivers that don't
> > handle IORDY properly.
>
> Erm, not necessarily...
Hmm, yes.
I'll just count on you with fixing all this IORDY stuff (as you have
much more expertise in this field) and concentrate on other things.
> >>>Oh yes, I keep forgetting about it - some nice FIXME comment
> >>>in <linux/ata.h> would be of a great help. :-)
>
> >> Well, some drivers (like pdc202xx_*) don't do the IORDY thing right for
> >>PIO modes < 3 as well...
>
> > Added to the existing IDE TODO at
>
> > http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO
>
> > Patches adding/removing items are welcomed.
>
> > Patches fixing actual issues are welcomed even more.
>
> Sigh, I'm trying to get some time (more like time slices :-) off to deal
> with my own issues...
Which reminds me about some HPT IDE patches... 8)
Bart
next prev parent reply other threads:[~2007-06-27 21:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-23 18:05 [PATCH 5/6] ide: add ata_dev_has_iordy() helper Bartlomiej Zolnierkiewicz
2007-06-23 19:36 ` Sergei Shtylyov
2007-06-23 22:09 ` Bartlomiej Zolnierkiewicz
2007-06-24 16:54 ` Sergei Shtylyov
2007-06-27 19:02 ` Bartlomiej Zolnierkiewicz
2007-06-27 20:28 ` Sergei Shtylyov
2007-06-27 22:10 ` Bartlomiej Zolnierkiewicz [this message]
2007-06-29 20:24 ` Sergei Shtylyov
2007-06-29 22:16 ` Bartlomiej Zolnierkiewicz
2007-07-04 16:33 ` Sergei Shtylyov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200706280010.34122.bzolnier@gmail.com \
--to=bzolnier@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=sshtylyov@ru.mvista.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.