From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CCA2CC25B0F for ; Fri, 12 Aug 2022 06:54:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235319AbiHLGyU (ORCPT ); Fri, 12 Aug 2022 02:54:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54466 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234733AbiHLGyT (ORCPT ); Fri, 12 Aug 2022 02:54:19 -0400 Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93B48A598F for ; Thu, 11 Aug 2022 23:54:17 -0700 (PDT) Received: by mail-lf1-x12d.google.com with SMTP id f20so122160lfc.10 for ; Thu, 11 Aug 2022 23:54:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc; bh=t3EERWAoHc/0sJzUnWIG/VXJqbeUHXe35Iq8kCfIwJ8=; b=H0Rll6qOqs0V2BmA7/rXq8aEvL568CpvV8lKQ3Rn5KRJjcfJsvnHeU6JN6ie3e+jQb KqVSjL28MvNTNAeD+f9a7p92RM33993ChXnC30O2V4iPLa4A0OpxfXLo6VnZD5mVX8j9 /mMs1KTE0M5xc5WVU0k7fH5YQbXIq4fBoNpYtztfLhZSob7Gk0NpSsvY9YNaagKTiGwM I9EcusoqsELsaNje/phNRiMzW2n/gsxSKDzrAhAbT3hRUa8n5dP4+yPB2DKjvPF9yX6S VR67nq/yClD4vERkuYuOmMD9YpDMf1R7iQqS3gt/3m2V+g6B3KzEgh87FPpqJAw6wbYc Hzvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=t3EERWAoHc/0sJzUnWIG/VXJqbeUHXe35Iq8kCfIwJ8=; b=AWLMV5SySqiFmU/58lg6tvyPpo21cLmhxvE/Pzg31iBbve2neGL4qPl0MqDJA4/8ud mp7f5hPwiWX33D8kMgLnT1g5UFTmAt2YMxjeC/Taj0BRk6VJazIXPQyA8GlekNTQfdmz qgjFzZHYw1XhnjyLrA8MThqrEA/z3thaoH05MhwQS2B4NgLoENaEeOnntyOdfGlcBwJk BtUwoR/HBUvZZN0y0cBsmuAoz2cFGHmgjKHvOLz0EKlm38W6z6wDf0F8AM8hjUhDXUZn l746T1bD4CJGIMFq8J6M7gkw9oz1HsqKAZVFAVBR2sPk2tnoJzR2RquTpn67trHiGrxD J/TQ== X-Gm-Message-State: ACgBeo10A3hnvpZ3UGp6WMe9UFJ/coIblbUMK3Cp30b/p5CG6qvBumdV yiqjT0FXxv3zYGhHVqI6YHTX2y5cXFAe4PSu X-Google-Smtp-Source: AA6agR6TWxarEea/EB4pqn9N8km5hXjJnqltHNgS5yLpYpMOPAOSyNuQ9fhHFdZxk7o0iINjK3m8vw== X-Received: by 2002:a05:6512:ac3:b0:48a:fa85:7b20 with SMTP id n3-20020a0565120ac300b0048afa857b20mr801551lfu.340.1660287255892; Thu, 11 Aug 2022 23:54:15 -0700 (PDT) Received: from [192.168.1.39] ([83.146.140.105]) by smtp.gmail.com with ESMTPSA id q30-20020ac2511e000000b0048dacaa8c36sm107895lfb.149.2022.08.11.23.54.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Aug 2022 23:54:15 -0700 (PDT) Message-ID: <40261b95-637a-1304-2e06-8c8ff7fc377b@linaro.org> Date: Fri, 12 Aug 2022 09:54:11 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH 1/2] dt-bindings: power: supply: Add Richtek RT9471 battery charger Content-Language: en-US To: ChiYuan Huang Cc: Rob Herring , Krzysztof Kozlowski , Sebastian Reichel , =?UTF-8?B?5ri45a2Q6aao?= , cy_huang , alinayu829@gmail.com, Linux PM , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , lkml References: <1660225318-4063-1-git-send-email-u0084500@gmail.com> <1660225318-4063-2-git-send-email-u0084500@gmail.com> <3cae9d60-4012-1dfd-abd9-4d0b9379e6bb@linaro.org> From: Krzysztof Kozlowski In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 12/08/2022 04:32, ChiYuan Huang wrote: > Krzysztof Kozlowski 於 2022年8月11日 週四 晚上10:12寫道： >> >> On 11/08/2022 16:41, cy_huang wrote: >>> From: ChiYuan Huang >>> >>> Add bindings for the Richtek RT9471 I2C controlled battery charger. >>> >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +properties: >>> + compatible: >>> + const: richtek,rt9471 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + ceb-gpios: >>> + maxItems: 1 >> >> This looks not standard, so please provide a description. > It's the external 'charge enable' pin that's used to control battery charging. > The priority is higher than the register 'CHG_EN' control. > In the word, 'b' means it's reverse logic, low to allow charging, high > to force disable charging. Isn't this standard enable-gpios property? > > description: > External charge enable pin that can force control not to charge the battery. > Low to allow charging, high to disable charging. > >> >>> + >>> + wakeup-source: true >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + interrupt-controller: true >>> + >>> + "#interrupt-cells": >>> + const: 1 >> >> Why a charger driver is a interrupt-controller? > There're 32 nested IRQs from RT9471. > The original thought is to make the user easy to bind the interrupt > into their driver. Bindings are not related to the driver but to hardware... > > For charger driver, does it mean legacy IRQ handler is more preferred? Who is the consumer of these interrupts? Can you show the DTS with the interrupt consumer? >> >>> + >>> + usb-otg-vbus-regulator: >>> + type: object >>> + unevaluatedProperties: false >>> + $ref: /schemas/regulator/regulator.yaml# >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - wakeup-source >>> + - interrupts >>> + - interrupt-controller >>> + - "#interrupt-cells" >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include >>> + i2c { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + charger@53 { >>> + compatible = "richtek,rt9471"; >>> + reg = <0x53>; >>> + ceb-gpios = <&gpio26 1 0>; >> >> Isn't the last value a GPIO flag? If yes, use appropriate define. > I already specify GPIOD_OUT_LOW in the gpiod_request flag. It is not related to the DTS. Anyway writing "low" for a meaning of high is not correct usually... > Do I need to convert the gpio request code to GPIOD_OUT_HIGH, > and specify here as GPIO_ACTIVE_LOW? You need to properly describe the hardware. The polarity of logical signal is defined by DTS, not by driver. It does not make sense to do it in driver. What if on some board the signal is inverted? Best regards, Krzysztof