lkml.org 
[lkml]   [2017]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] leds: lp55xx: make various arrays static const
From
Date
Hi,

On 06/29/2017 10:48 PM, Pavel Machek wrote:
> Hi!
>
>> Thanks for the patch.
>>
>> Do you have some profiling results showing the benefit of these changes?
>> It seems that these functions are called only on driver initialization.
>> Making the local arrays static will prevent releasing this memory even
>> though it will be no longer needed.
>>
>> Anyway, the size of arrays is 3 bytes, so the impact of these changes
>> seems to be virtually negligible. I am not taking this patch unless
>> you are able to provide the numbers showing the gain.
>
> C will have to initialize the array on stack, then copy values

OK, similar justification is used in case of few other similar patches.

Applied to the for-next branch of linux-leds.git. It's rather
cosmetic change so only a few days of testing before sending
upstream should be enough.

Thanks,
Jacek Anaszewski

> there.. which is ugly. "static const" also tells the reader what is
> going on there (nothing).
>
> I'd say this is a good patch.
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
>>> +++ b/drivers/leds/leds-lp5523.c
>>> @@ -168,13 +168,13 @@ static int lp5523_post_init_device(struct lp55xx_chip *chip)
>>> static void lp5523_load_engine(struct lp55xx_chip *chip)
>>> {
>>> enum lp55xx_engine_index idx = chip->engine_idx;
>>> - u8 mask[] = {
>>> + static const u8 mask[] = {
>>> [LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
>>> [LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
>>> [LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
>>> };
>>>
>>> - u8 val[] = {
>>> + static const u8 val[] = {
>>> [LP55XX_ENGINE_1] = LP5523_LOAD_ENG1,
>>> [LP55XX_ENGINE_2] = LP5523_LOAD_ENG2,
>>> [LP55XX_ENGINE_3] = LP5523_LOAD_ENG3,
>>> @@ -188,7 +188,7 @@ static void lp5523_load_engine(struct lp55xx_chip *chip)
>>> static void lp5523_load_engine_and_select_page(struct lp55xx_chip *chip)
>>> {
>>> enum lp55xx_engine_index idx = chip->engine_idx;
>>> - u8 page_sel[] = {
>>> + static const u8 page_sel[] = {
>>> [LP55XX_ENGINE_1] = LP5523_PAGE_ENG1,
>>> [LP55XX_ENGINE_2] = LP5523_PAGE_ENG2,
>>> [LP55XX_ENGINE_3] = LP5523_PAGE_ENG3,
>>> @@ -208,7 +208,7 @@ static void lp5523_stop_all_engines(struct lp55xx_chip *chip)
>>> static void lp5523_stop_engine(struct lp55xx_chip *chip)
>>> {
>>> enum lp55xx_engine_index idx = chip->engine_idx;
>>> - u8 mask[] = {
>>> + static const u8 mask[] = {
>>> [LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
>>> [LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
>>> [LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
>>> @@ -505,7 +505,7 @@ static int lp5523_load_mux(struct lp55xx_chip *chip, u16 mux, int nr)
>>> {
>>> struct lp55xx_engine *engine = &chip->engines[nr - 1];
>>> int ret;
>>> - u8 mux_page[] = {
>>> + static const u8 mux_page[] = {
>>> [LP55XX_ENGINE_1] = LP5523_PAGE_MUX1,
>>> [LP55XX_ENGINE_2] = LP5523_PAGE_MUX2,
>>> [LP55XX_ENGINE_3] = LP5523_PAGE_MUX3,
>>>
>

--
Best regards,
Jacek Anaszewski

\
 
 \ /
  Last update: 2017-06-30 23:28    [W:0.043 / U:27.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site