Re: [PATCH 2/3] ide: add at91_ide driver

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

On Friday 06 February 2009, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> >>> +void at91_ide_tf_load(ide_drive_t *drive, ide_task_t *task)
> >>> +{
> >>> +	ide_hwif_t *hwif = drive->hwif;
> >>> +	struct ide_io_ports *io_ports = &hwif->io_ports;
> >>> +	struct ide_taskfile *tf = &task->tf;
> >>> +	u8 HIHI = (task->tf_flags & IDE_TFLAG_LBA48) ? 0xE0 : 0xEF;
> >>> +
> >>> +	if (task->tf_flags & IDE_TFLAG_FLAGGED)
> >>> +		HIHI = 0xFF;
> >>> +
> >>> +	if (task->tf_flags & IDE_TFLAG_OUT_DATA) {
> >>>   
> >>>       
> >>    Sigh. Bart, couldn't we drop that stupid flag? I bet nobody ever used it.
> >>     
> >
> > It is there for HDIO_DRIVE_TASKFILE ioctl and I prefer not to break it.
> >
> > Just add ->{read,write}_data methods for IDE_TFLAG_{IN,OUT}_DATA to struct
> > ide_tp_ops -- it should also help some other host drivers like tx493*.
> >   
> 
>   That would be extremely senseless activity sicne I believe this flag 
> is totally useless. I have better thing to do. :-)

I would love to remove this flag but it is used by user-space exposed
interface (which was used by few drive vendors for their diagnostics
tools -- doesn't matter whether internal or external) so you should put
some technical arguments behind its removal (you know many of low-level
technical details better than me so I may be missing something which is
obvious to you).

OTOH while ->{read,write}_data approach would result in something like
~50 extra LOC (or even less with be_tp_ops) compared to removal it is
completely safe and we don't need to spend a single second wondering
about potential breakage (one day this ioctl will go away together with
the flag and issue will solve itself).

Thanks,
Bart