Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver

From: Eric Biederman
Date: Fri May 20 2011 - 18:40:22 EST


On Fri, May 20, 2011 at 11:46 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Friday 20 May 2011 20:05:05 Chris Metcalf wrote:
>> This works well, except for the fact that we take advantage of the fact
>> that the hypervisor driver internally buffers up writes to the current
>> EEPROM sector, and flushes it to hardware only when explicitly told to do
>> so, or when we start writing to another sector.  This avoids excessive wear
>> on the EEPROM and also handles detection of whether we need to do a sector
>> erase before the re-write.  However, it also means that we need to be able
>> to issue the final explicit flush, or else the last write just sits in the
>> hypervisor buffer indefinitely.  To make that happen I think I need to
>> extend the bin_attribute structure, and the bin.c release() function, slightly:
>
> Yes, that would work. Another option would be to add a flush() callback
> to the bin_attribute. Since filp_close calls both fops->flush and
> fops->release, that would also make it possible to flush the buffer
> without closing it, if necessary.

Ouch!

Please do not make sysfs the direct access method to your device.

I may be wrong but I don't think any other driver relies exclusively on sysfs.
Certainly with the introduction of a flush you are introducing an completely
different usage paradigm from current users and will need an entirely different
set of tools.

You are vastly exceeding the one value per file rule of sysfs.

Please look at the i2c core and if at all possible build a proper interface to
your eeprom, that existing programs have a chance of utilizing rather than
extending sysfs in ways that means cli tools can not work with it.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/