[v3,01/15] ARM: pxa: Don't rely on public mmc header to include leds.h

Message ID 1484313256-25993-2-git-send-email-ulf.hansson@linaro.org
State New
Headers show

Commit Message

Ulf Hansson Jan. 13, 2017, 1:14 p.m.
Some of the pxa platforms, balloon3, colibri-pxa270-income, corgi,
trizeps4, vpac270, zeus and zylonite depends on leds.h. Explicitly include
it instead of relying on the public mmc header host.h.

Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: <linux-arm-kernel@lists.infradead.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

---

I am seeking an ack for this change as following changes for mmc in the
series, has build-dependencies to it.

---
 arch/arm/mach-pxa/balloon3.c              | 1 +
 arch/arm/mach-pxa/colibri-pxa270-income.c | 1 +
 arch/arm/mach-pxa/corgi.c                 | 1 +
 arch/arm/mach-pxa/trizeps4.c              | 1 +
 arch/arm/mach-pxa/vpac270.c               | 1 +
 arch/arm/mach-pxa/zeus.c                  | 1 +
 arch/arm/mach-pxa/zylonite.c              | 1 +
 7 files changed, 7 insertions(+)

-- 
1.9.1

Comments

Robert Jarzmik Jan. 20, 2017, 8:01 a.m. | #1
Ulf Hansson <ulf.hansson@linaro.org> writes:

> Some of the pxa platforms, balloon3, colibri-pxa270-income, corgi,

> trizeps4, vpac270, zeus and zylonite depends on leds.h. Explicitly include

> it instead of relying on the public mmc header host.h.

>

> Cc: Daniel Mack <daniel@zonque.org>

> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>

> Cc: Robert Jarzmik <robert.jarzmik@free.fr>

> Cc: <linux-arm-kernel@lists.infradead.org>

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---

>

> I am seeking an ack for this change as following changes for mmc in the

> series, has build-dependencies to it.

Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>


Actually I already have a patch for arch/arm/mach-pxa/idp.c which adds this
include also in my -next tree, just in case you're tempted to modify this one,
which would conflict with my pxa tree.

