Re: [PATCH] fix pata-rb532-cf

[Phil Sutter <n0-1@freewrt.org>]

Hi,

On Fri, Oct 31, 2008 at 02:20:54AM +0300, Sergei Shtylyov wrote:
>   Some of the changes look unneeded and some just strange...

you're completely right, my apologies for that.

> >-#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?

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

Greetings, Phil