[PATCH 10/13] sl82c105: add ->speedproc support

[PATCH 10/13] sl82c105: add ->speedproc support

[Bartlomiej Zolnierkiewicz]

[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

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---

 drivers/ide/pci/sl82c105.c |   44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

Index: b/drivers/ide/pci/sl82c105.c
===================================================================
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
@@ -77,7 +77,7 @@ static unsigned int get_timing_sl82c105(
 /*
  * Configure the drive and chipset for PIO
  */
-static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
+static int sl82c105_tunepio(ide_drive_t *drive, u8 pio)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
@@ -91,7 +91,7 @@ static void sl82c105_tune_drive(ide_driv
 	xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
 
 	if (ide_config_drive_speed(drive, xfer_mode))
-		return;
+		return 1;
 
 	drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
 
@@ -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)
+	 */
+	(void)sl82c105_tunepio(drive, pio);
+}
+
+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);
+
+	/*
+	 * Configure the drive for DMA.
+	 * We'll program the chipset only when DMA is actually turned on.
+	 */
+	return ide_config_drive_speed(drive, mode);
 }
 
-/*
- * Configure the drive for DMA.
- * We'll program the chipset only when DMA is actually turned on.
- */
 static int config_for_dma (ide_drive_t *drive)
 {
+	u8 mode = ide_max_dma_mode(drive);
+
 	DBG(("config_for_dma(drive:%s)\n", drive->name));
 
-	if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0)
+	if (mode == 0 || sl82c105_tune_chipset(drive, mode))
 		return 0;
 
 	return ide_dma_enable(drive);
@@ -387,6 +415,8 @@ static void __devinit init_hwif_sl82c105
 		return;
 	}
 
+	hwif->speedproc			= &sl82c105_tune_chipset;
+
 	hwif->atapi_dma  = 1;
 	hwif->mwdma_mask = 0x04;
 

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Sergei Shtylyov]

Hello.

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!

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---

  > Index: b/drivers/ide/pci/sl82c105.c
> ===================================================================
> --- a/drivers/ide/pci/sl82c105.c
> +++ b/drivers/ide/pci/sl82c105.c
> @@ -77,7 +77,7 @@ static unsigned int get_timing_sl82c105(
>  /*
>   * Configure the drive and chipset for PIO
>   */
> -static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
> +static int sl82c105_tunepio(ide_drive_t *drive, u8 pio)

    The name sl82c105_tune_pio() would have been more in line with the other 
drivers but well, that's your patch (and the function behaves somewhat 
differently anyway :-)

>  {
>  	ide_hwif_t *hwif	= HWIF(drive);
>  	struct pci_dev *dev	= hwif->pci_dev;
> @@ -91,7 +91,7 @@ static void sl82c105_tune_drive(ide_driv
>  	xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
>  
>  	if (ide_config_drive_speed(drive, xfer_mode))
> -		return;
> +		return 1;
>  
>  	drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
>  
> @@ -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...

> +	(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. :-)

> +}
> +
> +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);

    Well, the other drivers just return non-zero in this case...

> +	/*
> +	 * Configure the drive for DMA.
> +	 * We'll program the chipset only when DMA is actually turned on.
> +	 */
> +	return ide_config_drive_speed(drive, mode);
>  }
>  
> -/*
> - * Configure the drive for DMA.
> - * We'll program the chipset only when DMA is actually turned on.
> - */
>  static int config_for_dma (ide_drive_t *drive)
>  {
> +	u8 mode = ide_max_dma_mode(drive);
> +
>  	DBG(("config_for_dma(drive:%s)\n", drive->name));
>  
> -	if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0)
> +	if (mode == 0 || sl82c105_tune_chipset(drive, mode))
>  		return 0;
>  
>  	return ide_dma_enable(drive);
> @@ -387,6 +415,8 @@ static void __devinit init_hwif_sl82c105
>  		return;
>  	}
>  
> +	hwif->speedproc			= &sl82c105_tune_chipset;
> +
>  	hwif->atapi_dma  = 1;
>  	hwif->mwdma_mask = 0x04;

MBR, Sergei

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Bartlomiej Zolnierkiewicz]

Hi,

