| Message ID | 1345648201-10746-1-git-send-email-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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;
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(-)