[PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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(+)

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

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Simon Horman]

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
> 

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Simon Horman]

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.

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Ulf Hansson]

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
>

Re: [PATCH] tmio_mmc_pio: prevent endless loop in tmio_mmc_set_clock()

[Sergei Shtylyov]

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