diff mbox

ARM: OMAP2: add type cast from 'unsigned' to 'signed'

Message ID 521479B4.8030604@asianux.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chen Gang Aug. 21, 2013, 8:26 a.m. UTC
Need add type cast, or can not notice the failure. The related warning
(allmodconfig, "EXTRA_CFLAGS=-W"):

  arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 arch/arm/mach-omap2/gpmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Tony Lindgren Aug. 22, 2013, 7:14 a.m. UTC | #1
* Chen Gang <gang.chen@asianux.com> [130821 01:34]:
> Need add type cast, or can not notice the failure. The related warning
> (allmodconfig, "EXTRA_CFLAGS=-W"):
> 
>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/arm/mach-omap2/gpmc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index f3fdd6a..62377b5 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -725,7 +725,7 @@ static int gpmc_setup_irq(void)
>  		return -EINVAL;
>  
>  	gpmc_irq_start = irq_alloc_descs(-1, 0, GPMC_NR_IRQ, 0);
> -	if (gpmc_irq_start < 0) {
> +	if ((signed)gpmc_irq_start < 0) {
>  		pr_err("irq_alloc_descs failed\n");
>  		return gpmc_irq_start;
>  	}

Hmm shouldn't we just have int gpmc_irq_start instead
of unsigned gpmc_irq_start?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen Gang Aug. 22, 2013, 7:43 a.m. UTC | #2
On 08/22/2013 03:14 PM, Tony Lindgren wrote:
> * Chen Gang <gang.chen@asianux.com> [130821 01:34]:
>> Need add type cast, or can not notice the failure. The related warning
>> (allmodconfig, "EXTRA_CFLAGS=-W"):
>>
>>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>>  arch/arm/mach-omap2/gpmc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index f3fdd6a..62377b5 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -725,7 +725,7 @@ static int gpmc_setup_irq(void)
>>  		return -EINVAL;
>>  
>>  	gpmc_irq_start = irq_alloc_descs(-1, 0, GPMC_NR_IRQ, 0);
>> -	if (gpmc_irq_start < 0) {
>> +	if ((signed)gpmc_irq_start < 0) {
>>  		pr_err("irq_alloc_descs failed\n");
>>  		return gpmc_irq_start;
>>  	}
> 
> Hmm shouldn't we just have int gpmc_irq_start instead
> of unsigned gpmc_irq_start?
>

Oh, thanks, that sounds reasonable to me, I will send patch v2.


> Regards,
> 
> Tony
> 
> 

Thanks.
Tony Lindgren Aug. 22, 2013, 7:57 a.m. UTC | #3
* Chen Gang <gang.chen@asianux.com> [130822 00:55]:
> 'gpmc_irq_start' is mostly used as 'int', and for a variable, do not
> suggest to only use 'unsigned' as its type, so use 'int' instead of
> 'unsigned' for variable 'gpmc_irq_start'.
> 
> Also it will fix the related issue (dummy the real world failure):
> 
>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> 

Thanks applying into omap-for-v3.12/fixes-non-critical.

Tony
 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/arm/mach-omap2/gpmc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index f3fdd6a..9f4795a 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -149,7 +149,7 @@ struct omap3_gpmc_regs {
>  
>  static struct gpmc_client_irq gpmc_client_irq[GPMC_NR_IRQ];
>  static struct irq_chip gpmc_irq_chip;
> -static unsigned gpmc_irq_start;
> +static int gpmc_irq_start;
>  
>  static struct resource	gpmc_mem_root;
>  static struct resource	gpmc_cs_mem[GPMC_CS_NUM];
> -- 
> 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen Gang Aug. 22, 2013, 8:01 a.m. UTC | #4
On 08/22/2013 03:57 PM, Tony Lindgren wrote:
> * Chen Gang <gang.chen@asianux.com> [130822 00:55]:
>> 'gpmc_irq_start' is mostly used as 'int', and for a variable, do not
>> suggest to only use 'unsigned' as its type, so use 'int' instead of
>> 'unsigned' for variable 'gpmc_irq_start'.
>>
>> Also it will fix the related issue (dummy the real world failure):
>>
>>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>
> 
> Thanks applying into omap-for-v3.12/fixes-non-critical.
> 

Thank you too.

Hmm... excuse me, I am not quite familiar with the omap version trees,
so may have a doubt: "it seems this is a real bug, is it suitable to
belong to fixes-none-critical tree ?"


Thanks.

> Tony
>  
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>>  arch/arm/mach-omap2/gpmc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index f3fdd6a..9f4795a 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -149,7 +149,7 @@ struct omap3_gpmc_regs {
>>  
>>  static struct gpmc_client_irq gpmc_client_irq[GPMC_NR_IRQ];
>>  static struct irq_chip gpmc_irq_chip;
>> -static unsigned gpmc_irq_start;
>> +static int gpmc_irq_start;
>>  
>>  static struct resource	gpmc_mem_root;
>>  static struct resource	gpmc_cs_mem[GPMC_CS_NUM];
>> -- 
>> 1.7.7.6
> 
>
Tony Lindgren Aug. 22, 2013, 8:32 a.m. UTC | #5
* Chen Gang <gang.chen@asianux.com> [130822 01:10]:
> On 08/22/2013 03:57 PM, Tony Lindgren wrote:
> > * Chen Gang <gang.chen@asianux.com> [130822 00:55]:
> >> 'gpmc_irq_start' is mostly used as 'int', and for a variable, do not
> >> suggest to only use 'unsigned' as its type, so use 'int' instead of
> >> 'unsigned' for variable 'gpmc_irq_start'.
> >>
> >> Also it will fix the related issue (dummy the real world failure):
> >>
> >>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> >>
> > 
> > Thanks applying into omap-for-v3.12/fixes-non-critical.
> > 
> 
> Thank you too.
> 
> Hmm... excuse me, I am not quite familiar with the omap version trees,
> so may have a doubt: "it seems this is a real bug, is it suitable to
> belong to fixes-none-critical tree ?"

For the -rc cycle we try to limit the patches to oopses and regressions,
especially this close to v3.11 being tagged.

It seems the bug has been there for quite some time, and that we don't
seem to have oopses or other failures reporting this. So it seems there's
no reason for urgency to have it merged into the current -rc series.

If you (or somebody else) strongly feels that it should be in applied
to the current -rc series as a fix instead of during the merge window,
then please let me know the failing cases with logs showing what
happens.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen Gang F T Aug. 22, 2013, 8:59 a.m. UTC | #6
On 08/22/2013 04:32 PM, Tony Lindgren wrote:
> * Chen Gang <gang.chen@asianux.com> [130822 01:10]:
>> On 08/22/2013 03:57 PM, Tony Lindgren wrote:
>>> * Chen Gang <gang.chen@asianux.com> [130822 00:55]:
>>>> 'gpmc_irq_start' is mostly used as 'int', and for a variable, do not
>>>> suggest to only use 'unsigned' as its type, so use 'int' instead of
>>>> 'unsigned' for variable 'gpmc_irq_start'.
>>>>
>>>> Also it will fix the related issue (dummy the real world failure):
>>>>
>>>>   arch/arm/mach-omap2/gpmc.c:728:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>>>
>>>
>>> Thanks applying into omap-for-v3.12/fixes-non-critical.
>>>
>>
>> Thank you too.
>>
>> Hmm... excuse me, I am not quite familiar with the omap version trees,
>> so may have a doubt: "it seems this is a real bug, is it suitable to
>> belong to fixes-none-critical tree ?"
> 
> For the -rc cycle we try to limit the patches to oopses and regressions,
> especially this close to v3.11 being tagged.
> 

Really necessary.

> It seems the bug has been there for quite some time, and that we don't
> seem to have oopses or other failures reporting this. So it seems there's
> no reason for urgency to have it merged into the current -rc series.
> 

I guess so too (I only find it by compiling).


> If you (or somebody else) strongly feels that it should be in applied
> to the current -rc series as a fix instead of during the merge window,
> then please let me know the failing cases with logs showing what
> happens.
> 

Hmm... for urgent bugs, the reporters really need supply the failing
cases with logs showing what happens.

Bugs can be sort of by urgent priority (e.g. this bug is not urgent).
but for most of bugs may cause critical issue which depends on the using
environments, and can be 'imagined' by us.

Hmm... I guess: for our case, what your meaning is "fixes-none-urgent",
not "fixes-none-critical", is it correct ?  :-)


Thanks.

> Regards,
> 
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Tony Lindgren Aug. 23, 2013, 5:48 a.m. UTC | #7
* Chen Gang F T <chen.gang.flying.transformer@gmail.com> [130822 02:08]:
> Hmm... I guess: for our case, what your meaning is "fixes-none-urgent",
> not "fixes-none-critical", is it correct ?  :-)

Right, that naming might be actually better :)

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen Gang Aug. 23, 2013, 6:14 a.m. UTC | #8
On 08/23/2013 01:48 PM, Tony Lindgren wrote:
> * Chen Gang F T <chen.gang.flying.transformer@gmail.com> [130822 02:08]:
>> Hmm... I guess: for our case, what your meaning is "fixes-none-urgent",
>> not "fixes-none-critical", is it correct ?  :-)
> 
> Right, that naming might be actually better :)
> 
> Tony
> 
> 

Thanks !
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index f3fdd6a..62377b5 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -725,7 +725,7 @@  static int gpmc_setup_irq(void)
 		return -EINVAL;
 
 	gpmc_irq_start = irq_alloc_descs(-1, 0, GPMC_NR_IRQ, 0);
-	if (gpmc_irq_start < 0) {
+	if ((signed)gpmc_irq_start < 0) {
 		pr_err("irq_alloc_descs failed\n");
 		return gpmc_irq_start;
 	}