All of lore.kernel.org
 help / color / mirror / Atom feed
From: jbrunet@baylibre.com (Jerome Brunet)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 10/14] phy: meson8b-usb2: simplify optional reset handling
Date: Wed, 15 Feb 2017 17:43:27 +0100	[thread overview]
Message-ID: <1487177007.30202.1.camel@baylibre.com> (raw)
In-Reply-To: <CAFBinCAASGS19naW76vfynRj4Fg4KjpHcwGr5h+pWZae47rwRg@mail.gmail.com>

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 <p.zabel@pengutronix.
> 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 <p.zabel@pengutronix.de>
> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> (based on reading the code along with the highly appreciated changes
> from bb475230b8e5)
> 
> > 
> > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Cc: Carlo Caione <carlo@caione.org>
> > Cc: Kishon Vijay Abraham I <kishon@ti.com>
> > ---
> > ?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 <jbrunet@baylibre.com>

> > ????????????????return PTR_ERR(priv->reset);
> > 
> > ????????priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev-
> > >dev.of_node, -1);
> > --
> > 2.11.0
> > 

WARNING: multiple messages have this Message-ID (diff)
From: Jerome Brunet <jbrunet@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Ramiro Oliveira <Ramiro.Oliveira@synopsys.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Carlo Caione <carlo@caione.org>,
	Kishon Vijay Abraham I <kishon@ti.com>
Subject: Re: [PATCH 10/14] phy: meson8b-usb2: simplify optional reset handling
Date: Wed, 15 Feb 2017 17:43:27 +0100	[thread overview]
Message-ID: <1487177007.30202.1.camel@baylibre.com> (raw)
In-Reply-To: <CAFBinCAASGS19naW76vfynRj4Fg4KjpHcwGr5h+pWZae47rwRg@mail.gmail.com>

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 <p.zabel@pengutronix.
> 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 <p.zabel@pengutronix.de>
> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> (based on reading the code along with the highly appreciated changes
> from bb475230b8e5)
> 
> > 
> > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Cc: Carlo Caione <carlo@caione.org>
> > Cc: Kishon Vijay Abraham I <kishon@ti.com>
> > ---
> >  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 <jbrunet@baylibre.com>

> >                 return PTR_ERR(priv->reset);
> > 
> >         priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev-
> > >dev.of_node, -1);
> > --
> > 2.11.0
> > 

  reply	other threads:[~2017-02-15 16:43 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-30 11:41 [PATCH 01/14] crypto: sun4i-ss - simplify optional reset handling Philipp Zabel
2017-01-30 11:41 ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 02/14] i2c: mv64xxx: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 03/14] [media] coda: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 04/14] [media] st_rc: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 12:33   ` Patrice CHOTARD
2017-01-30 12:33     ` Patrice CHOTARD
2017-01-30 11:41 ` [PATCH 05/14] [media] rc: sunxi-cir: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 06/14] mmc: dw_mmc: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-31 11:52   ` Ulf Hansson
2017-01-31 11:52     ` Ulf Hansson
2017-01-31 17:21     ` Philipp Zabel
2017-01-31 17:21       ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 07/14] mmc: sdhci-st: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 08/14] mmc: sunxi: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-31  3:50   ` Chen-Yu Tsai
2017-01-31  3:50     ` Chen-Yu Tsai
2017-01-30 11:41 ` [PATCH 09/14] mtd: nand: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 12:44   ` Boris Brezillon
2017-01-30 12:44     ` Boris Brezillon
2017-01-31 17:20     ` Philipp Zabel
2017-01-31 17:20       ` Philipp Zabel
2017-01-31 17:21     ` Philipp Zabel
2017-01-31 17:21       ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 10/14] phy: meson8b-usb2: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-02-14 23:36   ` Martin Blumenstingl
2017-02-14 23:36     ` Martin Blumenstingl
2017-02-15 16:43     ` Jerome Brunet [this message]
2017-02-15 16:43       ` Jerome Brunet
2017-02-15 16:52       ` Philipp Zabel
2017-02-15 16:52         ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 11/14] serial: 8250_dw: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-02-02 22:21   ` Andy Shevchenko
2017-02-02 22:21     ` Andy Shevchenko
2017-02-07 16:48     ` Philipp Zabel
2017-02-07 16:48       ` Philipp Zabel
2017-02-07 16:48     ` Philipp Zabel
2017-02-07 16:48       ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 12/14] usb: dwc2: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-31  0:28   ` John Youn
2017-01-31  0:28     ` John Youn
2017-01-31  0:39     ` John Youn
2017-01-31  0:39       ` John Youn
2017-04-10 10:21   ` Felipe Balbi
2017-04-10 10:21     ` Felipe Balbi
2017-04-10 10:36     ` Philipp Zabel
2017-04-10 10:36       ` Philipp Zabel
2017-04-10 11:23       ` Felipe Balbi
2017-04-10 11:23         ` Felipe Balbi
2017-01-30 11:41 ` [PATCH 13/14] usb: host: ehci-st: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 11:41 ` [PATCH 14/14] ASoC: sunxi: " Philipp Zabel
2017-01-30 11:41   ` Philipp Zabel
2017-01-30 12:30   ` Mark Brown
2017-01-30 12:30     ` Mark Brown
2017-01-30 13:45     ` Philipp Zabel
2017-01-30 13:45       ` Philipp Zabel
2017-01-31  3:49   ` Chen-Yu Tsai
2017-01-31  3:49     ` Chen-Yu Tsai
2017-01-31 21:00   ` Mark Brown
2017-01-31 21:00     ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1487177007.30202.1.camel@baylibre.com \
    --to=jbrunet@baylibre.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.