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;
Message-ID: <ce89ecb8-3505-6aff-7aec-f7f1cf133656@linaro.org>
Date:   Wed, 31 May 2023 21:52:09 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] dt-bindings: power: supply: Fix rt9467 charger enable
 gpio active level
Content-Language: en-US
To:     ChiYuan Huang <cy_huang@richtek.com>
Cc:     sre@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        chiaen_wu@richtek.com, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1685522813-14481-1-git-send-email-cy_huang@richtek.com>
 <1f966e93-50c3-142e-620c-8fbb16e9b1a7@linaro.org>
 <20230531093618.GA17005@linuxcarl2.richtek.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20230531093618.GA17005@linuxcarl2.richtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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