Re: LED subsystem child DT node ref counting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/27/2015 03:57 PM, Rob Herring wrote:
On Fri, Apr 17, 2015 at 6:03 AM, Jacek Anaszewski
<j.anaszewski@xxxxxxxxxxx> wrote:
Hi,

I'd like to clarify whether LED subsystem drivers behave correctly
or not, regarding child DT nodes reference counting.

In general, the DT reference counting is broken. It is really only
used on pSeries and only for certain nodes on it.

Single LED controller can have connected more then one LED to it.
The LEDs are represented by child DT nodes of the node representing
the controller (see Documentation/devicetree/bindings/leds).

LED subsystem drivers parse child DT nodes and use the node name,
or 'label' property string as the LED class device name.

This is usually accomplished like below:

for_each_child_of_node(np, child) {
     ...
     led.name = of_get_property(child, "label", NULL) ? : child->name;


The question is whether reference count of the child node shouldn't
be increased here with of_node_get(child). Whereas intuitively it could
be thought of as a right thing to do, empirical experiments don't
necessary confirm that.

When I print the value of child_node->kobj.kref.refcount.counter
inside for_each loop it is 3 and and after leaving the loop it gets
decreased to 2. On driver removal the value is also 2. It means that
label is available all the time, without increasing child node ref
counter.

I believe the ref count is only left incremented if you break from the
loop. Otherwise, it is only incremented during the loop one child at a
time.

Child node ref count is 3 right after obtaining it with
of_get_next_available_child.

Nonetheless my further source code investigation revealed that this
does not pose an issue for the LED subsystem, as the label is copied
in the device_create_with_groups function.

The related 'name' property should be probably removed from the
struct led_classdev, as it seems to be used only for device name
initialization. It will require modifications in the existing LED
drivers though.

--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux