Message ID | 20230202101032.26737-2-maarten.zanders@mind.be |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp152582wrn; Thu, 2 Feb 2023 02:18:28 -0800 (PST) X-Google-Smtp-Source: AK7set9+vHPrgIDWdaCsE8NyEj7G9KpLpYzazaBkWKj8ibA6zO0EZEF26WVxZG0/V3HCSOGKUdbe X-Received: by 2002:a17:906:ad96:b0:87b:d79f:9945 with SMTP id la22-20020a170906ad9600b0087bd79f9945mr6259915ejb.2.1675333108379; Thu, 02 Feb 2023 02:18:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675333108; cv=none; d=google.com; s=arc-20160816; b=CTmkT8yYeTiWyniLFfUgu2IKc2fRVoeyzL5ufOO6PnC9qC7XboYNOMgPmmG4QXv05+ wCv3EYxEvQXj3T09JdpcC8ANqkuDG+dSbXrzLfVgnI0k0fjEQmoVG3zsKU9WbBNFIdi1 AhuGfxNiihzcxWbjxDnnhSheU23b5epLldqpRMKTYkJvqV5dcPG8EegUqOXVqAPmN16m jHDdkE6QunudVnflJLasgaH7k6RxTaqco5+TjCYxWuE2ic5i9mRt9PQDeT8AFDCAlRIz hFncx9UyzofSHnXDdFckOwvD/z7rpxG6Jy65oKB1K6RZap4cq8ncSs+mRoROkxgmFwlH XvGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=PZ/eU/DKOSJTbU1YDMmaQL1fq94Z2preFzmC+8H/Gjo=; b=A41bEQxpfX/DM67ly6ivfdllSTsmMGcTVkRaIglwTs/iR2p58quI2qsh/jE0ARnIZk PGCxQYGaGZ5bk9RtXYPhrOFOsBbZ0L/rYFpgUMIQiURt9b4Wdtbygnbvz/P+YN6E9QYU +aYnJY81/RnGCQFPfx7Ag9ABwfKmf+BHmJPpyh3ckN0emAEiJd2pFmDbthpkZwKx7UGd VXVSHD2N3tzMR/So8KrnKTkbtfWSABAaGXg5hNZPOG1uYXI7gMdLe+VqQ9kHbry1OS1k JbV+PfkK/NRHo6I1Mg1oUCcc07nSYeIREBtbA4lBuIrXlOjFduVMJAC8t8/LRDgv2sLP ryQQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mind.be header.s=google header.b=dlq9Ei+i; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h21-20020aa7de15000000b004a25b52ae43si10512067edv.481.2023.02.02.02.18.04; Thu, 02 Feb 2023 02:18:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@mind.be header.s=google header.b=dlq9Ei+i; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232618AbjBBKLA (ORCPT <rfc822;il.mystafa@gmail.com> + 99 others); Thu, 2 Feb 2023 05:11:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232158AbjBBKKw (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 2 Feb 2023 05:10:52 -0500 Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A94FF875B7 for <linux-kernel@vger.kernel.org>; Thu, 2 Feb 2023 02:10:46 -0800 (PST) Received: by mail-ed1-x536.google.com with SMTP id v13so1416192eda.11 for <linux-kernel@vger.kernel.org>; Thu, 02 Feb 2023 02:10:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=PZ/eU/DKOSJTbU1YDMmaQL1fq94Z2preFzmC+8H/Gjo=; b=dlq9Ei+ies+wC8rqHr4+iStcSSd1YBuNlbxPxpucw6pmy9CAs13nEOikZ5BWFHXrSt 4E383ZAN2OJF0+kvlbvckkXieElTtInDird2S1kQz5ZSa80t4XsOPqaKZGKoPZBCALjM 5aXtX82oWDSwIyM9l2GGvmx0aD6zoiE5MYwUC6lVHU2vYkKYLJF0Zppnkdp5bmzuMhN3 6sozw9E9vt4QmVqF5tIYvXPIhbReVYOvJzvOPWmpr16h2SzI8g2E/Xt47NBF3gBRgij3 rshmwuAwsuWwh/YZt3QOCDWXsnnL2QqwkSXHypFmmQpT5YFDPMjsy73hkk0yTK5KKsKr wILg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PZ/eU/DKOSJTbU1YDMmaQL1fq94Z2preFzmC+8H/Gjo=; b=Wnau9GlDqn6UJADjPjt02xyX+AXXRq1IWHAz7EHUGRimP4Q8CfGGYxIP3UsQ3YU85i xD6tAzNbszClPMk7kQj9ia9BuGvVprwkNKHjUCvrnojiZQmg81fKtWG+hJ9YzQprX9Uh yo2k7/exm+19iYKloTxwJYlRAlF6ZK7GskAL5n2wsiRipvxzG9AS8Wewpnc2PzLOLiPq cPRc9o7vlDrfzEgXRhxEL7cxe37m1Y3iQaEXv9rkWpE4XV/V5PFaTHsM4NK4aROb8YYK IVmgnJ6WVLP6NXqh48JxKdvuqZg9JGviBuAR0I3PEPjBHPEsAiqNC9JcveEo2vSd3GFX tiAw== X-Gm-Message-State: AO0yUKXJl3hZmbTjWO31uwf/DbYldfKwrcx71I9WLiGkItVpf9lHKEM2 8BDqdlueheJKQvY3kF7D8dPcvw== X-Received: by 2002:a05:6402:1504:b0:499:70a8:f91a with SMTP id f4-20020a056402150400b0049970a8f91amr5501371edw.19.1675332645239; Thu, 02 Feb 2023 02:10:45 -0800 (PST) Received: from dtpc.zanders.be (78-22-137-109.access.telenet.be. [78.22.137.109]) by smtp.gmail.com with ESMTPSA id t13-20020a50d70d000000b00458b41d9460sm11155816edi.92.2023.02.02.02.10.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 02:10:44 -0800 (PST) From: Maarten Zanders <maarten.zanders@mind.be> To: Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Jacek Anaszewski <jacek.anaszewski@gmail.com> Cc: Maarten Zanders <maarten.zanders@mind.be>, linux-leds@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode Date: Thu, 2 Feb 2023 11:10:31 +0100 Message-Id: <20230202101032.26737-2-maarten.zanders@mind.be> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20230202101032.26737-1-maarten.zanders@mind.be> References: <20230202101032.26737-1-maarten.zanders@mind.be> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756714089635759686?= X-GMAIL-MSGID: =?utf-8?q?1756714089635759686?= |
Series |
leds: lp55xx: configure internal charge pump
|
|
Commit Message
Maarten Zanders
Feb. 2, 2023, 10:10 a.m. UTC
Add a binding to configure the internal charge pump for lp55xx.
Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
Notes:
v1: implement as bool to disable charge pump
v2: rewrite to use string configuration, supporting all modes
v3: simplification by replacing string option by u8 constant,
removing previous Reviewed-by tags as it's a complete
rewrite of the patch.
v4: added notes
.../devicetree/bindings/leds/leds-lp55xx.yaml | 8 ++++++++
include/dt-bindings/leds/leds-lp55xx.h | 10 ++++++++++
2 files changed, 18 insertions(+)
create mode 100644 include/dt-bindings/leds/leds-lp55xx.h
Comments
On 02/02/2023 11:10, Maarten Zanders wrote: > Add a binding to configure the internal charge pump for lp55xx. > > Signed-off-by: Maarten Zanders <maarten.zanders@mind.be> > --- > > Notes: > v1: implement as bool to disable charge pump > v2: rewrite to use string configuration, supporting all modes > v3: simplification by replacing string option by u8 constant, > removing previous Reviewed-by tags as it's a complete > rewrite of the patch. > v4: added notes > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 8 ++++++++ > include/dt-bindings/leds/leds-lp55xx.h | 10 ++++++++++ > 2 files changed, 18 insertions(+) > create mode 100644 include/dt-bindings/leds/leds-lp55xx.h > > diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > index ae607911f1db..22e63d89d770 100644 > --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > @@ -66,6 +66,12 @@ properties: > '#size-cells': > const: 0 > > + ti,charge-pump-mode: > + description: > + Set the operating mode of the internal charge pump as defined in > + <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto. > + $ref: /schemas/types.yaml#/definitions/uint8 > + > patternProperties: > '^multi-led@[0-8]$': > type: object > @@ -152,6 +158,7 @@ additionalProperties: false > examples: > - | > #include <dt-bindings/leds/common.h> > + #include <dt-bindings/leds/leds-lp55xx.h> > > i2c { > #address-cells = <1>; > @@ -164,6 +171,7 @@ examples: > reg = <0x32>; > clock-mode = /bits/ 8 <2>; > pwr-sel = /bits/ 8 <3>; /* D1~9 connected to VOUT */ > + ti,charge-pump-mode = /bits/ 8 <LP55XX_CP_BYPASS>; No. V2 was correct. What happened here? You got review for v2, but suddenly entire patch goes into other direction... Best regards, Krzysztof
On 02/02/2023 11:10, Maarten Zanders wrote: > Add a binding to configure the internal charge pump for lp55xx. > > Signed-off-by: Maarten Zanders <maarten.zanders@mind.be> > diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h > new file mode 100644 > index 000000000000..8f59c1c12dee > --- /dev/null > +++ b/include/dt-bindings/leds/leds-lp55xx.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _DT_BINDINGS_LEDS_LP55XX_H > +#define _DT_BINDINGS_LEDS_LP55XX_H > + > +#define LP55XX_CP_OFF 0 > +#define LP55XX_CP_BYPASS 1 > +#define LP55XX_CP_BOOST 2 > +#define LP55XX_CP_AUTO 3 Additionally, these are not used, so it's a dead binding. Drop. Sorry, this is not the approach you should take. Best regards, Krzysztof
First off, bear with me here, I only recently started upstreaming patches to kernel. It still feels like navigating an extremely busy shipping lane... Either way, your feedback is highly valued. On 2/2/23 14:15, Krzysztof Kozlowski wrote: > >> diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h >> new file mode 100644 >> index 000000000000..8f59c1c12dee >> --- /dev/null >> +++ b/include/dt-bindings/leds/leds-lp55xx.h >> @@ -0,0 +1,10 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _DT_BINDINGS_LEDS_LP55XX_H >> +#define _DT_BINDINGS_LEDS_LP55XX_H >> + >> +#define LP55XX_CP_OFF 0 >> +#define LP55XX_CP_BYPASS 1 >> +#define LP55XX_CP_BOOST 2 >> +#define LP55XX_CP_AUTO 3 > Additionally, these are not used, so it's a dead binding. Drop. Sorry, > this is not the approach you should take. > > Best regards, > Krzysztof > These definitions are intended to be used in the DTS's, so it seems normal to me that most of them go unused in the code? What am I missing? As for the changes v2 > v3, this was based on input I got on v2 from Lee Jones, maintainer for leds, on the implementation of the parsing of this option: >> + pdata->charge_pump_mode = LP55XX_CP_AUTO; >> + ret = of_property_read_string(np, "ti,charge-pump-mode", &pm); >> + if (!ret) { >> + for (cp_mode = LP55XX_CP_OFF; >> + cp_mode < ARRAY_SIZE(charge_pump_modes); >> + cp_mode++) { >> + if (!strcasecmp(pm, charge_pump_modes[cp_mode])) { >> + pdata->charge_pump_mode = cp_mode; >> + break; >> + } >> + } >> + } > A little over-engineered, no? > > Why not make the property a numerical value, then simply: > > ret = of_property_read_u32(np, "ti,charge-pump-mode", &pdata->charge_pump_mode); > if (ret) > data->charge_pump_mode = LP55XX_CP_AUTO; I found examples of similar configuration options of both types with other drivers in the kernel tree (ie string & uint8). I can appreciate both viewpoints but unfortunately cannot comply with both. Best regards, Maarten
On 02/02/2023 14:35, Maarten Zanders wrote: > First off, bear with me here, I only recently started upstreaming > patches to kernel. It still feels like navigating an extremely busy > shipping lane... Either way, your feedback is highly valued. > > On 2/2/23 14:15, Krzysztof Kozlowski wrote: >> >>> diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h >>> new file mode 100644 >>> index 000000000000..8f59c1c12dee >>> --- /dev/null >>> +++ b/include/dt-bindings/leds/leds-lp55xx.h >>> @@ -0,0 +1,10 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +#ifndef _DT_BINDINGS_LEDS_LP55XX_H >>> +#define _DT_BINDINGS_LEDS_LP55XX_H >>> + >>> +#define LP55XX_CP_OFF 0 >>> +#define LP55XX_CP_BYPASS 1 >>> +#define LP55XX_CP_BOOST 2 >>> +#define LP55XX_CP_AUTO 3 >> Additionally, these are not used, so it's a dead binding. Drop. Sorry, >> this is not the approach you should take. >> >> Best regards, >> Krzysztof >> > These definitions are intended to be used in the DTS's, so it seems > normal to me that most of them go unused in the code? What am I missing? Bindings mean drivers are using them. Your driver is not using it. It's a register value, isn't it? Register values are not suitable for bindings. There is no need for them to be in bindings. > > As for the changes v2 > v3, this was based on input I got on v2 from Lee > Jones, maintainer for leds, on the implementation of the parsing of this > option: > >>> + pdata->charge_pump_mode = LP55XX_CP_AUTO; >>> + ret = of_property_read_string(np, "ti,charge-pump-mode", &pm); >>> + if (!ret) { >>> + for (cp_mode = LP55XX_CP_OFF; >>> + cp_mode < ARRAY_SIZE(charge_pump_modes); >>> + cp_mode++) { >>> + if (!strcasecmp(pm, charge_pump_modes[cp_mode])) { >>> + pdata->charge_pump_mode = cp_mode; >>> + break; >>> + } >>> + } >>> + } >> A little over-engineered, no? >> >> Why not make the property a numerical value, then simply: >> >> ret = of_property_read_u32(np, "ti,charge-pump-mode", &pdata->charge_pump_mode); >> if (ret) >> data->charge_pump_mode = LP55XX_CP_AUTO; > > I found examples of similar configuration options of both types with > other drivers in the kernel tree (ie string & uint8). I can appreciate > both viewpoints but unfortunately cannot comply with both. Strings in DTS are usually easier to for humans to read, but it's not a requirement to use them. The problem of storing register values is that binding is tied/coupled with hardware programming model, so you cannot add a new device if the register value is a bit different (e.g. LP55XX_CP_OFF is 0x1). You need entire new binding for such case. With string - no need. With binding constants (IDs) also no need, so was this the intention? Just to be clear - it is then ID or binding constant, not a value for hardware register. Best regards, Krzysztof
On 2/2/23 14:43, Krzysztof Kozlowski wrote: > > Strings in DTS are usually easier to for humans to read, but it's not a > requirement to use them. The problem of storing register values is that > binding is tied/coupled with hardware programming model, so you cannot > add a new device if the register value is a bit different (e.g. > LP55XX_CP_OFF is 0x1). You need entire new binding for such case. With > string - no need. I understand and this is why I started with the string in the first place (as suggested by yourself in V1). > With binding constants (IDs) also no need, so was this > the intention? Just to be clear - it is then ID or binding constant, not > a value for hardware register. > For simplicity sake, yes, now the setting is propagating directly into the register as a bit value. But this is how the current implementation of the drivers work. If we add a device in the future which indeed has different bit mappings, that driver will have to do a mapping of the DT binding to its own bit field definitions. I consider this DT binding as the "master", which is now conveniently chosen to match the register values. Cheers, Maarten
On 02/02/2023 15:12, Maarten Zanders wrote: > > On 2/2/23 14:43, Krzysztof Kozlowski wrote: >> >> Strings in DTS are usually easier to for humans to read, but it's not a >> requirement to use them. The problem of storing register values is that >> binding is tied/coupled with hardware programming model, so you cannot >> add a new device if the register value is a bit different (e.g. >> LP55XX_CP_OFF is 0x1). You need entire new binding for such case. With >> string - no need. > I understand and this is why I started with the string in the first > place (as suggested by yourself in V1). >> With binding constants (IDs) also no need, so was this >> the intention? Just to be clear - it is then ID or binding constant, not >> a value for hardware register. >> > For simplicity sake, yes, now the setting is propagating directly into > the register as a bit value. But this is how the current implementation > of the drivers work. If we add a device in the future which indeed has > different bit mappings, that driver will have to do a mapping of the DT > binding to its own bit field definitions. I consider this DT binding as > the "master", which is now conveniently chosen to match the register values. OK, that makes sense. Best regards, Krzysztof
On 02/02/2023 11:10, Maarten Zanders wrote: > Add a binding to configure the internal charge pump for lp55xx. > > Signed-off-by: Maarten Zanders <maarten.zanders@mind.be> > --- > > Notes: > v1: implement as bool to disable charge pump > v2: rewrite to use string configuration, supporting all modes > v3: simplification by replacing string option by u8 constant, > removing previous Reviewed-by tags as it's a complete > rewrite of the patch. > v4: added notes > > .../devicetree/bindings/leds/leds-lp55xx.yaml | 8 ++++++++ > include/dt-bindings/leds/leds-lp55xx.h | 10 ++++++++++ > 2 files changed, 18 insertions(+) > create mode 100644 include/dt-bindings/leds/leds-lp55xx.h > > diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > index ae607911f1db..22e63d89d770 100644 > --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml > @@ -66,6 +66,12 @@ properties: > '#size-cells': > const: 0 > > + ti,charge-pump-mode: > + description: > + Set the operating mode of the internal charge pump as defined in > + <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto. > + $ref: /schemas/types.yaml#/definitions/uint8 This should be then uint32 default: 3 (and drop last sentence about default) > + > patternProperties: > '^multi-led@[0-8]$': > type: object > @@ -152,6 +158,7 @@ additionalProperties: false > examples: > - | > #include <dt-bindings/leds/common.h> > + #include <dt-bindings/leds/leds-lp55xx.h> > > i2c { > #address-cells = <1>; > @@ -164,6 +171,7 @@ examples: > reg = <0x32>; > clock-mode = /bits/ 8 <2>; > pwr-sel = /bits/ 8 <3>; /* D1~9 connected to VOUT */ > + ti,charge-pump-mode = /bits/ 8 <LP55XX_CP_BYPASS>; > > led@0 { > reg = <0>; > diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h > new file mode 100644 > index 000000000000..8f59c1c12dee > --- /dev/null > +++ b/include/dt-bindings/leds/leds-lp55xx.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ Dual license. > +#ifndef _DT_BINDINGS_LEDS_LP55XX_H > +#define _DT_BINDINGS_LEDS_LP55XX_H > + > +#define LP55XX_CP_OFF 0 > +#define LP55XX_CP_BYPASS 1 > +#define LP55XX_CP_BOOST 2 > +#define LP55XX_CP_AUTO 3 > + > +#endif /* _DT_BINDINGS_LEDS_LP55XX_H */ Best regards, Krzysztof
On 2/2/23 21:13, Krzysztof Kozlowski wrote: > + ti,charge-pump-mode: >> + description: >> + Set the operating mode of the internal charge pump as defined in >> + <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto. >> + $ref: /schemas/types.yaml#/definitions/uint8 > This should be then uint32 Why is that? I specifically chose uint8 because other settings for LED are also uint8. The implementation is also uint8. I surely hope we'll never get to >256 modes for a charge pump. > default: 3 > (and drop last sentence about default) OK > +/* SPDX-License-Identifier: GPL-2.0 */ > Dual license. OK Best regards, Maarten
On 03/02/2023 16:38, Maarten Zanders wrote: > > On 2/2/23 21:13, Krzysztof Kozlowski wrote: >> + ti,charge-pump-mode: >>> + description: >>> + Set the operating mode of the internal charge pump as defined in >>> + <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto. >>> + $ref: /schemas/types.yaml#/definitions/uint8 >> This should be then uint32 > > Why is that? I specifically chose uint8 because other settings for LED > are also uint8. The implementation is also uint8. I surely hope we'll > never get to >256 modes for a charge pump. Because all IDs are unsigned int. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml index ae607911f1db..22e63d89d770 100644 --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml @@ -66,6 +66,12 @@ properties: '#size-cells': const: 0 + ti,charge-pump-mode: + description: + Set the operating mode of the internal charge pump as defined in + <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto. + $ref: /schemas/types.yaml#/definitions/uint8 + patternProperties: '^multi-led@[0-8]$': type: object @@ -152,6 +158,7 @@ additionalProperties: false examples: - | #include <dt-bindings/leds/common.h> + #include <dt-bindings/leds/leds-lp55xx.h> i2c { #address-cells = <1>; @@ -164,6 +171,7 @@ examples: reg = <0x32>; clock-mode = /bits/ 8 <2>; pwr-sel = /bits/ 8 <3>; /* D1~9 connected to VOUT */ + ti,charge-pump-mode = /bits/ 8 <LP55XX_CP_BYPASS>; led@0 { reg = <0>; diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h new file mode 100644 index 000000000000..8f59c1c12dee --- /dev/null +++ b/include/dt-bindings/leds/leds-lp55xx.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _DT_BINDINGS_LEDS_LP55XX_H +#define _DT_BINDINGS_LEDS_LP55XX_H + +#define LP55XX_CP_OFF 0 +#define LP55XX_CP_BYPASS 1 +#define LP55XX_CP_BOOST 2 +#define LP55XX_CP_AUTO 3 + +#endif /* _DT_BINDINGS_LEDS_LP55XX_H */