Re: [PATCH 1/2] ide: Add tx4939ide driver (v2)

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello, I  wrote:

>>>> +static void tx4939ide_tf_load(ide_drive_t *drive, ide_task_t *task)
>>>> +{
>>>> +    mm_tf_load(drive, task);
>>>> +    if (task->tf_flags & IDE_TFLAG_OUT_DEVICE) {
>>>> +        ide_hwif_t *hwif = drive->hwif;
>>>> +        void __iomem *base = TX4939IDE_BASE(hwif);
>>>> +        /* Fix ATA100 CORE System Control Register */
>>>> +        tx4939ide_writew(tx4939ide_readw(base, TX4939IDE_Sys_Ctl) &
>>>> +                 0x07f0,
>>>> +                 base, TX4939IDE_Sys_Ctl);

>>>    Why? Doesn't page 17-4 of the datasheet say that these bits get 
>>> auto-cleared ona  write to the device/head register? Or is this to 
>>> address <CAUSION> on page 17-9?

>> Yes, that "CAUSION".  I will put it in the comment.

>   Frankly speaking, I couldn't make out much of tht passage:

> <CAUSION>
> The write to the register by the Device/Head register may cause an 
> unexpected function by write wrong
> data to the register. So please rewrite to the System Control register 
> after write to the Device/Head
> register to secure write to System Control register in ATA100 Core.

    I thought that this was related to loading the correct transfer mode for 
the selected drive. But if it's not only that, it would be quite pointless to 
also implement selectproc() method if you have to hook the tf_load() method...
    Frankly speaking, I don't understand why they didn't implement 2 timing 
registers like on TC86C001 while still implementing 2 transfer counter 
registers...

MBR, Sergei