On 04/10/2019 16:40, Mark Brown wrote:
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).
This is not a copy of a device of parent's of_node or something like that.
You can think of a LED controller as a bus. It 'enumerates' its children
LED, create the children devices (one per LED) and provides the
functions to interact with them.
The device node we are talking about here is a per-LED thing, it is a
child node of the node of the LED controller.
here is an example:
tlc59108: tlc59116@40 { /* this is the node for the LED controller */
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
compatible = "ti,tlc59108";
reg = <0x40>;
backlight_led: led@2 { /* this is the node of one LED attached
to pin#2 of the LED controller */
power-supply = <&bkl_fixed>;
reg = <0x2>;
};
other_led: led@3 { /* this is the node another LED attached to
pin #3 of the LED controller */
power-supply = <®_3v3>;
reg = <0x3>;
};
};
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.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
