Skip to contentSkip to conversation

LED subsystem child DT node ref counting

DISCUSSION Jacek Anaszewski 2 participants 4 messages Apr 17, 2015 – Apr 28, 2015 LED

lore
Share & tools
lore
#0 lore raw

Hi,

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

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'd like to ask if this is guaranteed to be always true.

Signature · 2 lines
Best Regards,
Jacek Anaszewski
#1 lore raw

Gentle reminder. Can anyone clarify this?

Quoted 32 lines · Jacek Anaszewski

Hi,

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

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'd like to ask if this is guaranteed to be always true.

Signature · 2 lines
Best Regards,
Jacek Anaszewski
#2 lore raw
In reply to Jacek Anaszewski
Quoted 4 lines · Jacek Anaszewski

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.

Quoted 19 lines

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.

Quoted 5 lines

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.

Rob

#3 lore raw
Quoted 35 lines · Rob Herring
Quoted 4 lines · Jacek Anaszewski

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.

Quoted 19 lines

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.

Quoted 5 lines

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.

Quoted 3 lines

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.

Signature · 2 lines
Best Regards,
Jacek Anaszewski