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 X-Spam-Level: X-Spam-Status: No, score=-7.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C85DC433E0 for ; Fri, 25 Dec 2020 11:34:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 061B022512 for ; Fri, 25 Dec 2020 11:34:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729263AbgLYLeK (ORCPT ); Fri, 25 Dec 2020 06:34:10 -0500 Received: from mail-40134.protonmail.ch ([185.70.40.134]:23401 "EHLO mail-40134.protonmail.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729254AbgLYLeJ (ORCPT ); Fri, 25 Dec 2020 06:34:09 -0500 Date: Fri, 25 Dec 2020 11:33:21 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail; t=1608896006; bh=AZddA3E2efwA1g+qUEHAnTmghSFKPKy2RpOAJ63m37w=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References:From; b=CulIJ8pTiO+N29SlJ4WYYiLNR/jgDe/D8X0fj6eDY0Mpd9ZD8w5goQnRBthx7a56E bMRJe30Fi6oZbhZ+23cwDQBLeS/2vM//Ys2koHBXTWoC3O4c+ExG7nPJhGbXIEm2gz eietdbr2GeFARhMuxFDn4X9V5v029lPKckzKNGGc= To: Krzysztof Kozlowski From: Timon Baetz Cc: Liam Girdwood , Mark Brown , Rob Herring , MyungJoo Ham , Chanwoo Choi , Lee Jones , Sebastian Reichel , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht, Marek Szyprowski Reply-To: Timon Baetz Subject: Re: [PATCH v4 4/7] power: supply: max8997_charger: Set CHARGER current limit Message-ID: <20201225123300.07c84dd8.timon.baetz@protonmail.com> In-Reply-To: <20201224140038.GA48009@kozik-lap> References: <20201223134221.804943-1-timon.baetz@protonmail.com> <20201223134221.804943-4-timon.baetz@protonmail.com> <20201224095559.GB10937@kozik-lap> <20201224141246.7ad0ffc4.timon.baetz@protonmail.com> <20201224133706.GA22856@kozik-lap> <20201224140038.GA48009@kozik-lap> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Thu, 24 Dec 2020 15:00:38 +0100, Krzysztof Kozlowski wrote: > On Thu, Dec 24, 2020 at 02:37:06PM +0100, Krzysztof Kozlowski wrote: > > On Thu, Dec 24, 2020 at 01:13:02PM +0000, Timon Baetz wrote: =20 > > > On Thu, 24 Dec 2020 10:55:59 +0100, Krzysztof Kozlowski wrote: =20 > > > > > @@ -170,6 +237,28 @@ static int max8997_battery_probe(struct plat= form_device *pdev) > > > > > =09=09return PTR_ERR(charger->battery); > > > > > =09} > > > > > > > > > > +=09charger->reg =3D devm_regulator_get(&pdev->dev, "charger"); = =20 > > > > > > > > Since you do not use get_optional, you will always get a dummy > > > > regulator. In case of error, you should either print it or entirely= fail > > > > the probe. Silently continuing makes it difficult to spot errors. > > > > > > > > Since the driver could operate in case of extcon/regulator error, j= ust > > > > dev_err() so failure will be spotted with dmesg. =20 > > > > > > I will switch to devm_regulator_get_optional() and print an error on > > > failure, thanks. > > > =20 > > > > It will complain on older DTBs because you are introducing incompat= ible > > > > change, but that's expected. Just correct all other in-tree DTS. = =20 > > > > > > The other 2 in-tree DTS don't have CHARGER regulators. Not sure > > > how to correct those. Should I add muic and charger nodes without a > > > charger-supply? It will still complain in that case. =20 > > > > +Cc Marek, > > > > This is why leaving the code as is - devm_regulator_get(), not optional > > - makes sense. Core would provide dummy regulator, so you only have to > > provide MUIC node. > > > > If you change the code to devm_regulator_get_optional(), you need to ad= d > > everything: the charger regulator, the charger node and MUIC node. > > > > For Trats, the configuration should be similar as i9100, although I > > don't know the exact values of chargign voltage. > > > > For Origen, there is no battery, so the power supply should not bind. > > Maybe this could be achieved with "status disabled" for charger node? I= t > > depends whether MFD will respect such field... If it disables the > > charger, you're done. =20 >=20 > I just looked at the MFD code and tested it - it nicely skips disabled > devices. Therefore, for Origen I propose to add disabled nodes for > charger and MUIC because these pins are not connected. No need to add > regulators in such case. With a dummy regulator regulator_set_current_limit() fails with -EINVAL. Isn't it better to just skip charging control (and dev_info()) when there= =20 is no extcon or regulator? The charger driver would still probe without those 2 properties and work as before. Adding disabled nodes for Origen would probably still makes sense. I also noticed that adding nodes for those MFD cells prints "DMA mask not set" which seems to be related to https://lkml.org/lkml/2020/4/23/873. Any suggestions on how to handle that? Thanks, Timon