All of lore.kernel.org
 help / color / mirror / Atom feed
From: Huang Shijie <b32955@freescale.com>
To: Hui Wang <jason77.wang@gmail.com>
Cc: shawn.guo@linaro.org, dedekind1@gmail.com,
	linux-mtd@lists.infradead.org, dong.aisheng@linaro.org,
	Huang Shijie <shijie8@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 3/3] mtd: gpmi: change the code for clocks
Date: Mon, 2 Jul 2012 14:00:18 +0800	[thread overview]
Message-ID: <4FF138F2.6070503@freescale.com> (raw)
In-Reply-To: <4FF133F9.50704@gmail.com>

于 2012年07月02日 13:39, Hui Wang 写道:
> Huang Shijie wrote:
>> 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 <shijie8@gmail.com>
>> ---
>> arch/arm/boot/dts/imx6q-arm2.dts | 2 +-
>> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 43 ++++++++++++++---
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 82 
>> ++++++++++++++++++++++++++++----
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 3 +-
>> 4 files changed, 111 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6q-arm2.dts 
>> b/arch/arm/boot/dts/imx6q-arm2.dts
>> index 14e72e2..027a2f8 100644
>> --- a/arch/arm/boot/dts/imx6q-arm2.dts
>> +++ b/arch/arm/boot/dts/imx6q-arm2.dts
>> @@ -25,7 +25,7 @@
>> gpmi-nand@00112000 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_gpmi_nand_1>;
>> - status = "disabled"; /* gpmi nand conflicts with SD */
>> + status = "okay"; /* gpmi nand conflicts with SD */
>> };
>>
>> aips-bus@02100000 { /* AIPS2 */
>> 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:
> Doesn't this design introduce clk_enalbe/disable un-balance problems?
>
> Suppose you successfully enabled 1st and 2nd clocks, and failed at the 
> third clock, this function will return, then the caller function will 
> check the return value and call
>
> __gpmi_enable_clk(this, 0), in this function, it will unconditionally 
> disable all five clocks.
thanks, fix it in next version.

Huang Shijie
>
>
> Regards,
> Hui.
>
>> + return ret;
>> +}
>> +
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

WARNING: multiple messages have this Message-ID (diff)
From: b32955@freescale.com (Huang Shijie)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 3/3] mtd: gpmi: change the code for clocks
Date: Mon, 2 Jul 2012 14:00:18 +0800	[thread overview]
Message-ID: <4FF138F2.6070503@freescale.com> (raw)
In-Reply-To: <4FF133F9.50704@gmail.com>

? 2012?07?02? 13:39, Hui Wang ??:
> Huang Shijie wrote:
>> 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 <shijie8@gmail.com>
>> ---
>> arch/arm/boot/dts/imx6q-arm2.dts | 2 +-
>> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 43 ++++++++++++++---
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 82 
>> ++++++++++++++++++++++++++++----
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 3 +-
>> 4 files changed, 111 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6q-arm2.dts 
>> b/arch/arm/boot/dts/imx6q-arm2.dts
>> index 14e72e2..027a2f8 100644
>> --- a/arch/arm/boot/dts/imx6q-arm2.dts
>> +++ b/arch/arm/boot/dts/imx6q-arm2.dts
>> @@ -25,7 +25,7 @@
>> gpmi-nand at 00112000 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_gpmi_nand_1>;
>> - status = "disabled"; /* gpmi nand conflicts with SD */
>> + status = "okay"; /* gpmi nand conflicts with SD */
>> };
>>
>> aips-bus at 02100000 { /* AIPS2 */
>> 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:
> Doesn't this design introduce clk_enalbe/disable un-balance problems?
>
> Suppose you successfully enabled 1st and 2nd clocks, and failed at the 
> third clock, this function will return, then the caller function will 
> check the return value and call
>
> __gpmi_enable_clk(this, 0), in this function, it will unconditionally 
> disable all five clocks.
thanks, fix it in next version.

Huang Shijie
>
>
> Regards,
> Hui.
>
>> + return ret;
>> +}
>> +
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

  reply	other threads:[~2012-07-02  6:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-02  3:38 [PATCH v3 0/3] add gpmi support for imx6q Huang Shijie
2012-07-02  3:38 ` Huang Shijie
2012-07-02  3:38 ` [PATCH v3 1/3] ARM: imx6q: add clocks for gpmi-nand Huang Shijie
2012-07-02  3:38   ` Huang Shijie
2012-07-02  2:10   ` Shawn Guo
2012-07-02  2:10     ` Shawn Guo
2012-07-02  2:36     ` Huang Shijie
2012-07-02  3:38 ` [PATCH v3 2/3] ARM: imx6q: add DT node for gpmi nand Huang Shijie
2012-07-02  3:38   ` Huang Shijie
2012-07-02  2:24   ` Shawn Guo
2012-07-02  2:24     ` Shawn Guo
2012-07-02  3:38 ` [PATCH v3 3/3] mtd: gpmi: change the code for clocks Huang Shijie
2012-07-02  3:38   ` Huang Shijie
2012-07-02  2:48   ` Huang Shijie
2012-07-02  2:48     ` Huang Shijie
2012-07-02  5:39   ` Hui Wang
2012-07-02  5:39     ` Hui Wang
2012-07-02  6:00     ` Huang Shijie [this message]
2012-07-02  6:00       ` Huang Shijie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FF138F2.6070503@freescale.com \
    --to=b32955@freescale.com \
    --cc=dedekind1@gmail.com \
    --cc=dong.aisheng@linaro.org \
    --cc=jason77.wang@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=shawn.guo@linaro.org \
    --cc=shijie8@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.