Re: [PATCH 3/3] mtd: gpmi: change the code for clocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

On Thu, Jun 28, 2012 at 11:52:05PM -0400, Huang Shijie wrote:
> From: Huang Shijie <b32955@xxxxxxxxxxxxx>
> 
> The gpmi nand driver may needs several clocks(MX6Q needs five clocks).
> 
> In the old clock framework, all these clocks are chained together,
> all you need is to manipulate the first clock.
> 
> But the kernel uses the common clk framework now, which forces us to
> get the clocks one by one. When we use them, we have to enable them
> one by one too.
> 
> Signed-off-by: Huang Shijie <b32955@xxxxxxxxxxxxx>
> Signed-off-by: Huang Shijie <shijie8@xxxxxxxxx>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   43 ++++++++++++++++++----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   63 +++++++++++++++++++++++++++-----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    3 +-
>  3 files changed, 91 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index a1f4332..c3778c0 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -124,12 +124,40 @@ error:
>  	return -ETIMEDOUT;
>  }
>  
> +static int __gpmi_enable_clk(struct gpmi_nand_data *this, bool v)
> +{
> +	struct clk *clk;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < GPMI_CLK_MAX; i++) {
> +		clk = this->resources.clock[i];
> +		if (!clk)
> +			break;
> +
> +		if (v) {
> +			ret = clk_prepare_enable(clk);
> +			if (ret)
> +				goto err_clk;
> +		} else {
> +			clk_disable_unprepare(clk);
> +		}
> +	}
> +	return 0;
> +
> +err_clk:
> +	return ret;
> +}
> +
> +#define gpmi_enable_clk(x) __gpmi_enable_clk(x, true)
> +#define gpmi_disable_clk(x) __gpmi_enable_clk(x, false)
> +
>  int gpmi_init(struct gpmi_nand_data *this)
>  {
>  	struct resources *r = &this->resources;
>  	int ret;
>  
> -	ret = clk_prepare_enable(r->clock);
> +	ret = gpmi_enable_clk(this);
>  	if (ret)
>  		goto err_out;
>  	ret = gpmi_reset_block(r->gpmi_regs, false);
> @@ -149,7 +177,7 @@ int gpmi_init(struct gpmi_nand_data *this)
>  	/* Select BCH ECC. */
>  	writel(BM_GPMI_CTRL1_BCH_MODE, r->gpmi_regs + HW_GPMI_CTRL1_SET);
>  
> -	clk_disable_unprepare(r->clock);
> +	gpmi_disable_clk(this);
>  	return 0;
>  err_out:
>  	return ret;
> @@ -205,7 +233,7 @@ int bch_set_geometry(struct gpmi_nand_data *this)
>  	ecc_strength  = bch_geo->ecc_strength >> 1;
>  	page_size     = bch_geo->page_size;
>  
> -	ret = clk_prepare_enable(r->clock);
> +	ret = gpmi_enable_clk(this);
>  	if (ret)
>  		goto err_out;
>  
> @@ -240,7 +268,7 @@ int bch_set_geometry(struct gpmi_nand_data *this)
>  	writel(BM_BCH_CTRL_COMPLETE_IRQ_EN,
>  				r->bch_regs + HW_BCH_CTRL_SET);
>  
> -	clk_disable_unprepare(r->clock);
> +	gpmi_disable_clk(this);
>  	return 0;
>  err_out:
>  	return ret;
> @@ -716,7 +744,7 @@ void gpmi_begin(struct gpmi_nand_data *this)
>  	int ret;
>  
>  	/* Enable the clock. */
> -	ret = clk_prepare_enable(r->clock);
> +	ret = gpmi_enable_clk(this);
>  	if (ret) {
>  		pr_err("We failed in enable the clk\n");
>  		goto err_out;
> @@ -727,7 +755,7 @@ void gpmi_begin(struct gpmi_nand_data *this)
>  		gpmi_regs + HW_GPMI_TIMING1);
>  
>  	/* Get the timing information we need. */
> -	nfc->clock_frequency_in_hz = clk_get_rate(r->clock);
> +	nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
>  	clock_period_in_ns = 1000000000 / nfc->clock_frequency_in_hz;
>  
>  	gpmi_nfc_compute_hardware_timing(this, &hw);
> @@ -784,8 +812,7 @@ err_out:
>  
>  void gpmi_end(struct gpmi_nand_data *this)
>  {
> -	struct resources *r = &this->resources;
> -	clk_disable_unprepare(r->clock);
> +	gpmi_disable_clk(this);
>  }
>  
>  /* Clears a BCH interrupt. */
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 941cfb7..edda3b1 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -464,9 +464,59 @@ acquire_err:
>  	return -EINVAL;
>  }
>  
> +static void gpmi_put_clks(struct gpmi_nand_data *this)
> +{
> +	struct resources *r = &this->resources;
> +	struct clk *clk;
> +	int i;
> +
> +	for (i = 0; i < GPMI_CLK_MAX; i++) {
> +		clk = r->clock[i];
> +		if (clk) {
> +			clk_put(clk);
> +			r->clock[i] = NULL;
> +		}
> +	}
> +}
> +
> +static char *extra_clks_for_mx6q[] = {
> +	"gpmi_apb", "gpmi_bch", "gpmi_bch_apb", "per1_bch",
> +};
> +
> +static int __devinit gpmi_get_clks(struct gpmi_nand_data *this)
> +{
> +	struct resources *r = &this->resources;
> +	char **extra_clks = NULL;
> +	struct clk *clk;
> +	int i;
> +
> +	r->clock[0] = clk_get(&this->pdev->dev, NULL);
> +	if (IS_ERR(r->clock[0]))
> +		goto err_clock;
> +
> +	/* Get extra clocks */
> +	if (GPMI_IS_MX6Q(this))
> +		extra_clks = extra_clks_for_mx6q;

We probably do not need this tweaking.  We can have the driver always
take all those 5 clocks, and I think the current imx28 clock driver
can just work with it, because the gpmi-nand clkdev lookup has NULL
con_id and all those 5 clocks can match the same one on imx28.

> +	if (!extra_clks)
> +		return 0;
> +
> +	for (i = 1; i < GPMI_CLK_MAX; i++) {
> +		clk = clk_get(NULL, extra_clks[i - 1]);

You should not put NULL but &this->pdev->dev here.

Regards,
Shawn

> +		if (IS_ERR(clk))
> +			goto err_clock;
> +
> +		r->clock[i] = clk;
> +	}
> +	return 0;
> +
> +err_clock:
> +	pr_err("failed in finding the clocks.\n");
> +	gpmi_put_clks(this);
> +	return -ENOMEM;
> +}
> +
>  static int __devinit acquire_resources(struct gpmi_nand_data *this)
>  {
> -	struct resources *res = &this->resources;
>  	struct pinctrl *pinctrl;
>  	int ret;
>  
> @@ -492,12 +542,9 @@ static int __devinit acquire_resources(struct gpmi_nand_data *this)
>  		goto exit_pin;
>  	}
>  
> -	res->clock = clk_get(&this->pdev->dev, NULL);
> -	if (IS_ERR(res->clock)) {
> -		pr_err("can not get the clock\n");
> -		ret = -ENOENT;
> +	ret = gpmi_get_clks(this);
> +	if (ret)
>  		goto exit_clock;
> -	}
>  	return 0;
>  
>  exit_clock:
> @@ -512,9 +559,7 @@ exit_regs:
>  
>  static void release_resources(struct gpmi_nand_data *this)
>  {
> -	struct resources *r = &this->resources;
> -
> -	clk_put(r->clock);
> +	gpmi_put_clks(this);
>  	release_register_block(this);
>  	release_bch_irq(this);
>  	release_dma_channels(this);
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index ce5daa1..1547a60 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -22,6 +22,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/fsl/mxs-dma.h>
>  
> +#define GPMI_CLK_MAX 5 /* MX6Q needs five clocks */
>  struct resources {
>  	void          *gpmi_regs;
>  	void          *bch_regs;
> @@ -29,7 +30,7 @@ struct resources {
>  	unsigned int  bch_high_interrupt;
>  	unsigned int  dma_low_channel;
>  	unsigned int  dma_high_channel;
> -	struct clk    *clock;
> +	struct clk    *clock[GPMI_CLK_MAX];
>  };
>  
>  /**
> -- 
> 1.7.4.4
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


[Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [PDAs]     [Linux]     [Linux MIPS]     [Yosemite Campsites]     [Photos]

Add to Google Follow linuxarm on Twitter