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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5efcba1b-995e-b6fd-9004-fbafaae5b8a3@ti.com>
Date:   Wed, 12 Aug 2020 11:00:23 -0500
From:   Dan Murphy <dmurphy@...com>
To:     Pavel Machek <pavel@....cz>
CC:     <jacek.anaszewski@...il.com>, <robh@...nel.org>,
        <marek.behun@....cz>, <devicetree@...r.kernel.org>,
        <linux-leds@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v32 2/6] leds: lp50xx: Add the LP50XX family of the RGB
 LED driver

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