Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1378795

Re: [PATCH v1 3/3] nvmem: core: fix regmap accessor usage

Show all headers | View raw

On 14/04/16 07:42, Mark Brown wrote:
> On Wed, Apr 13, 2016 at 06:39:14PM +0100, Srinivas Kandagatla wrote:
>> With the recent patch of removing the raw accessors form regmap-mmio,
>> broke the qfprom support. This patch attempts to fix that regression
>> by adding check before calling regmap raw accessors functions.
>>
>> Without this patch nvmem providers based on regmap mmio would not work.
>
...
>> +	if (regmap_can_raw_read(map))
>> +		return regmap_raw_read(map, reg, val, bytes);
>> +
>> +	for (i = 0; i < word_count; i++) {
>> +		ret = regmap_read(map, reg + i * nvmem->stride, &ival);
>> +		if (ret != 0)
>> +			return ret;
>> +
>> +		switch (nvmem->word_size) {
>> +		case 4:
>> +			u32_buf[i] = ival;
>> +			break;
>
> This is clearly an abstraction failure and probably broken for systems
> where the device endianness does not match the CPU endianness (like most
> big endian systems, the device hardware normally stays in little endian
> mode).
>
> We need to figure out what this stuff is trying to do before we go any
> further, I'm honestly not entirely clear.  I *think* that if regmap is a
> good fit then it probably wants to use the bulk operations rather than
> the raw operations (the bulk operations are AFAICT what is being open
> coded above) but bulk I/O still does endianness handling and I'm not
> sure if that's desired or not.  If the nvmem code really is just trying
> to get bytestreams then regmap really isn't what it should be using,
> it's all about dealing with registers and trying to force bytestreams
> through it seems like it's just going to lead to fragility.  Either
> whatever is happening should be abstracted within regmap or we shouldn't
> be using regmap.
Thanks for your comments,

I totally agree that there is an abstraction failure here in both sides. 
It should be fixed. moving to using bulk apis would solve the nvmem 
problem for now. But for long term, using regmap should be totally 
removed from nvmem and directly use the reg read/write callbacks from 
nvmem providers, This would be much robust solution. This was indeed 
Maxime's first proposal. I will try to fix it up and see how it looks 
without regmap.

>
> I'll try to have another look at this later.
>

thanks,
srini

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v1 0/3] nvmem: core: fix regmap accessor usage. Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-04-13 19:40 +0200
  [PATCH v1 3/3] nvmem: core: fix regmap accessor usage Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-04-13 19:50 +0200
    Re: [PATCH v1 3/3] nvmem: core: fix regmap accessor usage Mark Brown <broonie@kernel.org> - 2016-04-14 08:50 +0200
      Re: [PATCH v1 3/3] nvmem: core: fix regmap accessor usage Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-04-14 14:40 +0200
        Re: [PATCH v1 3/3] nvmem: core: fix regmap accessor usage Mark Brown <broonie@kernel.org> - 2016-04-14 17:20 +0200
          Re: [PATCH v1 3/3] nvmem: core: fix regmap accessor usage Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-04-14 18:50 +0200
  [PATCH v1 1/3] regmap: add regmap_can_raw_read() api Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-04-13 19:50 +0200
    Re: [PATCH v1 1/3] regmap: add regmap_can_raw_read() api Mark Brown <broonie@kernel.org> - 2016-04-14 08:00 +0200

csiph-web