Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: Dan Murphy <dmurphy@ti.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: <jacek.anaszewski@gmail.com>, <robh@kernel.org>,
	<marek.behun@nic.cz>, <devicetree@vger.kernel.org>,
	<linux-leds@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v32 2/6] leds: lp50xx: Add the LP50XX family of the RGB LED driver
Date: Wed, 12 Aug 2020 11:00:23 -0500
Message-ID: <5efcba1b-995e-b6fd-9004-fbafaae5b8a3@ti.com> (raw)
In-Reply-To: <20200811222602.GA10181@amd>

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


  reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-22 15:31 [PATCH v32 0/6] Multicolor Framework v32 Dan Murphy
2020-07-22 15:31 ` [PATCH v32 1/6] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers Dan Murphy
2020-07-28 13:39   ` Dan Murphy
2020-08-04 19:55     ` Dan Murphy
2020-08-07 13:42       ` Dan Murphy
2020-08-11 10:32         ` Pavel Machek
2020-08-11 10:20   ` Pavel Machek
2020-08-11 17:07     ` Dan Murphy
2020-07-22 15:31 ` [PATCH v32 2/6] leds: lp50xx: Add the LP50XX family of the RGB LED driver Dan Murphy
2020-08-11 10:29   ` Pavel Machek
2020-08-11 10:31   ` Pavel Machek
2020-08-11 10:54   ` Pavel Machek
2020-08-11 17:27     ` Dan Murphy
2020-08-11 22:01       ` Pavel Machek
2020-08-11 22:16         ` Dan Murphy
2020-08-11 22:26           ` Pavel Machek
2020-08-12 16:00             ` Dan Murphy [this message]
2020-07-22 15:31 ` [PATCH v32 3/6] ARM: defconfig: u8500: Add LP55XX_COMMON config flag Dan Murphy
2020-07-28 13:38   ` Dan Murphy
2020-08-03 23:14   ` Linus Walleij
2020-07-22 15:31 ` [PATCH v32 4/6] ARM: dts: n900: Add reg property to the LP5523 channel node Dan Murphy
2020-07-22 15:31 ` [PATCH v32 5/6] ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 " Dan Murphy
2020-07-22 15:31 ` [PATCH v32 6/6] ARM: dts: ste-href: Add reg property to the LP5521 channel nodes Dan Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5efcba1b-995e-b6fd-9004-fbafaae5b8a3@ti.com \
    --to=dmurphy@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=marek.behun@nic.cz \
    --cc=pavel@ucw.cz \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git