mbox series

[0/2] mmc: jz4740: Support PLL frequency changes

Message ID 20210307170742.70949-1-paul@crapouillou.net
Headers show
Series mmc: jz4740: Support PLL frequency changes | expand

Message

Paul Cercueil March 7, 2021, 5:07 p.m. UTC
Hi,

This set of two patches enables the MMC driver to cope with the main PLL
updating its rate, typically when the CPU frequency is being updated.

The first patch introduces clk_get_first_to_set_rate(), which will allow
the MMC driver to get a pointer to the clock that will effectively be
modified when calling clk_set_rate(); this is required to avoid a
chicken-and-egg situation with the clock notifier.

If accepted, this function will be reused in a few more drivers which
need to perform the same operation.

The patch to the MMC driver adds a atomic/mutex couple so that the
frequency change will happen when we know that the controller is not in
use.

Cheers,
-Paul

Paul Cercueil (2):
  clk: Add clk_get_first_to_set_rate
  mmc: jz4740: Add support for monitoring PLL clock rate changes

 drivers/clk/clk.c             |  9 +++++
 drivers/mmc/host/jz4740_mmc.c | 70 ++++++++++++++++++++++++++++++++++-
 include/linux/clk.h           | 16 ++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)

Comments

Paul Cercueil March 8, 2021, 2:46 p.m. UTC | #1
Le dim. 7 mars 2021 à 17:07, Paul Cercueil <paul@crapouillou.net> a 
écrit :
> The main PLL can have its rate changed at any moment. To keep the MMC

> clock running at a rate that fits the specifications, we need to

> recompute the MMC clock rate every time the PLL rate changes.

> 

> Use a mutex to ensure that the MMC is idle before performing the PLL 

> and

> MMC rate changes.

> 

> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

> ---

>  drivers/mmc/host/jz4740_mmc.c | 70 

> ++++++++++++++++++++++++++++++++++-

>  1 file changed, 69 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/mmc/host/jz4740_mmc.c 

> b/drivers/mmc/host/jz4740_mmc.c

> index b3c636edbb46..1197b8c6b6ed 100644

> --- a/drivers/mmc/host/jz4740_mmc.c

> +++ b/drivers/mmc/host/jz4740_mmc.c

> @@ -18,6 +18,7 @@

>  #include <linux/mmc/host.h>

>  #include <linux/mmc/slot-gpio.h>

>  #include <linux/module.h>

> +#include <linux/mutex.h>

>  #include <linux/of_device.h>

>  #include <linux/pinctrl/consumer.h>

>  #include <linux/platform_device.h>

