[PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Shawn Guo]

Since commit 30832ab (mmc: sdhci: Always pass clock request value zero
to set_clock host op) gets in, esdhc_set_clock starts hitting
"if (clock == 0)" where ESDHC_SYSTEM_CONTROL has been operated.  This
causes SDHCI card-detection function being broken.  Fix the regression
by moving "if (clock == 0)" above ESDHC_SYSTEM_CONTROL operation.

Cc: <stable@vger.kernel.org>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/mmc/host/sdhci-esdhc.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index b97b2f5..d25f9ab 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -48,14 +48,14 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
 	int div = 1;
 	u32 temp;
 
+	if (clock == 0)
+		goto out;
+
 	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
 	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
 		| ESDHC_CLOCK_MASK);
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
 
-	if (clock == 0)
-		goto out;
-
 	while (host->max_clk / pre_div / 16 > clock && pre_div < 256)
 		pre_div *= 2;
 
-- 
1.7.5.4

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Chris Ball]

Hi,

On Wed, Aug 22 2012, Shawn Guo wrote:
> Since commit 30832ab (mmc: sdhci: Always pass clock request value zero
> to set_clock host op) gets in, esdhc_set_clock starts hitting
> "if (clock == 0)" where ESDHC_SYSTEM_CONTROL has been operated.  This
> causes SDHCI card-detection function being broken.  Fix the regression
> by moving "if (clock == 0)" above ESDHC_SYSTEM_CONTROL operation.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/mmc/host/sdhci-esdhc.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
> index b97b2f5..d25f9ab 100644
> --- a/drivers/mmc/host/sdhci-esdhc.h
> +++ b/drivers/mmc/host/sdhci-esdhc.h
> @@ -48,14 +48,14 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
>  	int div = 1;
>  	u32 temp;
>  
> +	if (clock == 0)
> +		goto out;
> +
>  	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
>  	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
>  		| ESDHC_CLOCK_MASK);
>  	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
>  
> -	if (clock == 0)
> -		goto out;
> -
>  	while (host->max_clk / pre_div / 16 > clock && pre_div < 256)
>  		pre_div *= 2;

Thanks, pushed to mmc-next for 3.6.  (stable@ shouldn't be CC'd.)

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Shawn Guo]

On 28 August 2012 07:07, Chris Ball <cjb@laptop.org> wrote:
> Thanks, pushed to mmc-next for 3.6.  (stable@ shouldn't be CC'd.)
>
I categorised it as a fix for a regression.

Regards,
Shawn

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Chris Ball]

Hi,

On Mon, Aug 27 2012, Shawn Guo wrote:
> On 28 August 2012 07:07, Chris Ball <cjb@laptop.org> wrote:
>> Thanks, pushed to mmc-next for 3.6.  (stable@ shouldn't be CC'd.)
>>
> I categorised it as a fix for a regression.

It is a fix for a regression, but the rules for submitting patches to
stable@ are significantly more strict than "is it a regression?".
>From Documentation/stable-kernel-rules.txt:

==
Rules on what kind of patches are accepted, and which ones are not, into the
"-stable" tree:

 ...
 - It or an equivalent fix must already exist in Linus' tree (upstream).

Procedure for submitting patches to the -stable tree:

 - Send the patch, after verifying that it follows the above rules, to
   stable@vger.kernel.org.  You must note the upstream commit ID in the
   changelog of your submission, as well as the kernel version you wish
   it to be applied to.
==

It doesn't already exist in mainline -- so it shouldn't have been
e-mailed to stable@ -- and even if it were already in mainline, you
shouldn't e-mail stable@ without including the upstream commit ID
and kernel version to apply it to.  (Unless I'm missing something?)

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Shawn Guo]

On 28 August 2012 07:29, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Mon, Aug 27 2012, Shawn Guo wrote:
>> On 28 August 2012 07:07, Chris Ball <cjb@laptop.org> wrote:
>>> Thanks, pushed to mmc-next for 3.6.  (stable@ shouldn't be CC'd.)
>>>
>> I categorised it as a fix for a regression.
>
It means not only for stable but also for -rc series rather than -next.

