Re: [PATCH] fix pata-rb532-cf

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

Hello.

Phil Sutter wrote:

>>> -#define RB500_CF_REG_CMD	0x0800
>>> +#define RB500_CF_REG_BASE	0x0800
>>> #define RB500_CF_REG_CTRL	0x080E
>>> #define RB500_CF_REG_DATA	0x0C00
>>> +#define RB500_CF_REG_ERR	0x080D
>>>  
>>>       
>>   Hm...
>>     
>
> The original driver (see my patch) accesses 0x80D to read error codes,
> so I assumed this is correct. Indeed, the driver works also with the
> standard setting. Is there a way I can clearly verify which is the right
> offset?
>   

   I assume you don't have the documentation? I wodner why they needed 
to duplicate (or remap) this register...

>>> -static void rb532_pata_data_xfer(struct ata_device *adev, unsigned char 
>>> *buf,
>>> +static unsigned int rb532_pata_data_xfer(struct ata_device *adev, 
>>> unsigned char *buf,
>>> 				unsigned int buflen, int write_data)
>>> {
>>> 	struct ata_port *ap = adev->link->ap;
>>> 	void __iomem *ioaddr = ap->ioaddr.data_addr;
>>> +	unsigned int ret = buflen;
>>>
>>> 	if (write_data) {
>>> 		for (; buflen > 0; buflen--, buf++)
>>> @@ -87,6 +94,8 @@ static void rb532_pata_data_xfer(struct ata_device 
>>> *adev, unsigned char *buf,
>>> 	}
>>>
>>> 	rb532_pata_finish_io(adev->link->ap);
>>> +
>>> +	return ret;
>>>  
>>>       
>>   Totally unneeded variable.
>>     
>
> I wanted to preserve the behaviour of ata_sff_data_xfer(), i.e.
> returning the number of bytes written. The variable exists, because
> buflen is being decremented in the loop and therefore zero afterwards.
>   

   Oops, sorry -- I've managed to miss that decrement... :-<

> Returning a static value also should be no alternative, as it could lead
> to unexpected behaviour on the caller side. So, do I miss something?
>   

   No. The alternatives would be to use local variable as a loop counter 
or, better yet, using readsb()/writesb() instead of the loops...
Wait! The original driver used 32-bit I/O to this register, not 8-bit -- 
so it looks like you have artificially slowed it down... :-/

MBR, Sergei