An other point, I checked which files don't have a leds.h :
 - grep -ls _led arch/arm/mach-pxa/*.c > /tmp/a
 - grep -ls 'linux/leds.h' arch/arm/mach-pxa/*.c > /tmp/b
 - sdiff -s /tmp/a /tmp/b
This gives me :
	balloon3.c
	colibri-pxa270-income.c
	corgi.c
	corgi_pm.c
	em-x270.c
	ezx.c
	hx4700.c
	magician.c
	palmld.c
	palmtc.c
	palmtreo.c
	palmz72.c
	spitz_pm.c
	tosa.c
	trizeps4.c
	vpac270.c
	z2.c
	zeus.c
	zylonite.c
	zylonite_pxa300.c
	zylonite_pxa320.c

So I was wondering how you made the choice of which files you add the include to
and which you don't touch ?

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Jan. 20, 2017, 8:27 a.m. | #2
On 20 January 2017 at 09:01, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:

>

>> Some of the pxa platforms, balloon3, colibri-pxa270-income, corgi,

>> trizeps4, vpac270, zeus and zylonite depends on leds.h. Explicitly include

>> it instead of relying on the public mmc header host.h.

>>

>> Cc: Daniel Mack <daniel@zonque.org>

>> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>

>> Cc: Robert Jarzmik <robert.jarzmik@free.fr>

>> Cc: <linux-arm-kernel@lists.infradead.org>

>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

>> ---

>>

>> I am seeking an ack for this change as following changes for mmc in the

>> series, has build-dependencies to it.

> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

>

> Actually I already have a patch for arch/arm/mach-pxa/idp.c which adds this

> include also in my -next tree, just in case you're tempted to modify this one,

> which would conflict with my pxa tree.

>

> An other point, I checked which files don't have a leds.h :

>  - grep -ls _led arch/arm/mach-pxa/*.c > /tmp/a

>  - grep -ls 'linux/leds.h' arch/arm/mach-pxa/*.c > /tmp/b

>  - sdiff -s /tmp/a /tmp/b

> This gives me :

>         balloon3.c

>         colibri-pxa270-income.c

>         corgi.c

>         corgi_pm.c

>         em-x270.c

>         ezx.c

>         hx4700.c

>         magician.c

>         palmld.c

>         palmtc.c

>         palmtreo.c

>         palmz72.c

>         spitz_pm.c

>         tosa.c

>         trizeps4.c

>         vpac270.c

>         z2.c

>         zeus.c

>         zylonite.c

>         zylonite_pxa300.c

>         zylonite_pxa320.c

>

> So I was wondering how you made the choice of which files you add the include to

> and which you don't touch ?


By building the pxa_defconfig, see what errors it reports and then fix them.

Isn't pxa_defconfig building all the variants?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robert Jarzmik Jan. 20, 2017, 7:34 p.m. | #3
Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 20 January 2017 at 09:01, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

>> Ulf Hansson <ulf.hansson@linaro.org> writes:

>> So I was wondering how you made the choice of which files you add the include to

>> and which you don't touch ?

>

> By building the pxa_defconfig, see what errors it reports and then fix them.

>

> Isn't pxa_defconfig building all the variants?

Euh actually it should, but it's not.

The file was built from all _existing_ pxa defconfigs in arch/arm/configs/. The
trick is that there were many pxa boards for which no defconfig file was
available, and therefore they "escape" the pxa_defconfig coverage.

Cheers.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robert Jarzmik Jan. 24, 2017, 4:34 p.m. | #4
Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 22 January 2017 at 11:25, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

>> Robert Jarzmik <robert.jarzmik@free.fr> writes:

>>

>>> Ulf Hansson <ulf.hansson@linaro.org> writes:

>>>

>>>> On 20 January 2017 at 09:01, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

>>>>> Ulf Hansson <ulf.hansson@linaro.org> writes:

>>>>> So I was wondering how you made the choice of which files you add the include to

>>>>> and which you don't touch ?

>>>>

>>>> By building the pxa_defconfig, see what errors it reports and then fix them.

>>>>

>>>> Isn't pxa_defconfig building all the variants?

>>> Euh actually it should, but it's not.

>>

>> And actually thanks to a suggestion from Russell to improve my search line, I

>> came up with the command line bellow, and perhaps you could fix the others with

>> one command line and respin the patch ?

>>

>> Here it is :

>>

>> for f in $(grep -l _led arch/arm/mach-pxa/*.c | xargs grep -L linux/leds.h); do

>> sed -i '0,/^#include <linux\/[l-m].*>$/{s/\(#include <linux\/[l-m].*\)/#include

>> <linux\/leds.h>\n\1/}' $f; done

>>

>> Cheers.

>

> Robert, thanks for helping out!

>

> While running the above command-line, the following files becomes changed:

>

> arch/arm/mach-pxa/zylonite_pxa300.c

> arch/arm/mach-pxa/zylonite_pxa320.c

> arch/arm/mach-pxa/palmld.c

> arch/arm/mach-pxa/palmtc.c

> arch/arm/mach-pxa/ezx.c

> arch/arm/mach-pxa/em-x270.c

> arch/arm/mach-pxa/hx4700.c

> arch/arm/mach-pxa/magician.c

> arch/arm/mach-pxa/palmtreo.c

> arch/arm/mach-pxa/corgi_pm.c

> arch/arm/mach-pxa/spitz_pm.c

> arch/arm/mach-pxa/tosa.c

> arch/arm/mach-pxa/z2.c

Indeed.

> However, already by building pxa_defconfig, I have verified that these

> files are being build successfully without any changes needed.

Ha so for some (all ?) of them, another include is providing the leds.h,
interesting. For many of them it is either :
 - linux/power_supply.h
 - linux/gpio_charger.h
 - an mfd device (such as mfd/htc-pasic3.h)
 - etc ...

> Because of that, I am not sure I would like to make any additional

> changes as a part of this series. I think it's easier if you later on

> deal with that as clean-ups via your pxa tree instead, does that make

> sense to you as well?

Yeah as you wish, after all you're removing the leds.h and you'r patch addresses
that, it's beyond its scope to fix all pxa boards which don't include leds.h
properly.

Cheers.

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

Patch

diff --git a/arch/arm/mach-pxa/balloon3.c b/arch/arm/mach-pxa/balloon3.c
index 8a3c409..d452a49 100644
--- a/arch/arm/mach-pxa/balloon3.c
+++ b/arch/arm/mach-pxa/balloon3.c
@@ -17,6 +17,7 @@ 
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/sched.h>
 #include <linux/bitops.h>
 #include <linux/fb.h>
diff --git a/arch/arm/mach-pxa/colibri-pxa270-income.c b/arch/arm/mach-pxa/colibri-pxa270-income.c
index 8cff770..d7cf47d 100644
--- a/arch/arm/mach-pxa/colibri-pxa270-income.c
+++ b/arch/arm/mach-pxa/colibri-pxa270-income.c
@@ -17,6 +17,7 @@ 
 #include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/ioport.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
diff --git a/arch/arm/mach-pxa/corgi.c b/arch/arm/mach-pxa/corgi.c
index 183cd34..7270f0d 100644
--- a/arch/arm/mach-pxa/corgi.c
+++ b/arch/arm/mach-pxa/corgi.c
@@ -19,6 +19,7 @@ 
 #include <linux/major.h>
 #include <linux/fs.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/mmc/host.h>
 #include <linux/mtd/physmap.h>
 #include <linux/pm.h>
diff --git a/arch/arm/mach-pxa/trizeps4.c b/arch/arm/mach-pxa/trizeps4.c
index ea78bc5..3dd13b4 100644
--- a/arch/arm/mach-pxa/trizeps4.c
+++ b/arch/arm/mach-pxa/trizeps4.c
@@ -16,6 +16,7 @@ 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/export.h>
 #include <linux/sched.h>
 #include <linux/bitops.h>
diff --git a/arch/arm/mach-pxa/vpac270.c b/arch/arm/mach-pxa/vpac270.c
index c006ee9..70ab3ad 100644
--- a/arch/arm/mach-pxa/vpac270.c
+++ b/arch/arm/mach-pxa/vpac270.c
@@ -15,6 +15,7 @@ 
 #include <linux/irq.h>
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
+#include <linux/leds.h>
 #include <linux/gpio.h>
 #include <linux/usb/gpio_vbus.h>
 #include <linux/mtd/mtd.h>
diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
index 3b94ecf..ecbcaee 100644
--- a/arch/arm/mach-pxa/zeus.c
+++ b/arch/arm/mach-pxa/zeus.c
@@ -13,6 +13,7 @@ 
 
 #include <linux/cpufreq.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/irq.h>
 #include <linux/pm.h>
 #include <linux/gpio.h>
diff --git a/arch/arm/mach-pxa/zylonite.c b/arch/arm/mach-pxa/zylonite.c
index 3642389..4268552 100644
--- a/arch/arm/mach-pxa/zylonite.c
+++ b/arch/arm/mach-pxa/zylonite.c
@@ -16,6 +16,7 @@ 
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/interrupt.h>
+#include <linux/leds.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>