Re: [PATCH] leds: Add no-op gpio_led_register_device when LED subsystem is disabled

2016-06-21 Thread Jacek Anaszewski

Hi Andrew,

This patch doesn't apply, please rebase onto recent LED tree.

On 06/21/2016 12:13 AM, Andrew F. Davis wrote:

Some systems use 'gpio_led_register_device' to make an in-memory copy of
their LED device table so the original can be removed as .init.rodata.
When the LED subsystem is not enabled source in the led directory is not
built and so this function may be undefined. Fix this here.

Signed-off-by: Andrew F. Davis <a...@ti.com>
---
  include/linux/leds.h | 8 
  1 file changed, 8 insertions(+)

diff --git a/include/linux/leds.h b/include/linux/leds.h
index d2b1306..a4a3da6 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -386,8 +386,16 @@ struct gpio_led_platform_data {
 unsigned long *delay_off);


Currently there is some stuff here, and in fact it has been for
a long time.

Patch "[PATCH 12/12] leds: Only descend into leds directory when
CONFIG_NEW_LEDS is set" also doesn't apply.
What repository are you using?


  };

+#ifdef CONFIG_NEW_LEDS
  struct platform_device *gpio_led_register_device(
 int id, const struct gpio_led_platform_data *pdata);
+#else
+static inline struct platform_device *gpio_led_register_device(
+   int id, const struct gpio_led_platform_data *pdata)
+{
+   return 0;
+}
+#endif

  enum cpu_led_event {
 CPU_LED_IDLE_START, /* CPU enters idle */




--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/12] leds: Only descend into leds directory when CONFIG_NEW_LEDS is set

2016-06-20 Thread Jacek Anaszewski

On 06/18/2016 12:46 AM, Andrew F. Davis wrote:

On 06/15/2016 01:48 AM, Jacek Anaszewski wrote:

Hi Andrew,

Thanks for the patch.

Please address the issue [1] raised by test bot and resubmit.

Thanks,
Jacek Anaszewski

[1] https://lkml.org/lkml/2016/6/13/1091



It looks like some systems use 'gpio_led_register_device' to make an
in-memory copy of their LED device table so the original can be removed
as .init.rodata. This doesn't necessarily depend on the LED subsystem
but it kind of seems useless when the rest of the subsystem is disabled.

One solution could be to use a dummy 'gpio_led_register_device' when the
subsystem is not enabled.


It sounds good. Please add a no-op version of gpio_led_register_device()
to include/leds.h, in a separate patch.

Thanks,
Jacek Anaszewski


Another is just to remove the five or so uses
of 'gpio_led_register_device' and have those systems register LED device
tables like other systems do.

If nether of these are acceptable then this patch can be dropped from
this series for now.

Thanks,
Andrew


On 06/13/2016 10:02 PM, Andrew F. Davis wrote:

When CONFIG_NEW_LEDS is not set make will still descend into the leds
directory but nothing will be built. This produces unneeded build
artifacts and messages in addition to slowing the build. Fix this here.

Signed-off-by: Andrew F. Davis <a...@ti.com>
---
   drivers/Makefile | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index 567e32c..fa514d5 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,7 +127,7 @@ obj-$(CONFIG_CPU_FREQ)+= cpufreq/
   obj-$(CONFIG_CPU_IDLE)+= cpuidle/
   obj-$(CONFIG_MMC)+= mmc/
   obj-$(CONFIG_MEMSTICK)+= memstick/
-obj-y+= leds/
+obj-$(CONFIG_NEW_LEDS)+= leds/
   obj-$(CONFIG_INFINIBAND)+= infiniband/
   obj-$(CONFIG_SGI_SN)+= sn/
   obj-y+= firmware/









--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/12] leds: Only descend into leds directory when CONFIG_NEW_LEDS is set

2016-06-15 Thread Jacek Anaszewski

Hi Andrew,

Thanks for the patch.

Please address the issue [1] raised by test bot and resubmit.

Thanks,
Jacek Anaszewski

[1] https://lkml.org/lkml/2016/6/13/1091

On 06/13/2016 10:02 PM, Andrew F. Davis wrote:

When CONFIG_NEW_LEDS is not set make will still descend into the leds
directory but nothing will be built. This produces unneeded build
artifacts and messages in addition to slowing the build. Fix this here.

Signed-off-by: Andrew F. Davis <a...@ti.com>
---
  drivers/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index 567e32c..fa514d5 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,7 +127,7 @@ obj-$(CONFIG_CPU_FREQ)  += cpufreq/
  obj-$(CONFIG_CPU_IDLE)+= cpuidle/
  obj-$(CONFIG_MMC) += mmc/
  obj-$(CONFIG_MEMSTICK)+= memstick/
-obj-y  += leds/
+obj-$(CONFIG_NEW_LEDS) += leds/
  obj-$(CONFIG_INFINIBAND)  += infiniband/
  obj-$(CONFIG_SGI_SN)  += sn/
  obj-y += firmware/




--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types

2015-09-22 Thread Jacek Anaszewski

On 09/22/2015 11:13 AM, Andrzej Hajda wrote:

On 09/21/2015 03:42 PM, David Howells wrote:

Andrzej Hajda <a.hajda-sze3o3uu22jbdgjk7y7...@public.gmane.org> wrote:


Semantic patch finds comparisons of types:
 unsigned < 0
 unsigned >= 0
The former is always false, the latter is always true.
Such comparisons are useless, so theoretically they could be
safely removed, but their presence quite often indicates bugs.


Or someone has left them in because they don't matter and there's the
possibility that the type being tested might be or become signed under some
circumstances.  If the comparison is useless, I'd expect the compiler to just
discard it - for such cases your patch is pointless.

If I have, for example:

unsigned x;

if (x == 0 || x > 27)
give_a_range_error();

I will write this as:

unsigned x;

if (x <= 0 || x > 27)
give_a_range_error();

because it that gives a way to handle x being changed to signed at some point
in the future for no cost.  In which case, your changing the <= to an ==
"because the < part of the case is useless" is arguably wrong.


This is why I have not checked for such cases - I have skipped checks of type
unsigned <= 0
exactly for the reasons above.

However I have left two other checks as they seems to me more suspicious - they
are always true or false. But as Dmitry and Andrew pointed out Linus have quite
strong opinion against removing range checks in such cases as he finds it
clearer. I think it applies to patches 29-36. I am not sure about patches 
26-28,37.


Dropped 30/38 and 31/38 from LED tree then.

--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html