> @@ -149,6 +150,10 @@ struct jz4740_mmc_host {

>  	struct platform_device *pdev;

>  	struct clk *clk;

> 

> +	atomic_t clk_mutex_count;

> +	struct mutex clk_mutex;

> +	struct notifier_block clock_nb;

> +

>  	enum jz4740_mmc_version version;

> 

>  	int irq;

> @@ -338,6 +343,9 @@ static void jz4740_mmc_pre_request(struct 

> mmc_host *mmc,

>  	struct jz4740_mmc_host *host = mmc_priv(mmc);

>  	struct mmc_data *data = mrq->data;

> 

> +	if (atomic_inc_and_test(&host->clk_mutex_count))

> +		mutex_lock(&host->clk_mutex);


There's an obvious race here, let me rewrite this using the proper 
locking mechanism.

-Paul

> +

>  	if (!host->use_dma)

>  		return;

> 

> @@ -353,6 +361,9 @@ static void jz4740_mmc_post_request(struct 

> mmc_host *mmc,

>  	struct jz4740_mmc_host *host = mmc_priv(mmc);

>  	struct mmc_data *data = mrq->data;

> 

> +	if (atomic_dec_return(&host->clk_mutex_count) == -1)

> +		mutex_unlock(&host->clk_mutex);

> +

>  	if (data && data->host_cookie != COOKIE_UNMAPPED)

>  		jz4740_mmc_dma_unmap(host, data);

> 

> @@ -955,6 +966,48 @@ static const struct mmc_host_ops jz4740_mmc_ops 

> = {

>  	.enable_sdio_irq = jz4740_mmc_enable_sdio_irq,

>  };

> 

> +static inline struct jz4740_mmc_host *

> +jz4740_mmc_nb_get_priv(struct notifier_block *nb)

> +{

> +	return container_of(nb, struct jz4740_mmc_host, clock_nb);

> +}

> +

> +static struct clk *jz4740_mmc_get_parent_clk(struct clk *clk)

> +{

> +	/*

> +	 * Return the first clock above the one that will effectively modify

> +	 * its rate when clk_set_rate(clk) is called.

> +	 */

> +	clk = clk_get_first_to_set_rate(clk);

> +

> +	return clk_get_parent(clk);

> +}

> +

> +static int jz4740_mmc_update_clk(struct notifier_block *nb,

> +				 unsigned long action,

> +				 void *data)

> +{

> +	struct jz4740_mmc_host *host = jz4740_mmc_nb_get_priv(nb);

> +

> +	/*

> +	 * PLL may have changed its frequency; our clock may be running 

> above

> +	 * spec. Wait until MMC is idle (using host->clk_mutex) before 

> changing

> +	 * the PLL clock, and after it's done, reset our clock rate.

> +	 */

> +

> +	switch (action) {

> +	case PRE_RATE_CHANGE:

> +		mutex_lock(&host->clk_mutex);

> +		break;

> +	default:

> +		clk_set_rate(host->clk, host->mmc->f_max);

> +		mutex_unlock(&host->clk_mutex);

> +		break;

> +	}

> +

> +	return NOTIFY_OK;

> +}

> +

>  static const struct of_device_id jz4740_mmc_of_match[] = {

>  	{ .compatible = "ingenic,jz4740-mmc", .data = (void *) 

> JZ_MMC_JZ4740 },

>  	{ .compatible = "ingenic,jz4725b-mmc", .data = (void 

> *)JZ_MMC_JZ4725B },

> @@ -971,6 +1024,7 @@ static int jz4740_mmc_probe(struct 

> platform_device* pdev)

>  	struct mmc_host *mmc;

>  	struct jz4740_mmc_host *host;

>  	const struct of_device_id *match;

> +	struct clk *parent_clk;

> 

>  	mmc = mmc_alloc_host(sizeof(struct jz4740_mmc_host), &pdev->dev);

>  	if (!mmc) {

> @@ -1058,12 +1112,24 @@ static int jz4740_mmc_probe(struct 

> platform_device* pdev)

>  		goto err_free_irq;

>  	host->use_dma = !ret;

> 

> +	atomic_set(&host->clk_mutex_count, -1);

> +	mutex_init(&host->clk_mutex);

> +	host->clock_nb.notifier_call = jz4740_mmc_update_clk;

> +

> +	parent_clk = jz4740_mmc_get_parent_clk(host->clk);

> +

> +	ret = clk_notifier_register(parent_clk, &host->clock_nb);

> +	if (ret) {

> +		dev_err(&pdev->dev, "Unable to register clock notifier\n");

> +		goto err_release_dma;

> +	}

> +

>  	platform_set_drvdata(pdev, host);

>  	ret = mmc_add_host(mmc);

> 

>  	if (ret) {

>  		dev_err(&pdev->dev, "Failed to add mmc host: %d\n", ret);

> -		goto err_release_dma;

> +		goto err_unregister_clk_notifier;

>  	}

>  	dev_info(&pdev->dev, "Ingenic SD/MMC card driver registered\n");

> 

> @@ -1074,6 +1140,8 @@ static int jz4740_mmc_probe(struct 

> platform_device* pdev)

> 

>  	return 0;

> 

> +err_unregister_clk_notifier:

> +	clk_notifier_unregister(parent_clk, &host->clock_nb);

>  err_release_dma:

>  	if (host->use_dma)

>  		jz4740_mmc_release_dma_channels(host);

> --

> 2.30.1

>