| Message ID | 201405040219.30018.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, May 04, 2014 at 02:19:29AM +0400, Sergei Shtylyov wrote: > I've spent a couple of days with the driver just hanging due to me forgetting > to specify the external crystal frequency, so that clk_get_rate() returned 0 > and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an > acceptable behavior, so I suggest that the minimum frequency is checked for 0 > in tmio_mmc_host_probe(). > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> My suggestion is to update tmio_mmc_host_probe() so that it always exits, perhaps returning an error if appropriate. > > --- > The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. > > drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > Index: mmc/drivers/mmc/host/tmio_mmc_pio.c > =================================================================== > --- mmc.orig/drivers/mmc/host/tmio_mmc_pio.c > +++ mmc/drivers/mmc/host/tmio_mmc_pio.c > @@ -1044,6 +1044,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_ > } > > /* > + * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from > + * looping forever... > + */ > + if (mmc->f_min == 0) { > + ret = -EINVAL; > + goto pm_disable; > + } > + > + /* > * There are 4 different scenarios for the card detection: > * 1) an external gpio irq handles the cd (best for power savings) > * 2) internal sdhi irq handles the cd > -- > To unsubscribe from this list: send the line "unsubscribe linux-sh" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 05/13/2014 05:38 AM, Simon Horman wrote: >> I've spent a couple of days with the driver just hanging due to me forgetting >> to specify the external crystal frequency, so that clk_get_rate() returned 0 >> and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an >> acceptable behavior, so I suggest that the minimum frequency is checked for 0 >> in tmio_mmc_host_probe(). >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > My suggestion is to update tmio_mmc_host_probe() so that it always exits, > perhaps returning an error if appropriate. Did you mean tmio_mmc_set_clock()? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 13, 2014 at 04:09:57PM +0400, Sergei Shtylyov wrote: > Hello. > > On 05/13/2014 05:38 AM, Simon Horman wrote: > > >>I've spent a couple of days with the driver just hanging due to me forgetting > >>to specify the external crystal frequency, so that clk_get_rate() returned 0 > >>and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an > >>acceptable behavior, so I suggest that the minimum frequency is checked for 0 > >>in tmio_mmc_host_probe(). > > >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > >My suggestion is to update tmio_mmc_host_probe() so that it always exits, > >perhaps returning an error if appropriate. > > Did you mean tmio_mmc_set_clock()? Sorry for the cut-and-paste error. Yes, that is what I meant. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 05/14/2014 01:47 AM, Simon Horman wrote: >>>> I've spent a couple of days with the driver just hanging due to me forgetting >>>> to specify the external crystal frequency, so that clk_get_rate() returned 0 >>>> and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an >>>> acceptable behavior, so I suggest that the minimum frequency is checked for 0 >>>> in tmio_mmc_host_probe(). >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> My suggestion is to update tmio_mmc_host_probe() so that it always exits, >>> perhaps returning an error if appropriate. >> Did you mean tmio_mmc_set_clock()? > Sorry for the cut-and-paste error. Yes, that is what I meant. Not sure about that, really (there's no documentation). That's why I went for the probe time check. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 05/04/2014 02:19 AM, Sergei Shtylyov wrote: > I've spent a couple of days with the driver just hanging due to me forgetting > to specify the external crystal frequency, so that clk_get_rate() returned 0 > and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an > acceptable behavior, so I suggest that the minimum frequency is checked for 0 > in tmio_mmc_host_probe(). > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > --- > The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. I'm still not seeing this patch applied anywhere in this repo... what's the problem with it? > drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > Index: mmc/drivers/mmc/host/tmio_mmc_pio.c > =================================================================== > --- mmc.orig/drivers/mmc/host/tmio_mmc_pio.c > +++ mmc/drivers/mmc/host/tmio_mmc_pio.c > @@ -1044,6 +1044,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_ > } > > /* > + * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from > + * looping forever... > + */ > + if (mmc->f_min == 0) { > + ret = -EINVAL; > + goto pm_disable; > + } > + > + /* > * There are 4 different scenarios for the card detection: > * 1) an external gpio irq handles the cd (best for power savings) > * 2) internal sdhi irq handles the cd WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 06/26/2014 11:56 PM, Sergei Shtylyov wrote: >> I've spent a couple of days with the driver just hanging due to me forgetting >> to specify the external crystal frequency, so that clk_get_rate() returned 0 >> and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an >> acceptable behavior, so I suggest that the minimum frequency is checked for 0 >> in tmio_mmc_host_probe(). >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> --- >> The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. > I'm still not seeing this patch applied anywhere in this repo... what's > the problem with it? Chris, Ulf, Ian, what's the issue with this patch? Do you want me to rework it? >> drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) > >> Index: mmc/drivers/mmc/host/tmio_mmc_pio.c >> =================================================================== >> --- mmc.orig/drivers/mmc/host/tmio_mmc_pio.c >> +++ mmc/drivers/mmc/host/tmio_mmc_pio.c >> @@ -1044,6 +1044,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_ >> } >> >> /* >> + * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from >> + * looping forever... >> + */ >> + if (mmc->f_min == 0) { >> + ret = -EINVAL; >> + goto pm_disable; >> + } >> + >> + /* >> * There are 4 different scenarios for the card detection: >> * 1) an external gpio irq handles the cd (best for power savings) >> * 2) internal sdhi irq handles the cd WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3 September 2014 21:27, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Hello. > > > On 06/26/2014 11:56 PM, Sergei Shtylyov wrote: > >>> I've spent a couple of days with the driver just hanging due to me >>> forgetting >>> to specify the external crystal frequency, so that clk_get_rate() >>> returned 0 >>> and thus the loop in tmio_mmc_set_clock() never ended. I don't think >>> that's an >>> acceptable behavior, so I suggest that the minimum frequency is checked >>> for 0 >>> in tmio_mmc_host_probe(). > > >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > >>> --- >>> The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. > > >> I'm still not seeing this patch applied anywhere in this repo... >> what's >> the problem with it? > > > Chris, Ulf, Ian, what's the issue with this patch? Do you want me to > rework it? I guess it has been forgotten, sorry about that. Please rebase it towards my mmc tree, which is the one we are using currently and repost it. Kind regards Uffe > > >>> drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >> >> >>> Index: mmc/drivers/mmc/host/tmio_mmc_pio.c >>> =================================================================== >>> --- mmc.orig/drivers/mmc/host/tmio_mmc_pio.c >>> +++ mmc/drivers/mmc/host/tmio_mmc_pio.c >>> @@ -1044,6 +1044,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_ >>> } >>> >>> /* >>> + * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() >>> from >>> + * looping forever... >>> + */ >>> + if (mmc->f_min == 0) { >>> + ret = -EINVAL; >>> + goto pm_disable; >>> + } >>> + >>> + /* >>> * There are 4 different scenarios for the card detection: >>> * 1) an external gpio irq handles the cd (best for power savings) >>> * 2) internal sdhi irq handles the cd > > > WBR, Sergei > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 09/04/2014 03:00 PM, Ulf Hansson wrote: >>>> I've spent a couple of days with the driver just hanging due to me >>>> forgetting >>>> to specify the external crystal frequency, so that clk_get_rate() >>>> returned 0 >>>> and thus the loop in tmio_mmc_set_clock() never ended. I don't think >>>> that's an >>>> acceptable behavior, so I suggest that the minimum frequency is checked >>>> for 0 >>>> in tmio_mmc_host_probe(). >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>> --- >>>> The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. >>> I'm still not seeing this patch applied anywhere in this repo... >>> what's >>> the problem with it? >> Chris, Ulf, Ian, what's the issue with this patch? Do you want me to >> rework it? > I guess it has been forgotten, sorry about that. > Please rebase it towards my mmc tree, which is the one we are using > currently and repost it. It should still apply flawlessly to your tree, want me to repost it anyway? > Kind regards > Uffe WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: mmc/drivers/mmc/host/tmio_mmc_pio.c =================================================================== --- mmc.orig/drivers/mmc/host/tmio_mmc_pio.c +++ mmc/drivers/mmc/host/tmio_mmc_pio.c @@ -1044,6 +1044,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_ } /* + * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from + * looping forever... + */ + if (mmc->f_min == 0) { + ret = -EINVAL; + goto pm_disable; + } + + /* * There are 4 different scenarios for the card detection: * 1) an external gpio irq handles the cd (best for power savings) * 2) internal sdhi irq handles the cd
I've spent a couple of days with the driver just hanging due to me forgetting to specify the external crystal frequency, so that clk_get_rate() returned 0 and thus the loop in tmio_mmc_set_clock() never ended. I don't think that's an acceptable behavior, so I suggest that the minimum frequency is checked for 0 in tmio_mmc_host_probe(). Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- The patch is against Chris Ball's 'mmc.git' repo's 'master' branch. drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++ 1 file changed, 9 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html