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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 55E20C54E58 for ; Wed, 20 Mar 2024 10:17:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZNZM1P7G9J1o9uZ1HglfM6mjFcQWOA8VFl14tjKIkB4=; b=MQ3wM4Y5Hlsn7E 3cO2hVIdjH4nUlhSoA+7TxIpZq9lcuTF3i+kDGiUQwfZdEXBWXV2h1H5ZLTJ/pednexnNDH0IwCFu fjrDzH9J3FsbHBUUXLTeoIw+64hg8HnS08NyWniTuzJImAd/ApZFojkRDv+AC+ImVHcD/gevfh0ah dVhShXKs2g+eAqJNTcKAf18/9y0+rCxCMtTCSpNYDR1vFTmUvpqsaQ2my4OkoWfyoW+6uNWo9hZe1 Oocf7i72dS4fV3VQonvWC4SqI26VtrUrHgQHqsosKruuC8LwUQYTv1J5kMcvte3vxVZSi7lLmzeUV qnpjS9lVYjvYxz231REw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmt03-0000000Ggnn-1uiT; Wed, 20 Mar 2024 10:16:47 +0000 Received: from [118.185.121.105] (helo=esa1.ltts.com) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmszt-0000000Ggch-2YM6 for linux-arm-kernel@lists.infradead.org; Wed, 20 Mar 2024 10:16:45 +0000 IronPort-SDR: Fz53Zk0t0EsU7YNZvzfL2kRSkgKfcKXrLS1QbXWWHY8sp51KZim++lWWbrWyj7amxwc4ssHdYG FVjs7X2yBVtw== Received: from unknown (HELO localhost.localdomain) ([192.168.34.55]) by esa1.ltts.com with ESMTP; 20 Mar 2024 15:46:10 +0530 From: Bhargav Raviprakash To: eblanc@baylibre.com Cc: arnd@arndb.de, bhargav.r@ltts.com, broonie@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org, gregkh@linuxfoundation.org, jpanis@baylibre.com, kristo@kernel.org, krzysztof.kozlowski+dt@linaro.org, lee@kernel.org, lgirdwood@gmail.com, linus.walleij@linaro.org, linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, m.nirmaladevi@ltts.com, nm@ti.com, robh+dt@kernel.org, vigneshr@ti.com Subject: Re: [PATCH v3 09/11] regulator: tps6594-regulator: Add TI TPS65224 PMIC regulators Date: Wed, 20 Mar 2024 15:45:56 +0530 Message-Id: <20240320101556.464137-1-bhargav.r@ltts.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240320_031644_128798_0FF4B044 X-CRM114-Status: GOOD ( 30.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hello, On Thu, 14 Mar 2024 12:28:00 +0100, Esteban Blanc wrote: > On Fri Mar 8, 2024 at 11:34 AM CET, Bhargav Raviprakash wrote: > > From: Nirmala Devi Mal Nadar > > > > Add support for TPS65224 regulators (bucks and LDOs) to TPS6594 driver as > > they have significant functional overlap. TPS65224 PMIC has 4 buck > > regulators and 3 LDOs. BUCK12 can operate in dual phase. > > The output voltages are configurable and are meant to supply power to the > > main processor and other components. > > > > Signed-off-by: Nirmala Devi Mal Nadar > > Signed-off-by: Bhargav Raviprakash > > --- > > drivers/regulator/Kconfig | 4 +- > > drivers/regulator/tps6594-regulator.c | 236 +++++++++++++++++++++++--- > > 2 files changed, 215 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/regulator/tps6594-regulator.c b/drivers/regulator/tps6594-regulator.c > > index b7f0c8779..37d76c483 100644 > > --- a/drivers/regulator/tps6594-regulator.c > > +++ b/drivers/regulator/tps6594-regulator.c > > @@ -412,14 +562,20 @@ static int tps6594_regulator_probe(struct platform_device *pdev) > > struct tps6594_ext_regulator_irq_data *irq_ext_reg_data; > > struct tps6594_regulator_irq_type *irq_type; > > u8 buck_configured[BUCK_NB] = { 0 }; > > + u8 ldo_configured[LDO_NB] = { 0 }; > > u8 buck_multi[MULTI_PHASE_NB] = { 0 }; > > static const char * const multiphases[] = {"buck12", "buck123", "buck1234", "buck34"}; > > `multiphases` should prefixed like the new one. > Sure! This will be fixed in the next version. > > + static const char * const tps65224_multiphases[] = {"buck12"}; > > > @@ -495,25 +660,30 @@ static int tps6594_regulator_probe(struct platform_device *pdev) > > if (!irq_data) > > return -ENOMEM; > > > > - for (i = 0; i < MULTI_PHASE_NB; i++) { > > + for (i = 0; i < multi_phase_cnt; i++) { > > if (buck_multi[i] == 0) > > continue; > > > > + const struct regulator_desc *multi_regs = (tps->chip_id == TPS65224) ? > > + tps65224_multi_regs : > > + tps6594_multi_regs; > > This should be declared at the top of the function. > > > rdev = devm_regulator_register(&pdev->dev, &multi_regs[i], &config); > > - if (IS_ERR(rdev)) > > - return dev_err_probe(tps->dev, PTR_ERR(rdev), > > - "failed to register %s regulator\n", > > - pdev->name); > > + if (IS_ERR(rdev)) > > + return dev_err_probe(tps->dev, PTR_ERR(rdev), > > + "failed to register %s regulator\n", > > + pdev->name); > > The indentation of the `if` looks odd. You should revert this. > Sure, will fix this in the next version. > > @@ -537,21 +707,34 @@ static int tps6594_regulator_probe(struct platform_device *pdev) > > if (buck_configured[i] == 1) > > continue; > > > > - rdev = devm_regulator_register(&pdev->dev, &buck_regs[i], &config); > > + const struct regulator_desc *buck_cfg = (tps->chip_id == TPS65224) ? > > + tps65224_buck_regs : buck_regs; > > Same here, should be at the top of the function. > > > - /* LP8764 dosen't have LDO */ > > + /* LP8764 doesn't have LDO */ > > if (tps->chip_id != LP8764) { > > - for (i = 0; i < ARRAY_SIZE(ldo_regs); i++) { > > + for (i = 0; i < LDO_NB; i++) { > > + if (ldo_configured[i] == 1) > > + continue; > > + > > + struct tps6594_regulator_irq_type **ldos_irq_types = > > + (tps->chip_id == TPS65224) ? > > + tps65224_ldos_irq_types : tps6594_ldos_irq_types; > > + > > + const struct regulator_desc *ldo_regs = > > + (tps->chip_id == TPS65224) ? > > + tps65224_ldo_regs : tps6594_ldo_regs; > > Should be at the top of the function, please fix this in the whole file. Thanks! In the next version will move declaration to start of the function. > > Best regards, > > -- > Esteban "Skallwar" Blanc > BayLibre Regards, Bhargav _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel