From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751806AbdBOQnd (ORCPT ); Wed, 15 Feb 2017 11:43:33 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:35507 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751745AbdBOQnb (ORCPT ); Wed, 15 Feb 2017 11:43:31 -0500 Message-ID: <1487177007.30202.1.camel@baylibre.com> Subject: Re: [PATCH 10/14] phy: meson8b-usb2: simplify optional reset handling From: Jerome Brunet To: Martin Blumenstingl , Philipp Zabel Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ramiro Oliveira , Kevin Hilman , Carlo Caione , Kishon Vijay Abraham I Date: Wed, 15 Feb 2017 17:43:27 +0100 In-Reply-To: References: <20170130114116.22089-1-p.zabel@pengutronix.de> <20170130114116.22089-10-p.zabel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 (3.20.5-1.fc24) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-02-15 at 00:36 +0100, Martin Blumenstingl wrote: > Hi Philipp, > > sorry for the late reply. > unfortunately my GXBB board (which is supported by the driver below) > is dead. > I CC'ed Jerome Brunet - maybe he can give it a go on one of his > (GXBB) boards. > > On Mon, Jan 30, 2017 at 12:41 PM, Philipp Zabel de> 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. > > > > This allows to return errors from > > devm_reset_control_get_optional_shared > > and to call reset_control_reset unconditionally. > > > > Signed-off-by: Philipp Zabel > Acked-by: Martin Blumenstingl > > (based on reading the code along with the highly appreciated changes > from bb475230b8e5) > > > > > Cc: Martin Blumenstingl > > Cc: Kevin Hilman > > Cc: Carlo Caione > > Cc: Kishon Vijay Abraham I > > --- > >  drivers/phy/phy-meson8b-usb2.c | 12 +++++------- > >  1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/phy/phy-meson8b-usb2.c b/drivers/phy/phy- > > meson8b-usb2.c > > index 33c9f4ba157d1..87168f1fe3af6 100644 > > --- a/drivers/phy/phy-meson8b-usb2.c > > +++ b/drivers/phy/phy-meson8b-usb2.c > > @@ -141,12 +141,10 @@ static int phy_meson8b_usb2_power_on(struct > > phy *phy) > >         struct phy_meson8b_usb2_priv *priv = phy_get_drvdata(phy); > >         int ret; > > > > -       if (!IS_ERR_OR_NULL(priv->reset)) { > > -               ret = reset_control_reset(priv->reset); > > -               if (ret) { > > -                       dev_err(&phy->dev, "Failed to trigger USB > > reset\n"); > > -                       return ret; > > -               } > > +       ret = reset_control_reset(priv->reset); > > +       if (ret) { > > +               dev_err(&phy->dev, "Failed to trigger USB > > reset\n"); > > +               return ret; > >         } > > > >         ret = clk_prepare_enable(priv->clk_usb_general); > > @@ -241,7 +239,7 @@ static int phy_meson8b_usb2_probe(struct > > platform_device *pdev) > >                 return PTR_ERR(priv->clk_usb); > > > >         priv->reset = devm_reset_control_get_optional_shared(&pdev- > > >dev, NULL); > > -       if (PTR_ERR(priv->reset) == -EPROBE_DEFER) > > +       if (PTR_ERR(priv->reset)) This is wrong and will always exit on error. It should be "IS_ERR". Clearly the bug was there before your patch, but since you are changing the faulty line, would you mind using IS_ERR instead ? With this changed: Tested-by: Jerome Brunet > >                 return PTR_ERR(priv->reset); > > > >         priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev- > > >dev.of_node, -1); > > -- > > 2.11.0 > >