Jerome Brunet <[email protected]> writes:

> Perform basic initialisation of the clk register before providing it to
> the CCF.
>
> Thanks to devm, carrying the clock structure around after init is not
> necessary. Rework the function to remove these from the controller host
> data.
>
> Finally, set initial mmc clock rate before enabling it, simplifying the
> exit condition.
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 101 
> +++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 8f9ba5190c18..4cc7d6530536 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -42,10 +42,7 @@
>  
>  #define SD_EMMC_CLOCK 0x0
>  #define   CLK_DIV_MASK GENMASK(5, 0)
> -#define   CLK_DIV_MAX 63
>  #define   CLK_SRC_MASK GENMASK(7, 6)
> -#define   CLK_SRC_XTAL 0   /* external crystal */
> -#define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
>  #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
>  #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
>  #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
> @@ -137,13 +134,9 @@ struct meson_host {
>       spinlock_t lock;
>       void __iomem *regs;
>       struct clk *core_clk;
> -     struct clk_mux mux;
> -     struct clk *mux_clk;
> +     struct clk *signal_clk;
>       unsigned long req_rate;
>  
> -     struct clk_divider cfg_div;
> -     struct clk *cfg_div_clk;
> -
>       unsigned int bounce_buf_size;
>       void *bounce_buf;
>       dma_addr_t bounce_dma_addr;
> @@ -291,7 +284,7 @@ static int meson_mmc_clk_set(struct meson_host *host, 
> unsigned long clk_rate)
>               return 0;
>       }
>  
> -     ret = clk_set_rate(host->cfg_div_clk, clk_rate);
> +     ret = clk_set_rate(host->signal_clk, clk_rate);

minor nit: where does the name "signal" come from?  I called this
"div_clk" because it's the output of the divider right before the
sd/emmc IP block.   Admittedly, that's not a great name either, and I'm
not too picky about the naming, just curious...

Looking at the diagram we have since I initially wrote the driver, this
is more commonly referred to as device_clk.

Anyways, if you're going to rename...

[...]

>  static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
> @@ -987,7 +984,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>       dma_free_coherent(host->dev, host->bounce_buf_size,
>                         host->bounce_buf, host->bounce_dma_addr);
>  err_div_clk:

... probably should rename this too.

Otherwise,

Reviewed-by: Kevin Hilman <[email protected]>

Kevin

Reply via email to