Re: [PATCH v2 2/4] gpio: add viperboard gpio driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]

On Thu, Oct 25, 2012 at 12:02:36PM +0200, Lars Poeschel wrote:

> You can reread our conversation before Linus Walleij brought you in starting 
> with my first patch submission here:

> https://lkml.org/lkml/2012/10/12/218

This has all the problems of this and your previous mail - here we've
got about 40 lines of unbroken text (with very wide lines too, notice
how things start to wrap when I quote them) none of which is easy to
read.

> As there is no general regmap_bus for reading/writing registers over USB (and 
> probably will never be). I had to implement it in my driver for the special 
> case of this device. I also have a regmap_config, that provides a volatile_reg 
> function. I need this to decide if the pin the value is queried is an output 
> or an input. In the latter case I obviously can't use the cache, in the former 
Why would you want to implement this as a bus?  If you're not actually
rendering things down into a register and value on the bus then you
should be hooking in at the level before we do the marshalling since
that's totally irrelevant.  This should be done by making the
marashalling pluggable.

> case I want to use it. Here the first problem arises. In the volatile_reg 
> function I can not do a read of a regmap register since regmap internally uses 
> a mutex lock. Thus I have my own cache in a device private variable. This is a 
> bit strange. To use the cache to have to implement my own cache ;-)

What do you actually need to read back here?

> The other problem is, that the volatile_reg function is called with struct 
> device and I can not container_of up to my device vprbrd_gpio struct, since 

Why on earth can't you do that?  That sounds like something that should
be fixed for whatever bus you're using if it's an issue but normally
this is simply a case of setting and reading some driver data.

> The last problem is that I have 16 registers for setting/reading the first 16 
> pins value. And there is another 16 registers for setting the pins direction 
> (input/output). Setting the pin to output is an atomic operation of setting 
> the pins direction AND setting it's value. So if there is a set pin direction 
> to output operation in my driver I want to update the value of the 
> corresponding value register, since regmap does not know of this change. But 
> the regcache_write I would use for this seems also not intended for use by 
> drivers. It is not exported.

So just invalidate the cache and it'll get restored next time we look at
the register.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]