[PATCH] mmc: sh_mobile_sdhi: fix error return code in sh_mobile_sdhi_probe()

[Wei Yongjun]

Hi

On 05/29/2013 02:58 PM, Guennadi Liakhovetski wrote:
> Hi
>
> On Tue, 28 May 2013, Wei Yongjun wrote:
>
>> From: Wei Yongjun <yongjun_wei at trendmicro.com.cn>
>>
>> Fix to return a negative error code instead of 0 when we cannot get
>> IRQ source by platform_get_irq(), as done elsewhere in this function.
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei at trendmicro.com.cn>
> Thanks for the patch. Do you think the following version would be even a 
> bit simpler?
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index fe90853..76661b6 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -255,18 +255,17 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	if (multiplexed_isr) {
>  		while (1) {
>  			irq = platform_get_irq(pdev, i);
> -			if (irq < 0)
> -				break;
> +			/* There must be at least one IRQ source */
> +			if (irq < 0) {
> +				ret = irq;
> +				goto eirq;
> +			}
>  			i++;
>  			ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_irq, 0,
>  					  dev_name(&pdev->dev), host);
>  			if (ret)
>  				goto eirq;
>  		}
> -
> -		/* There must be at least one IRQ source */
> -		if (!i)
> -			goto eirq;
>  	}
>  
>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",

The orig code used while loop to get many irqs, but with your change,
we will leave the loop util platform_get_irq() or request_irq() fail
and nerver success. Do your means remove the while loop and this device
need only one irq?

>
> If you agree, maybe you could send a v2 of your patch in this form, I'll 
> ack it then.
>
> Thanks
> Guennadi
>
>> ---
>>  drivers/mmc/host/sh_mobile_sdhi.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
>> index cc4c872..a4316b3 100644
>> --- a/drivers/mmc/host/sh_mobile_sdhi.c
>> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
>> @@ -273,8 +273,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>>  		}
>>  
>>  		/* There must be at least one IRQ source */
>> -		if (!i)
>> +		if (!i) {
>> +			ret = irq;
>>  			goto eirq;
>> +		}
>>  	}
>>  
>>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
>>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
>
>