Hello Jacek, On 09.04.2017 14:37, Jacek Anaszewski wrote: > Hello Felix, > > On 04/07/2017 10:22 AM, Felix Brack wrote: >> Hello Jacek, >> >> On 06.04.2017 21:00, Jacek Anaszewski wrote: >>> Hi Pavel, >>> >>> On 04/06/2017 05:50 PM, Pavel Machek wrote: >>>> Hi! >>>> >>>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-pca9532.txt b/Documentation/devicetree/bindings/leds/leds-pca9532.txt >>>>>> index 198f3ba..8374075 100644 >>>>>> --- a/Documentation/devicetree/bindings/leds/leds-pca9532.txt >>>>>> +++ b/Documentation/devicetree/bindings/leds/leds-pca9532.txt >>>>>> @@ -17,6 +17,8 @@ Optional sub-node properties: >>>>>> - label: see Documentation/devicetree/bindings/leds/common.txt >>>>>> - type: Output configuration, see dt-bindings/leds/leds-pca9532.h (default NONE) >>>>>> - linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt >>>>>> + - default-state: see Documentation/devicetree/bindings/leds/common.txt >>>>>> + This property is only valid for sub-nodes of type <PCA9532_TYPE_LED>. >>>>>> >>>>>> Example: >>>>>> #include <dt-bindings/leds/leds-pca9532.h> >>>>>> @@ -33,6 +35,14 @@ Example: >>>>>> label = "pca:green:power"; >>>>>> type = <PCA9532_TYPE_LED>; >>>>>> }; >>>>>> + kernel-booting { >>>>>> + type = <PCA9532_TYPE_LED>; >>>>>> + default-state = "on"; >>>>>> + }; >>>>>> + sys-stat { >>>>>> + type = <PCA9532_TYPE_LED>; >>>>>> + default-state = "keep"; // don't touch, was set by U-Boot >>>>>> + }; >>>>> >>>>> Adjusted above indentation to match the preceding lines. >>>> >>>>>> @@ -475,6 +494,16 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np) >>>>>> of_property_read_u32(child, "type", &pdata->leds[i].type); >>>>>> of_property_read_string(child, "linux,default-trigger", >>>>>> &pdata->leds[i].default_trigger); >>>>>> + if (!of_property_read_string(child, "default-state", &state)) { >>>>>> + if (!strcmp(state, "on")) >>>>>> + pdata->leds[i].state = PCA9532_ON; >>>>>> + else if (!strcmp(state, "keep")) >>>>>> + pdata->leds[i].state = PCA9532_KEEP; >>>>>> + else if (!strcmp(state, "pwm0")) >>>>>> + pdata->leds[i].state = PCA9532_PWM0; >>>>>> + else if (!strcmp(state, "pwm1")) >>>>>> + pdata->leds[i].state = PCA9532_PWM1; >>>>>> + } >>>>>> if (++i >= maxleds) { >>>>>> of_node_put(child); >>>>>> break; >>>> >>>> This seems to look for "pwm0" and "pwm1" strings, which do not seem to >>>> be documented. >>>> >>>> Plus... is it useful to have default-state? We already have default >>>> trigger. If we keep the value by default (on PC, we do something like >>>> that) this patch should not be neccessary? >>> >>> Thanks for the heads-up. Dropping the patch for now. >> >> No, please do not drop the patch. >> >>> I guess that pwm0/1 got propagated to v2 by an omission. >>> >> >> Yes, I agree. However the two strings do not break anything and behave >> analog to the 'on' or 'keep' string. Though this code could be removed >> if absolutely necessary. An alternative would be to add a description >> for the strings. Just to be clear: these strings have nothing to with >> the exposition of device specific registers to the DT. >> >>> Regarding default-on: Felix, do you have any use case that require >>> default-on set to "keep"? >>> >> >> This patch is not about 'default-on' which is a value that could be >> assigned to the property 'linux,default-trigger' (according to DT >> bindings documentation file 'common.txt'). >> My patch does not introduce anything new with the'keep' state, it rather >> completes the existing bindings according to the description in >> Documentation/devicetree/bindings/leds/common.txt which states: >> >> .... >> - default-state : The initial state of the LED. Valid values are "on", >> "off", and "keep". If the LED is already on or off and the default-state >> property is set the to same value, then no glitch should be produced >> where the LED momentarily turns off (or on). The "keep" setting will >> keep the LED at whatever its current state is, without producing a >> glitch. The default is off if this property is not present. >> .... >> >> One of my use cases is to turn a LED on by U-Boot. This LED must remain >> on until eventually, under certain conditions, some userland code >> changes it's state. >> Setting 'default-state' to 'keep' is how you can sort of tell the >> kernel, or better the driver, 'not to initialize the LED' which would >> turn it off. > > Thanks for the explanation. Could you please sent v3 with removed pwm* > cases then? > Yes, I will try to do so next week. -- regards Felix -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html