[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200703152251.18615.bzolnier@gmail.com>
Date: Thu, 15 Mar 2007 22:51:18 +0100
From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To: Sergei Shtylyov <sshtylyov@...mvista.com>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/13] sl82c105: add ->speedproc support
On Wednesday 14 March 2007, Sergei Shtylyov wrote:
> Hello, I wrote:
>
> >>> Bartlomiej Zolnierkiewicz wrote:
>
> >>>> [PATCH] sl82c105: add ->speedproc support
>
> >>>> * add sl82c105_tunepio() wrapper for sl82c105_tune_drive()
> >>>> (just to get the error value)
>
> >>>> * add sl82c105_tune_chipset() (->speedproc method) for setting
> >>>> transfer mode
>
> >>> Thanks for the patch!
>
> >>> > Index: b/drivers/ide/pci/sl82c105.c
>
> >>>> ===================================================================
> >>>> --- a/drivers/ide/pci/sl82c105.c
> >>>> +++ b/drivers/ide/pci/sl82c105.c
>
> > [...]
>
> >>>> @@ -114,17 +114,45 @@ static void sl82c105_tune_drive(ide_driv
> >>>> */
> >>>> drive->io_32bit = 1;
> >>>> drive->unmask = 1;
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> +static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
> >>>> +{
> >>>> + /*
> >>>> + * TODO: find best PIO mode and set device speed here
> >>>> + * (requires adding helper function for getting PIO cycle
> >>>> time)
> >>>> + */
>
> >>> I thought we were doing it by calling ide_get_best_pio_mode() above...
>
> >> We are also using ide_get_best_pio_mode() to get PIO cycle time
> >> so we can't move it here ATM.
>
> > I've found/used quite convenient workaround for that -- return PIO
> > mode actually selected from xxx_tune_pio(), then call
> > ide_config_drive_speed() from the real tuneproc() method.
>
> Works like charm with ignoring the result, since ide_get_best_pio_mode()
> returns the same explicit mode as was passed to it -- so is good for the
> speedproc() methods also...
>
> >>>> + (void)sl82c105_tunepio(drive, pio);
>
> >>> Erm, I thought afterwards that I vainly folded one into another. I
> >>> think it's worth moving those io_32bit and unmask flag assignments
> >>> above back there... May also recast my patch. :-)
>
> >> Moving them to ->init_hwif where they belong would be even better... ;-)
>
> > Well, I wasn't sure where they belong... :-)
> > So, OK to recast that patch?
>
> Recasted it and reworked your patch atop of it (adding MWDMA 0/1 support as
> a bonus! :-) -- now need to conduct some testing on a remote target...
Great. :)
Bart
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists