From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacek Anaszewski Subject: Re: [PATCH v2] Extend pca9532 device tree support Date: Thu, 6 Apr 2017 21:00:09 +0200 Message-ID: References: <1490880833-22199-1-git-send-email-fb@ltec.ch> <20170406155005.GA10363@amd> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20170406155005.GA10363@amd> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Pavel Machek , Felix Brack Cc: rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, riku.voipio-X3B1VOXEql0@public.gmane.org, linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-leds@vger.kernel.org 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 . >>> >>> Example: >>> #include >>> @@ -33,6 +35,14 @@ Example: >>> label = "pca:green:power"; >>> type = ; >>> }; >>> + kernel-booting { >>> + type = ; >>> + default-state = "on"; >>> + }; >>> + sys-stat { >>> + type = ; >>> + 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. I guess that pwm0/1 got propagated to v2 by an omission. Regarding default-on: Felix, do you have any use case that require default-on set to "keep"? -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752632AbdDFTBE (ORCPT ); Thu, 6 Apr 2017 15:01:04 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:36764 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752271AbdDFTAy (ORCPT ); Thu, 6 Apr 2017 15:00:54 -0400 Subject: Re: [PATCH v2] Extend pca9532 device tree support To: Pavel Machek , Felix Brack References: <1490880833-22199-1-git-send-email-fb@ltec.ch> <20170406155005.GA10363@amd> Cc: rpurdie@rpsys.net, mark.rutland@arm.com, riku.voipio@iki.fi, linux-leds@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org From: Jacek Anaszewski X-Enigmail-Draft-Status: N1110 Message-ID: Date: Thu, 6 Apr 2017 21:00:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170406155005.GA10363@amd> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 . >>> >>> Example: >>> #include >>> @@ -33,6 +35,14 @@ Example: >>> label = "pca:green:power"; >>> type = ; >>> }; >>> + kernel-booting { >>> + type = ; >>> + default-state = "on"; >>> + }; >>> + sys-stat { >>> + type = ; >>> + 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. I guess that pwm0/1 got propagated to v2 by an omission. Regarding default-on: Felix, do you have any use case that require default-on set to "keep"? -- Best regards, Jacek Anaszewski