Re: Should regulator core support parsing OF based fwnode?

From: Jacek Anaszewski <jacek.anaszewski@gmail.com>
To: Mark Brown <broonie@kernel.org>, Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: mark.rutland@arm.com, daniel.thompson@linaro.org,
	Liam Girdwood <lgirdwood@gmail.com>,
	tomi.valkeinen@ti.com,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, pavel@ucw.cz, lee.jones@linaro.org,
	linux-leds@vger.kernel.org, dmurphy@ti.com
Subject: Re: Should regulator core support parsing OF based fwnode?
Date: Fri, 4 Oct 2019 22:55:17 +0200	[thread overview]
Message-ID: <1a81148b-7445-6b49-9cef-99c12549f715@gmail.com> (raw)
In-Reply-To: <20191004174233.GF4866@sirena.co.uk>

On 10/4/19 7:42 PM, Mark Brown wrote:
> On Fri, Oct 04, 2019 at 06:12:52PM +0200, Jean-Jacques Hiblot wrote:
>> On 04/10/2019 17:58, Mark Brown wrote:
> 
>>> Regulator supplies are supposed to be defined at the chip level rather
>>> than subfunctions with names corresponding to the names on the chip.
> 
> ...
> 
>>> good chance that they come up with the same mapping.  The supply_alias
>>> interface is there to allow mapping these through to subfunctions if
>>> needed, it looks like the LED framework should be using this.
> 
>> In case of current-sink LED drivers, each LED can be powered by a different
>> regulator, because the driver is only a switch between the LED cathod and
>> the ground.
> 
> Sure, it's common for devices to have supplies that are only needed by
> one part of the chip which is why we have the supply_alias interface for
> mapping things through.
> 
>>> That said if you are doing the above and the LEDs are appearing as
>>> devices it's extremely surprising that their of_node might not be
>>> initialized.
> 
>> That is because this is usually done by the platform core which is not
>> involved here.
> 
> The surprise is more that it got instantiated from the DT without
> keeping the node around than how it happened.

This is LED class driver that is instantiated from DT and it in
turn registers LED class devices - one per corresponding DT child
node found in the parent LED controller node.

LED class device is created via device_create_with_groups() that
returns struct device with uninitialized of_node. This is the point
we're discussing. In order to be able to obtain regulator handle
in the LED core from DT we need to have exactly of_node
(of course provided it contains *-supply property).

Here the question may arise why it is not the LED core that instantiates
LED class devices per child nodes - well it is tricky to implement
due to disparate possible LED channel routings across devices.
We can think of adding it to the TODO list, but this is another story.

This is the background. However, since Jean pointed to the few
other similar cases when of_node is being initialized in addition
to fwnode I deem there is no issue and we can do the same in the
LED core.

No action in regulator core is required then.

-- 
Best regards,
Jacek Anaszewski