Re: [PATCH v3 0/7] Remove unneeded build directory traversals

2017-03-15 Thread Andrew F. Davis
On 03/15/2017 04:03 PM, Arnd Bergmann wrote:
> On Wed, Mar 15, 2017 at 5:37 PM, Andrew F. Davis  wrote:
>> Hello all,
>>
>> I was building a kernel for x86 and noticed Make still descended into
>> directories like drivers/gpu/drm/hisilicon, this seems kind of odd given
>> nothing will be built here. It looks to be due to some directories being
>> included in obj-y unconditionally instead of only when the relevant
>> CONFIG_ is set.
>>
>> These patches are split by subsystem in-case, for some reason, a file in
>> a directory does need to be built, I believe I have checked for all
>> instances of this, but a quick review from some maintainers would be nice.
> 
> I didn't see anything wrong with the patches, and made sure that there
> are no tristate symbols controlling the subdirectory for anything that
> requires a built-in driver (which would cause a link failure).
> 
> I'm not sure about drivers/lguest, which has some special magic
> in its Makefile, it's possible that this now fails with CONFIG_LGUEST=m.
> 

lguest and mmc are the strange ones, so I put them last in the series in
case they did need to be dropped.

lguest was supposed to have been taken from v1:
https://lkml.org/lkml/2016/6/20/1086
but it looks like it didn't so I re-introduced it for v3.

mmc caught some 0-day build warnings but I never got to the bottom of them.

Anyway, I have no problem with these two being held back until the magic
in their Makefile is sorted out.

Thanks,
Andrew

>   Arnd
> 


Re: [PATCH v2 6/6] [media] Only descend into directory when CONFIG_MEDIA_SUPPORT is set

2017-01-05 Thread Andrew F. Davis
On 01/05/2017 03:42 PM, Arnd Bergmann wrote:
> On Thursday, January 5, 2017 3:01:58 PM CET Andrew F. Davis wrote:
>> @@ -109,7 +109,8 @@ obj-$(CONFIG_SERIO) += input/serio/
>>  obj-$(CONFIG_GAMEPORT) += input/gameport/
>>  obj-$(CONFIG_INPUT)+= input/
>>  obj-$(CONFIG_RTC_LIB)  += rtc/
>> -obj-y  += i2c/ media/
>> +obj-y  += i2c/
>> +obj-$(CONFIG_MEDIA_SUPPORT)+= media/
>>  obj-$(CONFIG_PPS)  += pps/
>>  obj-y  += ptp/
>>  obj-$(CONFIG_W1)   += w1/
>>
> 
> This one seems wrong: if CONFIG_MEDIA_SUPPORT=m, but some I2C drivers
> inside of drivers/media/ are built-in, we will fail to enter the directory,
> see drivers/media/Makefile.

Not sure if I see this, it looks like everything in drivers/media/
depends on CONFIG_MEDIA_SUPPORT (directly or indirectly). If
CONFIG_MEDIA_SUPPORT is =m then all dependents should be locked out of
being built-in.

Any bool symbol that controls compilation of source that depends on a
tristate symbol is broken and should be fixed anyway.

> 
> I checked the other five patches in the series as well, they all look
> ok to me.
> 
>   Arnd
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-06-21 Thread Andrew F. Davis
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 
>> ---
>>   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 */
>>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-06-20 Thread Andrew F. Davis
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 
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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-17 Thread Andrew F. Davis
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 
>> ---
>>   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-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] hsi: Build hsi_boardinfo.c into hsi core if enabled

2016-06-14 Thread Andrew F. Davis
If the HSI core is built as a module hsi_boardinfo may still
be built-in as its Kconfig type is bool, which can cause build
issues. Fix this by building this code into the HSI core when
enabled.

Reported-by: kbuild test robot 
Signed-off-by: Andrew F. Davis 
---
This build error seems to be due to Kconfig symbol CONFIG_HSI_BOARDINFO
being a bool but depending on a tristate (CONFIG_HSI). This is normally
okay when it is just a flag to enable a feature in source, but the
helper code file hsi_boardinfo.c is built as a separate entity when
enabled. This patch is probably how it was intended, and is more like
how others do this kind of thing.

This patch should be applied before the parent patch:

 drivers/hsi/Makefile  | 3 ++-
 drivers/hsi/{hsi.c => hsi_core.c} | 0
 2 files changed, 2 insertions(+), 1 deletion(-)
 rename drivers/hsi/{hsi.c => hsi_core.c} (100%)

diff --git a/drivers/hsi/Makefile b/drivers/hsi/Makefile
index 360371e..9694478 100644
--- a/drivers/hsi/Makefile
+++ b/drivers/hsi/Makefile
@@ -1,7 +1,8 @@
 #
 # Makefile for HSI
 #
-obj-$(CONFIG_HSI_BOARDINFO)+= hsi_boardinfo.o
 obj-$(CONFIG_HSI)  += hsi.o
+hsi-objs   := hsi_core.o
+hsi-$(CONFIG_HSI_BOARDINFO)+= hsi_boardinfo.o
 obj-y  += controllers/
 obj-y  += clients/
diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi_core.c
similarity index 100%
rename from drivers/hsi/hsi.c
rename to drivers/hsi/hsi_core.c
-- 
2.8.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html