Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "pavel@ucw.cz" <pavel@ucw.cz>
Cc: "dmurphy@ti.com" <dmurphy@ti.com>,
	"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>,
	"jacek.anaszewski@gmail.com" <jacek.anaszewski@gmail.com>,
	"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v10 00/13] Support ROHM BD71828 PMIC
Date: Wed, 26 Feb 2020 14:25:31 +0000
Message-ID: <b13161db0589951f86f241d5af8e8daa899be80d.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20200226134203.GD4080@duo.ucw.cz>

Hello Pavel,

I dropped some of the recipients as I don't think this is interesting
for everybody :)

On Wed, 2020-02-26 at 14:42 +0100, Pavel Machek wrote:
> Hi!
> 
> > > > Changelog v10:
> > > >   - Split RTC patch to a BD70528 fix (which hopefully goes to
> > > > 5.4)
> > > > and to
> > > >     BD71828 support
> > > 
> > > Still missing LED Acks.
> > > 
> > 
> > Yep. I know. I haven't heard from Pavel recently and the patch 12
> > definitely requires his ack. Can you take the other patches in and
> > leave 12 and 13 out for now? I can continue work on LEDs with Pavel
> > but
> > I would really like to have the regulators working and BD70528 RTC
> > fixed in next release...
> 
> Going through my backlogs now. You taking patches up-to 11 so
> that we have two left sounds good :-).

Nice to hear you're back in the business :)
This series (except patches 12 and 13) was merged to linux 5.6-rc1.

I have not continued work with the patch 12 as I am not entirely happy
with that approach even myself.

I still think the led core should parse common led bindings.

What I am wondering is if LED core should provide interface which could
register an array of LEDs at one go - by taking an array of LED descs
from the driver and by scanning through the child nodes in given LED
controller's root node. Or if we should stick to the approach
introduced in the patch 12 - which requires own 'register call' per
LED.

Problem with latter approach is that it requires the LED driver to know
how many LEDs there is attached - or then to ignore the errors from
register function assuming error is caused by missing LED. So currently
(after I looked through more of the existing drivers) it seems to me
the individual drivers need to either hard-code number of LEDs (which
might not be a real problem though) or anyways check the DT for LED
nodes and call the registration for each LED based on the DT nodes.

Ideally I would like to see approach where the LED controller driver
would only fill LED descriptors for possible LED connections - and give
that to a registration API. The registration API would see the
descriptors and check which of the descs match to a LED given in DT &&
register those LED devices. That would help LED drivers with no special
DT properties to truly get rid of DT parsing.

Eg, as a quickly written pseudo code to explain the idea:
driver would fill some array like:

struct const led_descriptors my_leds[] = {
	{
		.id = MY_LED1,
		.ops = led_control_functions,
		.match_data = &dt_match_data,
		.is_optional = true,
		.of_match_cb = my_controller_specific_dt_parser,
	},
	{
		.id = MY_LED2,
		.ops = led_control_functions,
		.match_
data = &dt_match_data,
		.is_optional = true,
		.of_match_cb =
my_controller_specific_dt_parser,
	},
};

where:
 - led_control_functions would be function pointers to set the LED
states etc.
 - id would be a LED ID which the led control functions could use to
ide1ntify LED in controller
 - match_data would contain DT node match information the LED core
could use when searching the LED from DT
 - is_optional would tell whether the core should treat missing LED DT
node as error
 - of_match_cb would be a callback to call when core has parsed common
DT properties so that the driver can parse any driver specific dt
stuff.

and the driver could pass this and the LED controller DT node to
registration API which would "mass register" all found LEDs.

I think it would not be a huge thing to implement - but it definitely
is some work. And if this idea is strongly objected - then I may not
have the energy to push it through... So.. I would like to know what
people think of it? Is it really worth of trying? Or should I stick
with the approach presented in this series? Or should I just forget it
and add yet one more LED drivers implementing the same DT parsing loops
as others?


Best Regards
	Matti

      reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-17  9:30 Matti Vaittinen
2020-01-17  9:31 ` [PATCH v10 01/13] dt-bindings: leds: ROHM BD71282 PMIC LED driver Matti Vaittinen
2020-01-17  9:32 ` [PATCH v10 02/13] dt-bindings: mfd: Document ROHM BD71828 bindings Matti Vaittinen
2020-01-17  9:34 ` [PATCH v10 03/13] mfd: rohm PMICs - use platform_device_id to match MFD sub-devices Matti Vaittinen
2020-01-17  9:35 ` [PATCH v10 04/13] mfd: bd718x7: Add compatible for BD71850 Matti Vaittinen
2020-01-17  9:36 ` [PATCH v10 05/13] mfd: bd71828: Support ROHM BD71828 PMIC - core Matti Vaittinen
2020-01-17  9:36 ` [PATCH v10 06/13] mfd: input: bd71828: Add power-key support Matti Vaittinen
2020-01-17  9:37 ` [PATCH v10 07/13] clk: bd718x7: Support ROHM BD71828 clk block Matti Vaittinen
2020-01-17  9:38 ` [PATCH v10 08/13] regulator: bd718x7: Split driver to common and bd718x7 specific parts Matti Vaittinen
2020-01-17 10:28   ` Lee Jones
2020-01-17 10:43     ` Vaittinen, Matti
2020-01-17 13:40       ` Lee Jones
2020-01-20  7:06         ` Vaittinen, Matti
2020-01-17  9:40 ` [PATCH v10 09/13] mfd: bd70528: Fix hour register mask Matti Vaittinen
2020-01-17 10:21   ` Lee Jones
2020-01-17  9:42 ` [PATCH v10 10/13] rtc: bd70528: add BD71828 support Matti Vaittinen
2020-01-17  9:42 ` [PATCH v10 11/13] gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs Matti Vaittinen
2020-01-17 10:21   ` Lee Jones
2020-01-20  6:48     ` Vaittinen, Matti
2020-01-20  8:02       ` Lee Jones
2020-01-23 15:18     ` Linus Walleij
2020-01-17  9:43 ` [PATCH v10 12/13] leds: Add common LED binding parsing support to LED class/core Matti Vaittinen
2020-01-20  8:56   ` Vaittinen, Matti
2020-01-17  9:44 ` [PATCH v10 13/13] led: bd71828: Support LED outputs on ROHM BD71828 PMIC Matti Vaittinen
2020-01-17 10:30 ` [PATCH v10 00/13] Support " Lee Jones
2020-01-17 10:48   ` Vaittinen, Matti
2020-01-17 13:44     ` Lee Jones
2020-02-26 13:42     ` Pavel Machek
2020-02-26 14:25       ` Vaittinen, Matti [this message]

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=b13161db0589951f86f241d5af8e8daa899be80d.camel@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmurphy@ti.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=mazziesaccount@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=robh+dt@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