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=-11.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 369DCC55179 for ; Thu, 5 Nov 2020 16:07:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B744820A8B for ; Thu, 5 Nov 2020 16:07:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730721AbgKEQHp (ORCPT ); Thu, 5 Nov 2020 11:07:45 -0500 Received: from netrider.rowland.org ([192.131.102.5]:52091 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1729862AbgKEQHp (ORCPT ); Thu, 5 Nov 2020 11:07:45 -0500 Received: (qmail 1615519 invoked by uid 1000); 5 Nov 2020 11:07:43 -0500 Date: Thu, 5 Nov 2020 11:07:43 -0500 From: Alan Stern To: Dmitry Osipenko Cc: Thierry Reding , Jonathan Hunter , Peter Chen , Mark Brown , Liam Girdwood , Adrian Hunter , Krzysztof Kozlowski , Greg Kroah-Hartman , Lee Jones , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ulf Hansson , Mauro Carvalho Chehab , Rob Herring , Marek Szyprowski , Peter Geis , Nicolas Chauvet , linux-samsung-soc@vger.kernel.org, devel@driverdev.osuosl.org, linux-usb@vger.kernel.org, linux-pwm@vger.kernel.org, linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linux-tegra@vger.kernel.org Subject: Re: [PATCH v1 21/30] usb: host: ehci-tegra: Support OPP and SoC core voltage scaling Message-ID: <20201105160743.GA1613614@rowland.harvard.edu> References: <20201104234427.26477-1-digetx@gmail.com> <20201104234427.26477-22-digetx@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201104234427.26477-22-digetx@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Thu, Nov 05, 2020 at 02:44:18AM +0300, Dmitry Osipenko wrote: > Add initial OPP and SoC core voltage scaling support to the Tegra EHCI > driver. This is required for enabling system-wide DVFS on older Tegra > SoCs. > > Tested-by: Peter Geis > Tested-by: Nicolas Chauvet > Signed-off-by: Dmitry Osipenko > --- I'm no expert on OPP stuff, but some of what you have done here looks peculiar. > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c > index 869d9c4de5fc..0976577f54b4 100644 > --- a/drivers/usb/host/ehci-tegra.c > +++ b/drivers/usb/host/ehci-tegra.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -364,6 +365,79 @@ static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) > free_dma_aligned_buffer(urb); > } > > +static void tegra_ehci_deinit_opp_table(void *data) > +{ > + struct device *dev = data; > + struct opp_table *opp_table; > + > + opp_table = dev_pm_opp_get_opp_table(dev); > + dev_pm_opp_of_remove_table(dev); > + dev_pm_opp_put_regulators(opp_table); > + dev_pm_opp_put_opp_table(opp_table); > +} > + > +static int devm_tegra_ehci_init_opp_table(struct device *dev) > +{ > + unsigned long rate = ULONG_MAX; > + struct opp_table *opp_table; > + const char *rname = "core"; > + struct dev_pm_opp *opp; > + int err; > + > + /* legacy device-trees don't have OPP table */ > + if (!device_property_present(dev, "operating-points-v2")) > + return 0; > + > + /* voltage scaling is optional */ > + if (device_property_present(dev, "core-supply")) > + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1); > + else > + opp_table = dev_pm_opp_get_opp_table(dev); > + > + if (IS_ERR(opp_table)) > + return dev_err_probe(dev, PTR_ERR(opp_table), > + "failed to prepare OPP table\n"); > + > + err = dev_pm_opp_of_add_table(dev); > + if (err) { > + dev_err(dev, "failed to add OPP table: %d\n", err); > + goto put_table; > + } > + > + /* find suitable OPP for the maximum clock rate */ > + opp = dev_pm_opp_find_freq_floor(dev, &rate); > + err = PTR_ERR_OR_ZERO(opp); > + if (err) { > + dev_err(dev, "failed to get OPP: %d\n", err); > + goto remove_table; > + } > + > + dev_pm_opp_put(opp); > + > + /* > + * First dummy rate-set initializes voltage vote by setting voltage > + * in accordance to the clock rate. > + */ > + err = dev_pm_opp_set_rate(dev, rate); > + if (err) { > + dev_err(dev, "failed to initialize OPP clock: %d\n", err); > + goto remove_table; > + } > + > + err = devm_add_action(dev, tegra_ehci_deinit_opp_table, dev); > + if (err) > + goto remove_table; > + > + return 0; > + > +remove_table: > + dev_pm_opp_of_remove_table(dev); > +put_table: > + dev_pm_opp_put_regulators(opp_table); Do you really want to use the same error unwinding for opp_table values obtained from dev_pm_opp_set_regulators() as from dev_pm_opp_get_opp_table()? > + > + return err; > +} > + > static const struct tegra_ehci_soc_config tegra30_soc_config = { > .has_hostpc = true, > }; > @@ -431,6 +505,11 @@ static int tegra_ehci_probe(struct platform_device *pdev) > goto cleanup_hcd_create; > } > > + err = devm_tegra_ehci_init_opp_table(&pdev->dev); > + if (err) > + return dev_err_probe(&pdev->dev, err, > + "Failed to initialize OPP\n"); Why log a second error message? Just return err. Alan Stern