Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754084AbdCOOfE (ORCPT ); Wed, 15 Mar 2017 10:35:04 -0400 Received: from netrider.rowland.org ([192.131.102.5]:52969 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751787AbdCOOfD (ORCPT ); Wed, 15 Mar 2017 10:35:03 -0400 Date: Wed, 15 Mar 2017 10:35:01 -0400 (EDT) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: Philipp Zabel cc: linux-usb@vger.kernel.org, Patrice Chotard , Greg Kroah-Hartman , Subject: Re: [PATCH v2 13/14] usb: host: ehci-st: simplify optional reset handling In-Reply-To: <20170315113155.10242-1-p.zabel@pengutronix.de> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1758 Lines: 54 On Wed, 15 Mar 2017, Philipp Zabel wrote: > As of commit bb475230b8e5 ("reset: make optional functions really > optional"), the reset framework API calls use NULL pointers to describe > optional, non-present reset controls. What does it use to describe genuine errors? > This allows to return errors from devm_reset_control_get_optional_shared > unconditionally. > > Signed-off-by: Philipp Zabel > --- > drivers/usb/host/ehci-st.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c > index be4a2788fc582..12e803d2c98df 100644 > --- a/drivers/usb/host/ehci-st.c > +++ b/drivers/usb/host/ehci-st.c > @@ -210,18 +210,14 @@ static int st_ehci_platform_probe(struct platform_device *dev) > devm_reset_control_get_optional_shared(&dev->dev, "power"); > if (IS_ERR(priv->pwr)) { > err = PTR_ERR(priv->pwr); > - if (err == -EPROBE_DEFER) > - goto err_put_clks; > - priv->pwr = NULL; > + goto err_put_clks; > } > > priv->rst = > devm_reset_control_get_optional_shared(&dev->dev, "softreset"); > if (IS_ERR(priv->rst)) { > err = PTR_ERR(priv->rst); > - if (err == -EPROBE_DEFER) > - goto err_put_clks; > - priv->rst = NULL; > + goto err_put_clks; > } > > if (pdata->power_on) { These changes do not agree with the patch description. If any sort of error besides EPROBE_DEFER occurs then: the old code sets priv->pwr or priv->rst to NULL and continues; the new code returns with an error. The only way the patch could be equivalent to the existing code would be if devm_reset_control_get_optional_shared() returns no errors other than EPROBE_DEFER. But the patch description doesn't say this. Alan Stern