Re: [PATCH v3 3/4] clk: renesas: rcar-usb2-clock-sel: Add multiple clocks management

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Shimoda-san,

On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> This hardware needs to enable clocks of both host and peripheral.
> So, this patch adds multiple clocks management.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
> +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

> @@ -128,6 +146,14 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
>         if (IS_ERR(priv->base))
>                 return PTR_ERR(priv->base);
>
> +       priv->clks[CLK_INDEX_EHCI_OHCI].clk = devm_clk_get(dev, "ehci_ohci");
> +       if (IS_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk))
> +               return PTR_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk);
> +
> +       priv->clks[CLK_INDEX_HS_USB].clk = devm_clk_get(dev, "hs-usb-if");
> +       if (IS_ERR(priv->clks[CLK_INDEX_HS_USB].clk))
> +               return PTR_ERR(priv->clks[CLK_INDEX_HS_USB].clk);
> +

Is these any specific reason you're not just filling in the .id fields first,
and calling devm_clk_bulk_get()?

    static const struct clk_bulk_data rcar_usb2_clocks[] = {
            { .id = "ehci_ohci", },
            { .id = "hs-usb-if", },
    };

    memcpy(priv->clks, rcar_usb2_clocks, sizeof(priv->clks));
    ... = devm_clk_bulk_get(dev, ARRAY_SIZE(priv->clks), priv->clks);
    ...

That way you can drop the enums, and use ARRAY_SIZE(rcar_usb2_clocks)
instead of CLK_NUM.

>         pm_runtime_enable(dev);
>         pm_runtime_get_sync(dev);
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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