Hello, I wrote:

quoted text
>>> Bartlomiej Zolnierkiewicz wrote:

quoted text
>>>> [PATCH] sl82c105: add ->speedproc support

quoted text
>>>> * add sl82c105_tunepio() wrapper for sl82c105_tune_drive()
>>>>  (just to get the error value)

quoted text
>>>> * add sl82c105_tune_chipset() (->speedproc method) for setting
>>>>  transfer mode

quoted text
>>>    Thanks for the patch!

quoted text
>>>  > Index: b/drivers/ide/pci/sl82c105.c

quoted text
>>>> ===================================================================
>>>> --- a/drivers/ide/pci/sl82c105.c
>>>> +++ b/drivers/ide/pci/sl82c105.c

quoted text
> [...]

quoted text
>>>> @@ -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)
>>>> +     */

quoted text
>>>   I thought we were doing it by calling ide_get_best_pio_mode() above...

quoted text
>> We are also using ide_get_best_pio_mode() to get PIO cycle time
>> so we can't move it here ATM.

quoted text
>   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...

quoted text
>>>> +    (void)sl82c105_tunepio(drive, pio);

quoted text
>>>    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. :-)

quoted text
>> Moving them to ->init_hwif where they belong would be even better... ;-)

quoted text
>    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...

quoted text
>>>> +static int sl82c105_tune_chipset(ide_drive_t *drive, u8 mode)
>>>> +{
>>>> +    mode = ide_rate_filter(drive, mode);
>>>> +
>>>> +    if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
>>>> +        return sl82c105_tunepio(drive, mode - XFER_PIO_0);
>>>> +
>>>> +    /*
>>>> +     * TODO: add MWDMA0/1 support
>>>> +     */
>>>> +    BUG_ON(mode != XFER_MW_DMA_2);

   I wonder is there are some W83C554 users anywhere -- that chipset also 
supports UltraDMA...

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