All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Felipe Balbi <balbi@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	USB list <linux-usb@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>
Subject: RE: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
Date: Thu, 6 Sep 2018 12:09:46 +0000	[thread overview]
Message-ID: <OSBPR01MB2293B264BC79AA974CE6F7F4D8010@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdWCKSBydcbFwKqGi9azVARFr-0YboC65Zgde24FJVeZZQ@mail.gmail.com>

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, September 6, 2018 4:28 PM
> 
> Hi Shimoda-san,
> 
> On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > R-Car Gen3 needs to enable clocks of both host and peripheral.
> > Since [eo]hci-platform disables the reset(s) when the drivers are
> > removed, renesas_usbhs driver doesn't work correctly. To fix this
> > issue, this patch adds multiple clocks management on this
> > renesas_usbhs driver.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
> 
> > @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto probe_fail_rst;
> >
> > +       if (priv->num_clks) {
> > +               ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks);
> 
> (inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero.

That's right. Thank you for your comment.

> > +               if (ret == -EPROBE_DEFER)
> 
> Why this special handling for -EPROBE_DEFER only?
> Shouldn't all errors be considered fatal?
> 
> Perhaps this is needed for backwards compatibility with old DT?

Yes, this is needed for backward s compatibility with old DT.

> In that case, you should do more thorough checking (first clock should
> exist, second should return -ENOENT).

I understood it. (For backwards compatibility with old DT, if second clock
returns -ENOENT, the driver should do a special handling.)
I'll fix this patch.

Best regards,
Yoshihiro Shimoda

> > +                       goto probe_fail_clks;
> > +       }
> > +
> >         /*
> >          * deviece reset here because
> >          * USB device might be used in boot loader.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

WARNING: multiple messages have this Message-ID (diff)
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Felipe Balbi <balbi@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	USB list <linux-usb@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>
Subject: [v3,3/3] usb: renesas_usbhs: Add multiple clocks management
Date: Thu, 6 Sep 2018 12:09:46 +0000	[thread overview]
Message-ID: <OSBPR01MB2293B264BC79AA974CE6F7F4D8010@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw)

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, September 6, 2018 4:28 PM
> 
> Hi Shimoda-san,
> 
> On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > R-Car Gen3 needs to enable clocks of both host and peripheral.
> > Since [eo]hci-platform disables the reset(s) when the drivers are
> > removed, renesas_usbhs driver doesn't work correctly. To fix this
> > issue, this patch adds multiple clocks management on this
> > renesas_usbhs driver.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
> 
> > @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto probe_fail_rst;
> >
> > +       if (priv->num_clks) {
> > +               ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks);
> 
> (inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero.

That's right. Thank you for your comment.

> > +               if (ret == -EPROBE_DEFER)
> 
> Why this special handling for -EPROBE_DEFER only?
> Shouldn't all errors be considered fatal?
> 
> Perhaps this is needed for backwards compatibility with old DT?

Yes, this is needed for backward s compatibility with old DT.

> In that case, you should do more thorough checking (first clock should
> exist, second should return -ENOENT).

I understood it. (For backwards compatibility with old DT, if second clock
returns -ENOENT, the driver should do a special handling.)
I'll fix this patch.

Best regards,
Yoshihiro Shimoda

> > +                       goto probe_fail_clks;
> > +       }
> > +
> >         /*
> >          * deviece reset here because
> >          * USB device might be used in boot loader.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

  reply	other threads:[~2018-09-06 12:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-06  5:50 [PATCH v3 0/3] usb: renesas_usbhs: add reset_control and multiple clocks management Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 1/3] usb: renesas_usbhs: Add reset_control Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,1/3] " Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 2/3] dt-bindings: usb: renesas_usbhs: add clock-names property Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,2/3] " Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,3/3] " Yoshihiro Shimoda
2018-09-06  6:49   ` [PATCH v3 3/3] " Kuninori Morimoto
2018-09-06  6:49     ` [v3,3/3] " Kuninori Morimoto
2018-09-06  6:49     ` [PATCH v3 3/3] " Kuninori Morimoto
2018-09-06  7:28   ` Geert Uytterhoeven
2018-09-06  7:28     ` [v3,3/3] " Geert Uytterhoeven
2018-09-06 12:09     ` Yoshihiro Shimoda [this message]
2018-09-06 12:09       ` Yoshihiro Shimoda

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=OSBPR01MB2293B264BC79AA974CE6F7F4D8010@OSBPR01MB2293.jpnprd01.prod.outlook.com \
    --to=yoshihiro.shimoda.uh@renesas.com \
    --cc=balbi@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.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.