Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE

[Bartlomiej Zolnierkiewicz]

Hi,

On Monday 28 February 2005 16:24, Tejun Heo wrote:
>   Hi,
> 
> Bartlomiej Zolnierkiewicz wrote:
> > On Sunday 27 February 2005 08:36, Tejun Heo wrote:
> > 
> >> Hello, Bartlomiej.
> >>
> >> This patch should be modified to use flagged taskfile if the
> >>task_end_request_fix patch isn't applied.  As non-flagged taskfile
> >>won't return valid result registers, TASK ioctl users won't get the
> >>correct register output.
> > 
> > 
> > Nope, it works just fine because REQ_DRIVE_TASK used only
> > no-data protocol, please check task_no_data_intr().
> > 
> 
>   Sorry, I missed that.  IDE really has a lot of ways to finish a 
> command, doesn't it?  hdio.txt is gonna look ugly. :-)

Yep but it was a lot more "fun" when there were three PIO codepaths. ;-)

hdio.txt doesn't need to know about driver internals so no problem here.

> > 
> >> IMHO, this flag-to-get-result-registers thing is way too subtle.  How
> >>about keeping old behavior by just not copying out register outputs in
> >>ide_taskfile_ioctl() in applicable cases instead of not reading
> >>registers when ending commands?  That is, unless there's some
> >>noticeable performance impacts I'm not aware of.
> > 
> > 
> > This would miss whole point of not _reading_ these registers (IO is slow).
> > IMHO new flags denoting {in,out} registers should be added (to <linux/ata.h>
> > to share them with libata) so new code can be sane and old flags would map
> > on new flags when needed.
> 
>   Please do it.
> 
>   Or, let me know what you have in mind (added fields, flag names, 
> etc...); then, I'll do it.  I think we also need to hear Jeff's opinion 
> as things need to be added to ata.h.

I was thinking about:

* adding ATA_TFLAG_{IN,OUT}_xxx flags (there is enough free
  place for all flags in ->flags field of struct ata_taskfile)
* teaching flagged_taskfile() about these flags and mapping ->tf_out_flags
  onto ATA_TFLAG_OUT_xxx (simple mapping no need to move ->tf_out_flags
  to ide_taskfile_ioctl() etc. - no risk of breaking something)
* moving flagged taskfile writing to ide_tf_load_discrete() helper
* adding ide_tf_read_discrete() helper for use by ide_end_drive_cmd()
  and mapping ->tf_in_flags onto ATA_TFLAG_IN_xxx (ditto)

If you like this plan feel free to implement it.
I'm also open for better ideas, comments etc.

Bartlomiej
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/