Message ID | 1685522813-14481-1-git-send-email-cy_huang@richtek.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp2732535vqr; Wed, 31 May 2023 02:02:09 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4c3yzqzJI0l7tu4KT47r8j96ndhgIBTmDGllD01MG3DWaclOsTCuQKxwurjC5OThvmHksQ X-Received: by 2002:a05:6830:1e34:b0:69f:578a:d1ea with SMTP id t20-20020a0568301e3400b0069f578ad1eamr1599538otr.32.1685523729237; Wed, 31 May 2023 02:02:09 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685523729; cv=none; d=google.com; s=arc-20160816; b=VaiEiE0RULuLvGT2rmO9x0x6MZrklm/zeUA9GN2ORJ9BLm81hgZk/mOTejc51v0v8o O5rUGwMqvchMCbYh5dX7AwmlHZKZ8pONK2hqraoI2pnXLkdAR/0tY1SwAPoogz7/7b58 MZvWJPrchRBmJUYEwE7fgS5J4B9qu8ah/2TWapSZXM9pbRSLyZCUBhWUrZIFlHg28vh4 MPPLWBRUqCs3jYxSrkSGJ8eWscID3n1hQGTUOXSxojZMEZFw7RzrC36OGk9xWIDMf7bj 7SIjLmY3wHryWKypbA4Nr254kDAuFtEf5GR6eGzO6/culTaIWNvSGpFNaspjVktFJSJO 4U3w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:subject:cc:to:from; bh=t64Hzu6zM+aWHS7TdPfza23+IuD8Tjx7jNFzWlii980=; b=wFCZcAFfFlVPq5dh8KbX9S9V5hVOzZyPOb7EV7XS38UrYI0K6Y+mD//XDbc1zVLXce A+mnseBmzKfi6E6jz3M7D9U6cDg0Nh2nYEv6stb0j0kjoIk4CrbRhtIFAEFAdLlR/XpD kG0/1qIHpyi5L/j9vFni7tTmyW00CqYkG51ERFr1R7i7LJXIDlm0SSQA2by6Tnv9i7rH a3u/xaSCkkiXo5AkwteKBUt/IVLHAXguFPBDGuDVOpVpGup80RuANfZCR0XSob2DMPWd 68liSHKTCgjwQMYfdFpvS0E8SKut9WKkhaJc/O6HpqQVfFzdIIhr6NsWp1FEJBSZOAEx kAHg== ARC-Authentication-Results: i=1; mx.google.com; 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 q26-20020aa7843a000000b0064d35b6cdd8si1130821pfn.168.2023.05.31.02.01.56; Wed, 31 May 2023 02:02:09 -0700 (PDT) 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; 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 S235201AbjEaIrm (ORCPT <rfc822;callmefire3@gmail.com> + 99 others); Wed, 31 May 2023 04:47:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33388 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234424AbjEaIrT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 31 May 2023 04:47:19 -0400 Received: from mg.richtek.com (mg.richtek.com [220.130.44.152]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id AB78611C; Wed, 31 May 2023 01:47:08 -0700 (PDT) X-MailGates: (flag:4,DYNAMIC,BADHELO,RELAY,NOHOST:PASS)(compute_score:DE LIVER,40,3) Received: from 192.168.10.47 by mg.richtek.com with MailGates ESMTP Server V5.0(746:0:AUTH_RELAY) (envelope-from <cy_huang@richtek.com>); Wed, 31 May 2023 16:46:54 +0800 (CST) Received: from ex4.rt.l (192.168.10.47) by ex4.rt.l (192.168.10.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.25; Wed, 31 May 2023 16:46:54 +0800 Received: from linuxcarl2.richtek.com (192.168.10.154) by ex4.rt.l (192.168.10.45) with Microsoft SMTP Server id 15.2.1118.25 via Frontend Transport; Wed, 31 May 2023 16:46:54 +0800 From: <cy_huang@richtek.com> To: <sre@kernel.org>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org> CC: <cy_huang@richtek.com>, <chiaen_wu@richtek.com>, <linux-pm@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH] dt-bindings: power: supply: Fix rt9467 charger enable gpio active level Date: Wed, 31 May 2023 16:46:53 +0800 Message-ID: <1685522813-14481-1-git-send-email-cy_huang@richtek.com> X-Mailer: git-send-email 1.8.3.1 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham 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?1767399729676189625?= X-GMAIL-MSGID: =?utf-8?q?1767399729676189625?= |
Series |
dt-bindings: power: supply: Fix rt9467 charger enable gpio active level
|
|
Commit Message
ChiYuan Huang
May 31, 2023, 8:46 a.m. UTC
From: ChiYuan Huang <cy_huang@richtek.com> The RT9467 charger enable pin is an external signal that used to enable battery charging. From the datasheet, the active level is low. Although it's already configured to logic low at driver probe function, but the current binding example declared it as 'GPIO_ACTIVE_LOW', this causes this pin be output high and disable battery charging. Fixes: e1b4620fb503 ("dt-bindings: power: supply: Add Richtek RT9467 battery charger") Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> --- Hi, This patch is to fix the active level for charger enable gpio polarity. Currently, the wrong active level makes the user confused and unexpectedly disable battery charging by default. --- Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 31/05/2023 10:46, cy_huang@richtek.com wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > The RT9467 charger enable pin is an external signal that used to enable > battery charging. From the datasheet, the active level is low. Although > it's already configured to logic low at driver probe function, but the NAK. You mix two different things. Driver behavior and DTS. Driver can operate either on real level - matching hardware - or on logical level (high as enable, low as disable). First choice is usually wrong, because it does not allow inverted signals. 'Correcting' bindings to wrong approach is wrong. If the signal is active low, then the flag is active low. Simple as that. > current binding example declared it as 'GPIO_ACTIVE_LOW', this causes > this pin be output high and disable battery charging. > > Fixes: e1b4620fb503 ("dt-bindings: power: supply: Add Richtek RT9467 battery charger") > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > Hi, > > This patch is to fix the active level for charger enable gpio polarity. This is just example - it does not fix anything... > Currently, the wrong active level makes the user confused and > unexpectedly disable battery charging by default. > --- > Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > index 3723717..cdc7678 100644 > --- a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > @@ -69,7 +69,7 @@ examples: > reg = <0x5b>; > wakeup-source; > interrupts-extended = <&gpio_intc 32 IRQ_TYPE_LEVEL_LOW>; > - charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_LOW>; > + charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_HIGH>; > > rt9467_otg_vbus: usb-otg-vbus-regulator { > regulator-name = "rt9467-usb-otg-vbus"; Best regards, Krzysztof
On Wed, May 31, 2023 at 11:17:37AM +0200, Krzysztof Kozlowski wrote: > On 31/05/2023 10:46, cy_huang@richtek.com wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > The RT9467 charger enable pin is an external signal that used to enable > > battery charging. From the datasheet, the active level is low. Although > > it's already configured to logic low at driver probe function, but the > > NAK. > > You mix two different things. Driver behavior and DTS. Driver can > operate either on real level - matching hardware - or on logical level > (high as enable, low as disable). First choice is usually wrong, because > it does not allow inverted signals. > > 'Correcting' bindings to wrong approach is wrong. If the signal is > active low, then the flag is active low. Simple as that. > If my understanding is right, so the correct way is to fix the driver code, not binding exmaple. > > current binding example declared it as 'GPIO_ACTIVE_LOW', this causes > > this pin be output high and disable battery charging. > > > > Fixes: e1b4620fb503 ("dt-bindings: power: supply: Add Richtek RT9467 battery charger") > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > --- > > Hi, > > > > This patch is to fix the active level for charger enable gpio polarity. > > This is just example - it does not fix anything... > Sorry, this issue comes from the customer. They directly copy the example into their platform dts. That's why originally I think it may be a fix. Anyway, you're right. To maintain the maximum flexibility, the choice shouldn't be to fix the example. It has to correct the driver code for this pin behavior. Thanks. > > Currently, the wrong active level makes the user confused and > > unexpectedly disable battery charging by default. > > --- > > Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > > index 3723717..cdc7678 100644 > > --- a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > > +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml > > @@ -69,7 +69,7 @@ examples: > > reg = <0x5b>; > > wakeup-source; > > interrupts-extended = <&gpio_intc 32 IRQ_TYPE_LEVEL_LOW>; > > - charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_LOW>; > > + charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_HIGH>; > > > > rt9467_otg_vbus: usb-otg-vbus-regulator { > > regulator-name = "rt9467-usb-otg-vbus"; > > Best regards, > Krzysztof >
On 31/05/2023 11:36, ChiYuan Huang wrote: > On Wed, May 31, 2023 at 11:17:37AM +0200, Krzysztof Kozlowski wrote: >> On 31/05/2023 10:46, cy_huang@richtek.com wrote: >>> From: ChiYuan Huang <cy_huang@richtek.com> >>> >>> The RT9467 charger enable pin is an external signal that used to enable >>> battery charging. From the datasheet, the active level is low. Although >>> it's already configured to logic low at driver probe function, but the >> >> NAK. >> >> You mix two different things. Driver behavior and DTS. Driver can >> operate either on real level - matching hardware - or on logical level >> (high as enable, low as disable). First choice is usually wrong, because >> it does not allow inverted signals. >> >> 'Correcting' bindings to wrong approach is wrong. If the signal is >> active low, then the flag is active low. Simple as that. >> > If my understanding is right, so the correct way is to fix the driver code, > not binding exmaple. Yes, that's my opinion. The DTS should describe the hardware and in the hardware the pin is active low. Now the driver should operate on logical state, so it want to enable (value of 1) or disable something (value of 0). Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml index 3723717..cdc7678 100644 --- a/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt9467.yaml @@ -69,7 +69,7 @@ examples: reg = <0x5b>; wakeup-source; interrupts-extended = <&gpio_intc 32 IRQ_TYPE_LEVEL_LOW>; - charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_LOW>; + charge-enable-gpios = <&gpio26 1 GPIO_ACTIVE_HIGH>; rt9467_otg_vbus: usb-otg-vbus-regulator { regulator-name = "rt9467-usb-otg-vbus";