Re: [PATCH] Add hook for custom xfer function in PATA Platform driver

[Graeme Russ <graeme.russ@gmail.com>]

Sergei Shtylyov wrote:
> Hello.
> 
> Graeme Russ wrote:
> 
>> [Added linux-ide back onto distribution list]
>>   
> 
>   Right, I didn't intend to exclude it -- probably didn't press all the
> keys at once for the reply-to-all function.
> 
> [...]
>>
>>>   You should have also taught the symmetric ide-platfrom driver the same
>>> trick. However, IDE core's data transfer methods has a different
>>> prototype.
>>>     
>>
>> I did think about the other drivers (OF Platform etc) but I have no
>> way of
>> testing them.
>>   
> 
>   pata_of_platform is not easily extensible in this way. It gets all the
> information about the device from the device tree -- and you can't
> encode all your complications there, unless you invent a new OF device.
> 
>>> I suggest to rather add a new flag, indicating 8-bit data I/O, and have
>>> the alternate sff_data_xfer() method defined inside the driver.

The vast majority of implementations are 16-bit (no one has complained
about the lack of 8-bit support to date). I don't think the majority of
users should be carrying around the extra code for a tiny minority. Yes, it
could be wrapped around an #ifdef but then things start to get ugly (why
not just ditch the flag and #ifdef the 8-bit transfers entirely, hack
Kconfig etc) eeewwwww....

>> other devices on the bus). By overriding the data transfer function I can
>> arbitrate access to the bus and switch the bus timings based on the
>> peripheral being accessed. This cannot be done be a generic data transfer
>> function.
>>   
> 
>   I disagree with your approach of overriding the libata methods at the
> board level, so I suggest to write a new driver. This is too complicated
> stuff for poor old pata_platform. :-)

My custom function to date looks like:

unsigned int ata_enet_data_xfer(struct ata_device *dev, unsigned char *buf,
			       unsigned int buflen, int rw)
{
	struct ata_port *ap = dev->link->ap;
	void __iomem *data_addr = ap->ioaddr.data_addr;

	set_gp_bus_slow();
	/* Transfer bytes */
	if (rw == READ)
		ioread8_rep(data_addr, buf, buflen);
	else
		iowrite8_rep(data_addr, buf, buflen);

	set_gp_bus_fast();
	return buflen;
}

set_gp_bus_slow() and set_gp_bus_fast() (at the moment) simply set a few
config registers to set the GP bus timing (no arbitration yet, but these
functions will also handle that using a mutex). I don't see the point in
re-writing the entire PATA Platform driver when the existing driver appears
to be perfectly capable with a very minor extension hook.

Regards,

Graeme