Re: Should regulator core support parsing OF based fwnode?

[Mark Brown]

On Fri, Oct 04, 2019 at 03:33:13PM +0200, Jean-Jacques Hiblot wrote:
> On 04/10/2019 13:39, Mark Brown wrote:

> > Consumers should just be able to request a regulator without having to
> > worry about how that's being provided - they should have no knowledge at
> > all of firmware bindings or platform data for defining this.  If they
> > do that suggests there's an abstraction issue somewhere, what makes you
> > think that doing something with of_node is required?

> The regulator core accesses consumer->of_node to get a phandle to a
> regulator's node. The trouble arises from the fact that the LED core does
> not populate of_node anymore, instead it populates fwnode. This allows the
> LED core to be agnostic of ACPI or OF to get the properties of a LED.

Why is the LED core populating anything?  Is the LED core copying bits
out of the struct device for the actual device into a synthetic device
rather than passing the actual device in?  That really doesn't seem like
a good idea, it's likely to lead to things like this where you don't
copy something that's required (or worse where something directly in the
struct device that can't be copied is needed).

> IMO it is better to populate both of_node and fwnode in the LED core at the
> moment. It has already been fixed this way for the platform driver [0], MTD
> [1] and PCI-OF [2].

Yeah, if you're going to be copying stuff out of the real device I'd
copy the of_node as well.

> > Further, unless you have LEDs that work without power you probably
> > shouldn't be using _get_optional() for their supply.  That interface is
> > intended only for supplies that may be physically absent.

> Not all LEDs have a regulator to provide the power. The power can be
> supplied by the LED controller for example.

This code probably shouldn't be being run at all for LEDs like that, I
was assuming this was just for GPIO LEDs and similar rather than all
LEDs.

Attachment: signature.asc
Description: PGP signature