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

[Lars Poeschel <poeschel@lemonage.de>]

On Thursday 25 October 2012 at 18:06:51, Mark Brown wrote:
> Are you saying that whoever designed this USB device has done it so that
> it takes a byte stream with something like RRVV where RR is the register
> and VV is the value?  That's the marshalling, as you'll have seen that's
> done before the buses see the data.

Aaah, I understand! Now your hint that the marshalling should be made 
pluggable makes sense to me.
The device is more complicated and worse. For the first 16 gpio pins it is 
something like 01RR0000FD00VV for setting the value and 03RR00FF00FEVV00 for 
setting the direction. For the second 16 gpio pins it is completly different. 
And yes, for this I completely remarshal what I get marshalled from regmap in 
my bus implementation. This is bad. Thank you for your clarification. This 
was not that clear to me as I began to use regmap and I think it also was not 
to Linus Walleji, as he asked me to change the driver to use regmap. This is 
why he brought you into our discussion.

I am a bit unsure what to do now. Since Linus said he would take my driver 
without regmap, this will be my way for the driver.
I had a look at the regmap code in sense of making the marshalling pluggable. 
It seems that the current assumption is that the register is always the first 
value in the buffer after the marshalling happend which would not fit for my 
device. Changing that is a bit hard. This marshalling is not the only issue. 
For reading values I would need a split buffer or two seperate buffers. 
Reading the device means doing a usb write using the first buffer telling the 
device which register I am interested in and then doing a usb read into the 
second buffer which then contains the actual value of the register. (Both 
transfers marshalled different of course.)

> Is the I/O selection per GPIO or per device?

The I/O selection is per GPIO.
 
> > > So just invalidate the cache and it'll get restored next time we look
> > > at the register.
> > 
> > Yes, this is exactly what I gave as an alternative in my second to last
> > mail. But this invalidates the whole register map although I just want
> > to update one register value.
> 
> So add that functionality to the core if it's not there.

If I reach the limitations of some api/framework/something it is not my first 
idea to change that api but if I did something wrong. But this word from a 
maintainer is clear!