linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Chris Ruehl <chris.ruehl@gtsys.com.hk>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>, Heiko Stuebner <heiko@sntech.de>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>
Subject: Re: [PATCH] phy: rockchip-emmc: emmc_phy_init() always return 0
Date: Tue, 1 Dec 2020 08:05:18 -0800	[thread overview]
Message-ID: <CAD=FV=Wyk9BYR3cnfm=9tBh=XBxEP=udMTeaEYPzAqa5m8x=yg@mail.gmail.com> (raw)
In-Reply-To: <20201201031015.23314-1-chris.ruehl@gtsys.com.hk>

Hi,

On Mon, Nov 30, 2020 at 7:10 PM Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote:
>
> rockchip_emmc_phy_init() return variable is not set with the error value
> if clk_get() failed. The debug message print 0 on error and the function
> always return 0.
> Fix it using PTR_ERR().
>
> Fixes: 52c0624a10cce phy: rockchip-emmc: Set phyctrl_frqsel based on card clock
>
> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> ---
>  drivers/phy/rockchip/phy-rockchip-emmc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c b/drivers/phy/rockchip/phy-rockchip-emmc.c
> index 48e2d75b1004..75faee5c0d27 100644
> --- a/drivers/phy/rockchip/phy-rockchip-emmc.c
> +++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
> @@ -253,6 +253,7 @@ static int rockchip_emmc_phy_init(struct phy *phy)
>          */
>         rk_phy->emmcclk = clk_get(&phy->dev, "emmcclk");
>         if (IS_ERR(rk_phy->emmcclk)) {
> +               ret = PTR_ERR(rk_phy->emmcclk);

I'm pretty sure your patch isn't correct and it would break use cases.
Is it fixing some bug that you're aware of, or you found it via code
inspection?

Specifically:

* The big comment block in this function says that the clock is
optional and that we're ignoring errors.

* The printout in this function is "dbg" level, which is an extra
indication that we aren't concerned with these errors.

Arguably the code could be made better.  If you want to improve it,
you could check for just the error we expect if the clock isn't
specified (probably -ENODEV, but you should check) and treat all other
failures as real errors.


-Doug

  reply	other threads:[~2020-12-01 16:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-01  3:10 Chris Ruehl
2020-12-01 16:05 ` Doug Anderson [this message]
2020-12-02  8:36   ` Chris Ruehl
2020-12-04  3:01     ` Chris Ruehl
2020-12-04 17:53       ` Doug Anderson

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='CAD=FV=Wyk9BYR3cnfm=9tBh=XBxEP=udMTeaEYPzAqa5m8x=yg@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=chris.ruehl@gtsys.com.hk \
    --cc=heiko@sntech.de \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vkoul@kernel.org \
    --subject='Re: [PATCH] phy: rockchip-emmc: emmc_phy_init() always return 0' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).