Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1422671 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 12/12] leds: Only descend into leds directory when  CONFIG_NEW_LEDS is set Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-06-15 08:50 +0200
    Re: [PATCH 12/12] leds: Only descend into leds directory when  CONFIG_NEW_LEDS is set "Andrew F. Davis" <afd@ti.com> - 2016-06-18 00:50 +0200
      Re: [PATCH 12/12] leds: Only descend into leds directory when  CONFIG_NEW_LEDS is set Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-06-20 10:00 +0200
        [PATCH] leds: Add no-op gpio_led_register_device when LED subsystem  is disabled "Andrew F. Davis" <afd@ti.com> - 2016-06-21 00:30 +0200
          Re: [PATCH] leds: Add no-op gpio_led_register_device when LED  subsystem is disabled Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-06-21 09:20 +0200
            Re: [PATCH] leds: Add no-op gpio_led_register_device when LED  subsystem is disabled "Andrew F. Davis" <afd@ti.com> - 2016-06-21 14:10 +0200
              Re: [PATCH] leds: Add no-op gpio_led_register_device when LED  subsystem is disabled Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-06-21 15:20 +0200

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

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 <afd@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

[toc] | [next] | [standalone]

#1425535

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. 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 <afd@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/
>>
> 
> 

[toc] | [prev] | [next] | [standalone]

#1426299

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 <afd@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/
>>>
>>
>>
>
>

[toc] | [prev] | [next] | [standalone]

#1427104 — [PATCH] leds: Add no-op gpio_led_register_device when LED subsystem is disabled

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 <afd@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);
 };

+#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 */
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

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

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 <afd@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

[toc] | [prev] | [next] | [standalone]

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

On 06/21/2016 02:09 AM, Jacek Anaszewski wrote:
> 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 <afd@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?
> 

v4.7-rc4, it may not apply due to the surrounding lines being changed in
the other patches which may not be applied to your tree. It is a single
line change per patch so hopefully the merge conflict resolutions will
be trivial.

A better solution could have been getting an ack from each maintainer
and having someone pull the whole series into one tree, but parts have
already been picked so it may be a little late for that.

>>   };
>>
>> +#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 */
>>
> 
> 

[toc] | [prev] | [next] | [standalone]

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

On 06/21/2016 01:48 PM, Andrew F. Davis wrote:
> On 06/21/2016 02:09 AM, Jacek Anaszewski wrote:
>> 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 <afd@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?
>>
>
> v4.7-rc4, it may not apply due to the surrounding lines being changed in
> the other patches which may not be applied to your tree. It is a single
> line change per patch so hopefully the merge conflict resolutions will
> be trivial.
>
> A better solution could have been getting an ack from each maintainer
> and having someone pull the whole series into one tree, but parts have
> already been picked so it may be a little late for that.

OK, I resolved the issues and applied, thanks.

>>>    };
>>>
>>> +#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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web