Re: [PATCH v32 2/6] leds: lp50xx: Add the LP50XX family of the RGB LED driver

From: Dan Murphy
Date: Wed Aug 12 2020 - 12:00:47 EST


Pavel

On 8/11/20 5:26 PM, Pavel Machek wrote:
Hi!

Well it depends on where we want to create the default cache values.

Either we run through a for..loop during driver probe and delay device start
up or we keep the simple arrays and increase the driver total size.
for loop will be better.

Plus, REGCACHE_RBTREE is very likely overkill.
Well if I eliminate the reg_cache then I can eliminate the defaults too.
I'm not asking for that. But please investigate REGCACHE_FLAT.

Pavel

After looking at this a loop makes no sense here.  The regmap call back values are determined at build time not during runtime.

Adding a loop here makes the code more complex just to reduce the overall LoC.  In adding the loop the reg_default array will have to be re-allocated and copied at run time and then be expanded to include the additional values.

And the regmap defaults call backs will need to be updated to reflect the new values.  And these are part of a const struct because the devm_regmap_init declares the config as a const.

    .reg_defaults = lp5012_reg_defs,
    .num_reg_defaults = ARRAY_SIZE(lp5012_reg_defs),

So I am not sure that adding a loop here just to eliminate some LoC is adding any value here.  I can remove the #defines for the unused runtime registers and hard code the additional register addresses in the default array.  That will at least eliminate some LoC and reduce the object size.

I have no issue with using the REGCACHE_FLAT so I will make that change.

Dan