Re: [PATCH 1/5] max44000: Initial commit

From: Lars-Peter Clausen
Date: Mon Apr 18 2016 - 06:59:24 EST


On 04/18/2016 12:32 PM, Mark Brown wrote:
[...]
>>>> This always seems like a good idea, but tends to cause issues.
>>>> FLAT is really only meant for very high performance devices, you
>>>> are probably better with something else here. If you are doing this
>>>> deliberately to make the below writes actually occur, then please
>>>> add a comment here.
>
>>> I used REGCACHE_FLAT because my device has a very small number of
>>> registers and I assume it uses less memory. Honestly it would make
>>> sense for regmap to include a REGCACHE_AUTO cache_type and pick the
>>> cache implementation automatically based on number of registers.
>
>> I've fallen for that one in the past as well. AUTO would indeed
>> be good if it was easy to do.
>
> It's extremely easy to do. Unless you've got a good reason to do
> anything else you should always be using an rbtree. The core would
> never select anything else.

Just to add some technical background, maybe that helps to clear things up.
The rbtree does not have a one node for each register, it has one node for
each continuous register region. You can think of the rbtree regmap as a
tree with a flat cache at each node. If there is only one continuous region
there will only be one node and the behavior is very similar to the flat cache.

The memory overhead is the size of single node, which is usually negligible.

For register reads and writes there is a slight overhead of looking up the
node. But since the rbtree caches the node that was looked up last the
overhead is just checking if the current node is still the correct one
(which it will be since there is only one node). This check is about 4-5 hw
instructions which is completely negligible to the time it takes to execute
the SPI or I2C transfer. The only place where flat makes sense is where the
hardware register access itself only takes a few CPU cycles and where the
overhead of the lookup is noticeable.

Attachment: signature.asc
Description: OpenPGP digital signature