On Saturday 10 March 2007, Sergei Shtylyov wrote:
> Hello.
> 
> 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!
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> 
>   > Index: b/drivers/ide/pci/sl82c105.c
> > ===================================================================
> > --- a/drivers/ide/pci/sl82c105.c
> > +++ b/drivers/ide/pci/sl82c105.c
> > @@ -77,7 +77,7 @@ static unsigned int get_timing_sl82c105(
> >  /*
> >   * Configure the drive and chipset for PIO
> >   */
> > -static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
> > +static int sl82c105_tunepio(ide_drive_t *drive, u8 pio)
> 
>     The name sl82c105_tune_pio() would have been more in line with the other 
> drivers but well, that's your patch (and the function behaves somewhat 
> differently anyway :-)

I will change it.

> >  {
> >  	ide_hwif_t *hwif	= HWIF(drive);
> >  	struct pci_dev *dev	= hwif->pci_dev;
> > @@ -91,7 +91,7 @@ static void sl82c105_tune_drive(ide_driv
> >  	xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
> >  
> >  	if (ide_config_drive_speed(drive, xfer_mode))
> > -		return;
> > +		return 1;
> >  
> >  	drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
> >  
> > @@ -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.

> > +	(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... ;-)

> > +}
> > +
> > +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);
> 
>     Well, the other drivers just return non-zero in this case...

They are are buggy and need fixing.

IDE driver itself should never ask for an unsupported mode and if user passes
such through ->speedproc interface OOPSing is IMO better than possible data
corruption.  On the second thought ide_rate_filter() takes care of filtering
UDMA modes, so it may as well take care of filtering SWDMA/MWDMA/PIO and the
problem will be solved in more gracefull way... but more work is needed...

Thanks,
Bart

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

>>Hello.

>>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
[...]
>>>@@ -91,7 +91,7 @@ static void sl82c105_tune_drive(ide_driv
>>> 	xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
>>> 
>>> 	if (ide_config_drive_speed(drive, xfer_mode))
>>>-		return;
>>>+		return 1;
>>> 
>>> 	drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
>>> 
>>>@@ -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.

>>>+	(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?

>>>+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);

>>    Well, the other drivers just return non-zero in this case...

> They are are buggy and need fixing.

    Ugh, that'll be a lot of fixing... :-)
    Why BUG on being passed unsupported mode from userspace though (when 
there's no filtering involved before that)?

> IDE driver itself should never ask for an unsupported mode and if user passes
> such through ->speedproc interface OOPSing is IMO better than possible data
> corruption.  On the second thought ide_rate_filter() takes care of filtering
> UDMA modes, so it may as well take care of filtering SWDMA/MWDMA/PIO and the
> problem will be solved in more gracefull way... but more work is needed...

    Yeah. :-)

> Thanks,
> Bart

MBR, Sergei

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Sergei Shtylyov]

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

>>>> +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

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Woody Suwalski]

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...
>
>>>>> +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
Sergei,

ARM Netwinder machines are running hard disk IDE on SL82c105.
Could you send me the actual source file to try (or a patch)?

Thanks, Woody

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Russell King]

On Thu, Mar 15, 2007 at 10:22:44AM -0400, Woody Suwalski wrote:
> Sergei Shtylyov wrote:
> >  I wonder is there are some W83C554 users anywhere -- that chipset 
> >also supports UltraDMA...
> >
> >MBR, Sergei
> Sergei,
> 
> ARM Netwinder machines are running hard disk IDE on SL82c105.
> Could you send me the actual source file to try (or a patch)?

That's actually a W83C553 which does not support UDMA in any shape or
form according to the documents.

I guess it's an enhancement found on W83C554, and in that case any
attempt to add UDMA support to the SL82C105 driver should be done
such that these older chipsets continue to work.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Sergei Shtylyov]

Hello.

Russell King wrote:

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

>>Sergei,

>>ARM Netwinder machines are running hard disk IDE on SL82c105.
>>Could you send me the actual source file to try (or a patch)?

> That's actually a W83C553 which does not support UDMA in any shape or
> form according to the documents.

> I guess it's an enhancement found on W83C554, and in that case any
> attempt to add UDMA support to the SL82C105 driver should be done
> such that these older chipsets continue to work.

    I had no plans of adding it so far since there seemed to be no designs 
using that chip but decided to ask if somebody is interested just in case...

MBR, Sergei

Re: [PATCH 10/13] sl82c105: add ->speedproc support

[Bartlomiej Zolnierkiewicz]

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