Message-Id: <201210251202.36708.poeschel@lemonage.de>
Date:	Thu, 25 Oct 2012 12:02:36 +0200
From:	Lars Poeschel <poeschel@...onage.de>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Lars Poeschel <larsi@....tu-dresden.de>, sameo@...ux.intel.com,
	linux-kernel@...r.kernel.org, jic23@....ac.uk, khali@...ux-fr.org,
	ben-linux@...ff.org, w.sang@...gutronix.de,
	grant.likely@...retlab.ca
Subject: Re: [PATCH v2 2/4] gpio: add viperboard gpio driver

On Wednesday 24 October 2012 at 18:31:55, Mark Brown wrote:
> On Wed, Oct 24, 2012 at 09:53:06AM +0200, Linus Walleij wrote:
> > On Tue, Oct 23, 2012 at 5:24 PM, Lars Poeschel <poeschel@...onage.de> 
wrote:
> > > Ok, I tried to implement it (at least for the gpioa part of the
> > > driver).
> > 
> > > It does not work. I got stuck. There are three problems I could not 
solve:
> > So I'm looping in Mark Brown to get some hints whether the following
> > can be resolved easily or needs to be given up on...
> 
> Some context would've been enormously useful for working out what the
> discussion is all about...  this is *very* verbose and not at all clear.

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

I'll try to sum up, what happend so far:
I'd submit a driver some strange device (called "viperboard") that besides 
some other functionality is a USB interface that has 2x16 GPIO at the other 
side. Some USB to GPIO converter.
In my driver I had two 16 bit variables that cache the values of the GPIO and 
two 16 bit variables holding the direction of the GPIO pin (input or output). 
The idea is that in case a pin is an output and there is a read on that pin, 
that I wanted to save the USB ping pong for reading the value of the pin.
Linus Walleij pointed out that there is already a framework for marshalling 
and caching register read/writes called regmap.
I gave it a try and used it for my driver but I had problems and this is where 
you came in.
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 
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 ;-)
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 
the struct regmap seems to be regmap private - not for use in drivers.
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.
You can have a look at my current development code of the driver in question 
in this mail:
https://lkml.org/lkml/2012/10/23/374
 
> > > Setting the direction to output is an atomic set direction and set
> > > value. The register number is different from setting the value only.
> > > So after a successful call, I want to update the cache of output value
> > > using regcache_write. This is also not publicy available api. The only
> > > thing I could do is invalidate the whole cache using
> > > regcache_mark_dirty. (3rd problem)
> > 
> > Same thing.
> 
> The above doesn't sound like marking the register as dirty is going to
> be helpful...  at most you'd want to discard the cache which is already
> supported per register.  Marking the register as dirty would leave the
> existing (presumably wrong if there's a problem) value in place in the
> cache.  It's not at all clear to me that a cache is even useful for
> these registers.
> 
> > > I attach my current working state here. This is NOT WORKING and only as
> > > reference.
> > 
> > So if you want to do this with regmap, I suggest the way forward would
> > be to go solve it at the root by implementing something like
> > drivers/base/regmap/regmap-usb-ctrl.c with your desired semantics
> > and using that.
> 
> So the issue is that we're trying to use regmap for some USB device?

This is not an issue, since I can implement my regmap_bus for this inside my 
driver.

Thanks for your help!

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

Powered by blists - more mailing lists