Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-03-01 Thread Michael Ellerman
Mathieu Malaterre  writes:

> On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
>> Mathieu Malaterre  writes:
>>
>>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>>> array feature_properties is defined as an empty array, which in turn
>>> triggers the following warning (treated as error on W=1):
>>>
>>>   CC  arch/powerpc/kernel/prom.o
>>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>>> < 0 is always false [-Werror=type-limits]
>>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> ^
>>> cc1: all warnings being treated as errors
>>
>> Ugh, that's annoying.
>>
>> This seems to work?
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 4dffef947b8a..5215119e249c 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
>> node)
>>
>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -   unsigned long i;
>> struct feature_property *fp = feature_properties;
>> const __be32 *prop;
>> +   int i;
>>
>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> prop = of_get_flat_dt_prop(node, fp->name, NULL);
>> if (prop && be32_to_cpup(prop) >= fp->min_value) {
>> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>>
>
> Indeed that looks like the less invasive solution, I'll re-submit.

Thanks.

> Should I resubmit the entire patch series (21 indep patches) or
> re-submit only the 3 patches that were discussed (as part of a
> different series) ?

Just resubmit the ones that need changes. You can either send them as a
new series of 3, or post each as a reply to the original patch with v2
in the subject.

cheers


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-03-01 Thread Michael Ellerman
Mathieu Malaterre  writes:

> On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
>> Mathieu Malaterre  writes:
>>
>>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>>> array feature_properties is defined as an empty array, which in turn
>>> triggers the following warning (treated as error on W=1):
>>>
>>>   CC  arch/powerpc/kernel/prom.o
>>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>>> < 0 is always false [-Werror=type-limits]
>>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> ^
>>> cc1: all warnings being treated as errors
>>
>> Ugh, that's annoying.
>>
>> This seems to work?
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 4dffef947b8a..5215119e249c 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
>> node)
>>
>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -   unsigned long i;
>> struct feature_property *fp = feature_properties;
>> const __be32 *prop;
>> +   int i;
>>
>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> prop = of_get_flat_dt_prop(node, fp->name, NULL);
>> if (prop && be32_to_cpup(prop) >= fp->min_value) {
>> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>>
>
> Indeed that looks like the less invasive solution, I'll re-submit.

Thanks.

> Should I resubmit the entire patch series (21 indep patches) or
> re-submit only the 3 patches that were discussed (as part of a
> different series) ?

Just resubmit the ones that need changes. You can either send them as a
new series of 3, or post each as a reply to the original patch with v2
in the subject.

cheers


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Mathieu Malaterre
On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
> Mathieu Malaterre  writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=1):
>>
>>   CC  arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>> < 0 is always false [-Werror=type-limits]
>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
> +   int i;
>
> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>

Indeed that looks like the less invasive solution, I'll re-submit.

Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?

Thanks


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Mathieu Malaterre
On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
> Mathieu Malaterre  writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=1):
>>
>>   CC  arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>> < 0 is always false [-Werror=type-limits]
>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
> +   int i;
>
> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>

Indeed that looks like the less invasive solution, I'll re-submit.

Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?

Thanks


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Michael Ellerman
Mathieu Malaterre  writes:

> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC  arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 
> 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> ^
> cc1: all warnings being treated as errors

Ugh, that's annoying.

This seems to work?

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..5215119e249c 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-   unsigned long i;
struct feature_property *fp = feature_properties;
const __be32 *prop;
+   int i;
 
