Hi Chris,

On Sat, Jan 21, 2017 at 4:06 AM, Chris Brandt <chris.bra...@renesas.com> wrote:
> Some controllers have 2 clock sources instead of 1. The 2nd clock
> is for the internal card detect logic and must be enabled/disabled
> along with the main core clock for proper operation.
>
> Signed-off-by: Chris Brandt <chris.bra...@renesas.com>
> ---
> v4:
> * add technical explanation within probe routine
> v3:
> * add more clarification to the commit log
> v2:
> * changed clk2 to clk_cd
> * disable clk if clk_cd enable fails
> * changed clock name from "carddetect" to "cd"

Thanks for the updates!

> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 59db14b..360d922 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -143,6 +143,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
>
>  struct sh_mobile_sdhi {
>         struct clk *clk;
> +       struct clk *clk_cd;
>         struct tmio_mmc_data mmc_data;
>         struct tmio_mmc_dma dma_priv;
>         struct pinctrl *pinctrl;
> @@ -190,6 +191,12 @@ static int sh_mobile_sdhi_clk_enable(struct 
> tmio_mmc_host *host)
>         if (ret < 0)
>                 return ret;
>
> +       ret = clk_prepare_enable(priv->clk_cd);
> +       if (ret < 0) {
> +               clk_disable_unprepare(priv->clk);
> +               return ret;
> +       }
> +

As enabling the "core" clock but not the "cd" clock is not a valid setting,
shouldn't the cd clock be enabled first?

>         /*
>          * The clock driver may not know what maximum frequency
>          * actually works, so it should be set with the max-frequency
> @@ -255,6 +262,8 @@ static void sh_mobile_sdhi_clk_disable(struct 
> tmio_mmc_host *host)
>         struct sh_mobile_sdhi *priv = host_to_priv(host);
>
>         clk_disable_unprepare(priv->clk);
> +       if (priv->clk_cd)

No need to check for a NULL pointer first.

> +               clk_disable_unprepare(priv->clk_cd);

Disabling is already done in the correct order ;-)

>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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 via email to