Hi Adam, Geert-san,
> From: Geert Uytterhoeven, Sent: Thursday, July 1, 2021 6:07 PM
<snip>
> To: Adam Ford <aford173@xxxxxxxxx>
> Cc: USB list <linux-usb@xxxxxxxxxxxxxxx>; Adam Ford-BE <aford@xxxxxxxxxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Magnus Damm <magnus.damm@xxxxxxxxx>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>; Linux-Renesas
> <linux-renesas-soc@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 2/3] usb: renesas_usbhs: Enable support for more than two clks
>
> Hi Adam,
>
> Thanks for your patch!
>
> On Wed, Jun 30, 2021 at 7:30 PM Adam Ford wrote:
<snip>
> > diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
> > index 3af91b2b8f76..255e4bd68ed3 100644
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
<snip>
> > @@ -309,11 +311,13 @@ static int usbhsc_clk_get(struct device *dev, struct usbhs_priv *priv)
> > * To backward compatibility with old DT, this driver checks the return
> > * value if it's -ENOENT or not.
> > */
> > - priv->clks[1] = of_clk_get(dev_of_node(dev), 1);
> > - if (PTR_ERR(priv->clks[1]) == -ENOENT)
> > - priv->clks[1] = NULL;
> > - else if (IS_ERR(priv->clks[1]))
> > - return PTR_ERR(priv->clks[1]);
> > + for (i = 1; i < ARRAY_SIZE(priv->clks); i++) {
> > + priv->clks[1] = of_clk_get(dev->of_node, i);
s/clks[1]/[i]/
> > + if (PTR_ERR(priv->clks[i]) == -ENOENT)
> > + priv->clks[i] = NULL;
> > + else if (IS_ERR(priv->clks[i]))
> > + return PTR_ERR(priv->clks[i]);
> > + }
>
> This is identical to the current code, as ARRAY_SIZE(priv->clks) == 2.
> Probably you wanted to increase usbhs_priv.clks[], too?
I think so.
# I realized the clks array is only 2, so that this driver cannot
# enable usb2_clksel...
> Does it make sense to start using the clk_bulk*() API?
clk_bulk*() API seems to need clock-names property.
Is my understanding correct? However, the hsusb nodes doesn't
have the property for now...
Best regards,
Yoshihiro Shimoda