> It is a fix for a regression, but the rules for submitting patches to
> stable@ are significantly more strict than "is it a regression?".
> From Documentation/stable-kernel-rules.txt:
>
> ==
> Rules on what kind of patches are accepted, and which ones are not, into the
> "-stable" tree:
>
>  ...
>  - It or an equivalent fix must already exist in Linus' tree (upstream).
>
> Procedure for submitting patches to the -stable tree:
>
>  - Send the patch, after verifying that it follows the above rules, to
>    stable@vger.kernel.org.  You must note the upstream commit ID in the
>    changelog of your submission, as well as the kernel version you wish
>    it to be applied to.
> ==
>
> It doesn't already exist in mainline -- so it shouldn't have been
> e-mailed to stable@ -- and even if it were already in mainline, you
> shouldn't e-mail stable@ without including the upstream commit ID
> and kernel version to apply it to.  (Unless I'm missing something?)
>
Ok, thanks for pointing it out.  But in practice, the process seems
not so strict.  I have a lot of successful cases with Greg (Cc-ed) by
simply Cc-ing stable when submit a fix that should be applied on
stable kernel.

Doing this strictly means people have to keep tracking if the fix hits
mainline and remember to resubmit it to stable afterwards.  Should
subsystem maintainer or author be that people?  It's not easy anyway.

Regards,
Shawn

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Chris Ball]

Hi Shawn,

On Mon, Aug 27 2012, Shawn Guo wrote:
>>> On 28 August 2012 07:07, Chris Ball <cjb@laptop.org> wrote:
>>>> Thanks, pushed to mmc-next for 3.6.  (stable@ shouldn't be CC'd.)
>>>>
>>> I categorised it as a fix for a regression.
>>
> It means not only for stable but also for -rc series rather than -next.

Sure; I've already applied it to mmc-next for 3.6.

> Doing this strictly means people have to keep tracking if the fix hits
> mainline and remember to resubmit it to stable afterwards.  Should
> subsystem maintainer or author be that people?  It's not easy anyway.

No, the stable team look for patches entering mainline with "Cc:
stable@vger.kernel.org" in the commit message, and apply them to
stable@ at that point.  That's the right time to apply them.

So, as I understand it, you did the right thing in having "Cc:
<stable@vger.kernel.org>" inside your commit message, and the
wrong thing in sending the mail to stable@ at the same time.
That's why I removed it from CC.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Shawn Guo]

On 28 August 2012 08:41, Chris Ball <cjb@laptop.org> wrote:
> So, as I understand it, you did the right thing in having "Cc:
> <stable@vger.kernel.org>" inside your commit message, and the
> wrong thing in sending the mail to stable@ at the same time.
> That's why I removed it from CC.
>
Ah, ok.  So it seems that I should have --suppress-cc in the git
send-email command for such patches.

Regards,
Shawn

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Greg Kroah-Hartman]

On Tue, Aug 28, 2012 at 09:50:13AM +0800, Shawn Guo wrote:
> On 28 August 2012 08:41, Chris Ball <cjb@laptop.org> wrote:
> > So, as I understand it, you did the right thing in having "Cc:
> > <stable@vger.kernel.org>" inside your commit message, and the
> > wrong thing in sending the mail to stable@ at the same time.
> > That's why I removed it from CC.
> >
> Ah, ok.  So it seems that I should have --suppress-cc in the git
> send-email command for such patches.

Not really, I don't mind it getting sent to stable@ for stuff like this,
it's trivial to filter away, and it helps me track, or pay attention to,
stuff that will be showing up in Linus's tree sometime soon.

So don't worry about it.

greg k-h

Re: [PATCH] mmc: sdhci-esdhc: break out early if clock is 0

[Chris Ball]

Hi,

On Mon, Aug 27 2012, Greg Kroah-Hartman wrote:
> On Tue, Aug 28, 2012 at 09:50:13AM +0800, Shawn Guo wrote:
>> On 28 August 2012 08:41, Chris Ball <cjb@laptop.org> wrote:
>> > So, as I understand it, you did the right thing in having "Cc:
>> > <stable@vger.kernel.org>" inside your commit message, and the
>> > wrong thing in sending the mail to stable@ at the same time.
>> > That's why I removed it from CC.
>> >
>> Ah, ok.  So it seems that I should have --suppress-cc in the git
>> send-email command for such patches.
>
> Not really, I don't mind it getting sent to stable@ for stuff like this,
> it's trivial to filter away, and it helps me track, or pay attention to,
> stuff that will be showing up in Linus's tree sometime soon.
>
> So don't worry about it.

Oh, that's good -- I'm sorry for the bogus correction, Shawn!

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child