-   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
prop = of_get_flat_dt_prop(node, fp->name, NULL);
if (prop && be32_to_cpup(prop) >= fp->min_value) {
cur_cpu_spec->cpu_features |= fp->cpu_feature;


cheers


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Michael Ellerman
Mathieu Malaterre  writes:

> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC  arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 
> 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> ^
> cc1: all warnings being treated as errors

Ugh, that's annoying.

This seems to work?

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..5215119e249c 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-   unsigned long i;
struct feature_property *fp = feature_properties;
const __be32 *prop;
+   int i;
 
-   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
prop = of_get_flat_dt_prop(node, fp->name, NULL);
if (prop && be32_to_cpup(prop) >= fp->min_value) {
cur_cpu_spec->cpu_features |= fp->cpu_feature;


cheers


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Andy Shevchenko
On Tue, Feb 27, 2018 at 10:42 PM, Segher Boessenkool
 wrote:
> On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
>> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
>> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>> >  wrote:
>>
>> > Much simpler is just add
>> >
>> > if (ARRAY_SIZE() == 0)
>> >   return;
>>
>> >> Or add in front:
>> >> if (!ARRAY_SIZE(feature_properties))
>> >> return;
>> >
>> > (not tested) I believe the compiler still go over the for() loop and
>> > will complain about the original unsigned comparison.
>>
>> Did you run tests? Did you look into object file?
>>
>> In kernel we much rely on the compiling away the code which is
>> deterministically not in use.
>> Here I'm pretty sure it will compile away entire function.
>
> It does, but it also still warns (this warning is done very early in the
> compiler pipeline).

Oh, I see.
Then the while () approach looks to me the best here.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-28 Thread Andy Shevchenko
On Tue, Feb 27, 2018 at 10:42 PM, Segher Boessenkool
 wrote:
> On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
>> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
>> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>> >  wrote:
>>
>> > Much simpler is just add
>> >
>> > if (ARRAY_SIZE() == 0)
>> >   return;
>>
>> >> Or add in front:
>> >> if (!ARRAY_SIZE(feature_properties))
>> >> return;
>> >
>> > (not tested) I believe the compiler still go over the for() loop and
>> > will complain about the original unsigned comparison.
>>
>> Did you run tests? Did you look into object file?
>>
>> In kernel we much rely on the compiling away the code which is
>> deterministically not in use.
>> Here I'm pretty sure it will compile away entire function.
>
> It does, but it also still warns (this warning is done very early in the
> compiler pipeline).

Oh, I see.
Then the while () approach looks to me the best here.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Segher Boessenkool
On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> >  wrote:
> 
> > Much simpler is just add
> >
> > if (ARRAY_SIZE() == 0)
> >   return;
> 
> >> Or add in front:
> >> if (!ARRAY_SIZE(feature_properties))
> >> return;
> >
> > (not tested) I believe the compiler still go over the for() loop and
> > will complain about the original unsigned comparison.
> 
> Did you run tests? Did you look into object file?
> 
> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.

It does, but it also still warns (this warning is done very early in the
compiler pipeline).


Segher


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Segher Boessenkool
On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> >  wrote:
> 
> > Much simpler is just add
> >
> > if (ARRAY_SIZE() == 0)
> >   return;
> 
> >> Or add in front:
> >> if (!ARRAY_SIZE(feature_properties))
> >> return;
> >
> > (not tested) I believe the compiler still go over the for() loop and
> > will complain about the original unsigned comparison.
> 
> Did you run tests? Did you look into object file?
> 
> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.

It does, but it also still warns (this warning is done very early in the
compiler pipeline).


Segher


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Mathieu Malaterre
On Tue, Feb 27, 2018 at 4:52 PM, Andy Shevchenko
 wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
>> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>>  wrote:
>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>>   return;
>
>>> Or add in front:
>>> if (!ARRAY_SIZE(feature_properties))
>>> return;
>>
>> (not tested) I believe the compiler still go over the for() loop and
>> will complain about the original unsigned comparison.
>
> Did you run tests? Did you look into object file?

The goal of this series is simply to remove warning treated as error.

I tried from home and I can still see the error using gcc 6.3.0, so
the original for() loop needs to be rewritten.

  CC  arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:301:16: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^
cc1: all warnings being treated as errors


> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.
>
> --
> With Best Regards,
> Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Mathieu Malaterre
On Tue, Feb 27, 2018 at 4:52 PM, Andy Shevchenko
 wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
>> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>>  wrote:
>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>>   return;
>
>>> Or add in front:
>>> if (!ARRAY_SIZE(feature_properties))
>>> return;
>>
>> (not tested) I believe the compiler still go over the for() loop and
>> will complain about the original unsigned comparison.
>
> Did you run tests? Did you look into object file?

The goal of this series is simply to remove warning treated as error.

I tried from home and I can still see the error using gcc 6.3.0, so
the original for() loop needs to be rewritten.

  CC  arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:301:16: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^
cc1: all warnings being treated as errors


> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.
>
> --
> With Best Regards,
> Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Andy Shevchenko
On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>  wrote:

> Much simpler is just add
>
> if (ARRAY_SIZE() == 0)
>   return;

>> Or add in front:
>> if (!ARRAY_SIZE(feature_properties))
>> return;
>
> (not tested) I believe the compiler still go over the for() loop and
> will complain about the original unsigned comparison.

Did you run tests? Did you look into object file?

In kernel we much rely on the compiling away the code which is
deterministically not in use.
Here I'm pretty sure it will compile away entire function.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-27 Thread Andy Shevchenko
On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre  wrote:
> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>  wrote:

> Much simpler is just add
>
> if (ARRAY_SIZE() == 0)
>   return;

>> Or add in front:
>> if (!ARRAY_SIZE(feature_properties))
>> return;
>
> (not tested) I believe the compiler still go over the for() loop and
> will complain about the original unsigned comparison.

Did you run tests? Did you look into object file?

In kernel we much rely on the compiling away the code which is
deterministically not in use.
Here I'm pretty sure it will compile away entire function.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Mathieu Malaterre
On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
 wrote:
>
>
> Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
>>
>> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
>>  wrote:
>>>
>>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>>>  wrote:

 On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre 
 wrote:
>>>
>>>
>   static void __init check_cpu_feature_properties(unsigned long node)
>   {
> -   unsigned long i;
>  struct feature_property *fp = feature_properties;
>  const __be32 *prop;
>

 Much simpler is just add

 if (ARRAY_SIZE() == 0)
   return;

> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (; fp != feature_properties +
> ARRAY_SIZE(feature_properties); ++fp) {
>>>
>>>
>>> ...or convert to while(), which will be more readable.
>>
>>
>> So you'd prefer something like:
>>
>> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
>>...
>>++fp;
>> }
>>
>> right ?
>>
>
>
> Why not do as suggested by Segher, ie just replace < by != in the original
> form ?

I can do that.

> Or add in front:
> if (!ARRAY_SIZE(feature_properties))
> return;

(not tested) I believe the compiler still go over the for() loop and
will complain about the original unsigned comparison.

> Christophe


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Mathieu Malaterre
On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
 wrote:
>
>
> Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
>>
>> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
>>  wrote:
>>>
>>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>>>  wrote:

 On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre 
 wrote:
>>>
>>>
>   static void __init check_cpu_feature_properties(unsigned long node)
>   {
> -   unsigned long i;
>  struct feature_property *fp = feature_properties;
>  const __be32 *prop;
>

 Much simpler is just add

 if (ARRAY_SIZE() == 0)
   return;

> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (; fp != feature_properties +
> ARRAY_SIZE(feature_properties); ++fp) {
>>>
>>>
>>> ...or convert to while(), which will be more readable.
>>
>>
>> So you'd prefer something like:
>>
>> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
>>...
>>++fp;
>> }
>>
>> right ?
>>
>
>
> Why not do as suggested by Segher, ie just replace < by != in the original
> form ?

I can do that.

> Or add in front:
> if (!ARRAY_SIZE(feature_properties))
> return;

(not tested) I believe the compiler still go over the for() loop and
will complain about the original unsigned comparison.

> Christophe


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Christophe LEROY



Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :

On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
 wrote:

On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
 wrote:

On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:



  static void __init check_cpu_feature_properties(unsigned long node)
  {
-   unsigned long i;
 struct feature_property *fp = feature_properties;
 const __be32 *prop;



Much simpler is just add

if (ARRAY_SIZE() == 0)
  return;


-   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) 
{


...or convert to while(), which will be more readable.


So you'd prefer something like:

while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
   ...
   ++fp;
}

right ?




Why not do as suggested by Segher, ie just replace < by != in the 
original form ?

Or add in front:
if (!ARRAY_SIZE(feature_properties))
return;

Christophe


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Christophe LEROY



Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :

On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
 wrote:

On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
 wrote:

On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:



  static void __init check_cpu_feature_properties(unsigned long node)
  {
-   unsigned long i;
 struct feature_property *fp = feature_properties;
 const __be32 *prop;



Much simpler is just add

if (ARRAY_SIZE() == 0)
  return;


-   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) 
{


...or convert to while(), which will be more readable.


So you'd prefer something like:

while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
   ...
   ++fp;
}

right ?




Why not do as suggested by Segher, ie just replace < by != in the 
original form ?

Or add in front:
if (!ARRAY_SIZE(feature_properties))
return;

Christophe


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Mathieu Malaterre
On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
 wrote:
> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>  wrote:
>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:
>
>>>  static void __init check_cpu_feature_properties(unsigned long node)
>>>  {
>>> -   unsigned long i;
>>> struct feature_property *fp = feature_properties;
>>> const __be32 *prop;
>>>
>>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>>  return;
>>
>>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
>>> ++fp) {
>
> ...or convert to while(), which will be more readable.

So you'd prefer something like:

while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
  ...
  ++fp;
}

right ?


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Mathieu Malaterre
On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
 wrote:
> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>  wrote:
>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:
>
>>>  static void __init check_cpu_feature_properties(unsigned long node)
>>>  {
>>> -   unsigned long i;
>>> struct feature_property *fp = feature_properties;
>>> const __be32 *prop;
>>>
>>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>>  return;
>>
>>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
>>> ++fp) {
>
> ...or convert to while(), which will be more readable.

So you'd prefer something like:

while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
  ...
  ++fp;
}

right ?


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Andy Shevchenko
On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
 wrote:
> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:

>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -   unsigned long i;
>> struct feature_property *fp = feature_properties;
>> const __be32 *prop;
>>
>
> Much simpler is just add
>
> if (ARRAY_SIZE() == 0)
>  return;
>
>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
>> ++fp) {

...or convert to while(), which will be more readable.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Andy Shevchenko
On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
 wrote:
> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:

>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -   unsigned long i;
>> struct feature_property *fp = feature_properties;
>> const __be32 *prop;
>>
>
> Much simpler is just add
>
> if (ARRAY_SIZE() == 0)
>  return;
>
>> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
>> ++fp) {

...or convert to while(), which will be more readable.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Andy Shevchenko
On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC  arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 
> 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> ^
> cc1: all warnings being treated as errors
>
> Signed-off-by: Mathieu Malaterre 
> ---
>  arch/powerpc/kernel/prom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..6e8e4122820e 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
>

Much simpler is just add

if (ARRAY_SIZE() == 0)
 return;

> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
> ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-26 Thread Andy Shevchenko
On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre  wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC  arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 
> 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> ^
> cc1: all warnings being treated as errors
>
> Signed-off-by: Mathieu Malaterre 
> ---
>  arch/powerpc/kernel/prom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..6e8e4122820e 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
>

Much simpler is just add

if (ARRAY_SIZE() == 0)
 return;

> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (; fp != feature_properties + ARRAY_SIZE(feature_properties); 
> ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-25 Thread Segher Boessenkool
Hi!

On Sun, Feb 25, 2018 at 06:22:16PM +0100, Mathieu Malaterre wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):

> - for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> + for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) 
> {

You could just write != instead of < ?  Seems more readable.

Maybe something can be done to ARRAY_SIZE to make this not warn.


Segher


Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

2018-02-25 Thread Segher Boessenkool
Hi!

On Sun, Feb 25, 2018 at 06:22:16PM +0100, Mathieu Malaterre wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):

> - for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> + for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) 
> {

You could just write != instead of < ?  Seems more readable.

Maybe something can be done to ARRAY_SIZE to make this not warn.


Segher