Skip to contentSkip to conversation

ARM: plat-versatile: convert LEDs to platform device

PATCH In review Linus Walleij 4 participants 10 messages Feb 27, 2014 – Mar 27, 2014

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 ARM: plat-versatile: convert LEDs to platform device

The LEDs were initialized unconditionally with an fs_initcall() which doesn't play well with multiplatform. Convert the driver to a platform device and convert all boards with these LEDs to register a platform device and pass the register as a resource instead.

Tested successfully on the Versatile/AB and RealView PB1176.

Cc Bryan Wu <[email protected]>
Cc Richard Purdie <[email protected]>
Cc Russell King <[email protected]>
Cc Pawel Moll <[email protected]>
---
 arch/arm/mach-realview/core.c            | 15 ++++++++++
 arch/arm/mach-realview/core.h            |  1 +
 arch/arm/mach-realview/realview_eb.c     |  1 +
 arch/arm/mach-realview/realview_pb1176.c |  1 +
 arch/arm/mach-realview/realview_pb11mp.c |  1 +
 arch/arm/mach-realview/realview_pba8.c   |  1 +
 arch/arm/mach-realview/realview_pbx.c    |  1 +
 arch/arm/mach-versatile/core.c           | 16 +++++++++++
 arch/arm/plat-versatile/leds.c           | 49 ++++++++++++++++++--------------
 9 files changed, 65 insertions(+), 21 deletions(-)
arch/arm/mach-realview/core.c +15 -0
index 1d5ee5c9a1dc..960b8dd78c44 100644
@@ -148,6 +148,21 @@ struct platform_device realview_cf_device = {
 	},
 };
 
+static struct resource realview_leds_resources[] = {
+	{
+		.start	= REALVIEW_SYS_BASE + REALVIEW_SYS_LED_OFFSET,
+		.end	= REALVIEW_SYS_BASE + REALVIEW_SYS_LED_OFFSET + 4,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+struct platform_device realview_leds_device = {
+	.name		= "versatile-leds",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(realview_leds_resources),
+	.resource	= realview_leds_resources,
+};
+
 static struct resource realview_i2c_resource = {
 	.start		= REALVIEW_I2C_BASE,
 	.end		= REALVIEW_I2C_BASE + SZ_4K - 1,
arch/arm/mach-realview/core.h +1 -0
index 602ca5ec52c5..13dc830ef469 100644
@@ -37,6 +37,7 @@ struct machine_desc;
 
 extern struct platform_device realview_flash_device;
 extern struct platform_device realview_cf_device;
+extern struct platform_device realview_leds_device;
 extern struct platform_device realview_i2c_device;
 extern struct mmci_platform_data realview_mmc0_plat_data;
 extern struct mmci_platform_data realview_mmc1_plat_data;
arch/arm/mach-realview/realview_eb.c +1 -0
index c85ddb2a0ad0..6bb070e80128 100644
@@ -452,6 +452,7 @@ static void __init realview_eb_init(void)
 	realview_flash_register(&realview_eb_flash_resource, 1);
 	platform_device_register(&realview_i2c_device);
 	platform_device_register(&char_lcd_device);
+	platform_device_register(&realview_leds_device);
 	eth_device_register();
 	realview_usb_register(realview_eb_isp1761_resources);
 
arch/arm/mach-realview/realview_pb1176.c +1 -0
index c5eade76461b..173f2c15de49 100644
@@ -367,6 +367,7 @@ static void __init realview_pb1176_init(void)
 	realview_usb_register(realview_pb1176_isp1761_resources);
 	platform_device_register(&pmu_device);
 	platform_device_register(&char_lcd_device);
+	platform_device_register(&realview_leds_device);
 
 	for (i = 0; i < ARRAY_SIZE(amba_devs); i++) {
 		struct amba_device *d = amba_devs[i];
arch/arm/mach-realview/realview_pb11mp.c +1 -0
index f4b0962578fe..bde7e6b1fd44 100644
@@ -347,6 +347,7 @@ static void __init realview_pb11mp_init(void)
 	realview_eth_register(NULL, realview_pb11mp_smsc911x_resources);
 	platform_device_register(&realview_i2c_device);
 	platform_device_register(&realview_cf_device);
+	platform_device_register(&realview_leds_device);
 	realview_usb_register(realview_pb11mp_isp1761_resources);
 	platform_device_register(&pmu_device);
 
arch/arm/mach-realview/realview_pba8.c +1 -0
index 10a3e1d76891..4e57a8599265 100644
@@ -289,6 +289,7 @@ static void __init realview_pba8_init(void)
 	realview_eth_register(NULL, realview_pba8_smsc911x_resources);
 	platform_device_register(&realview_i2c_device);
 	platform_device_register(&realview_cf_device);
+	platform_device_register(&realview_leds_device);
 	realview_usb_register(realview_pba8_isp1761_resources);
 	platform_device_register(&pmu_device);
 
arch/arm/mach-realview/realview_pbx.c +1 -0
index 9d75493e3f0c..72c96caebefa 100644
@@ -385,6 +385,7 @@ static void __init realview_pbx_init(void)
 	realview_eth_register(NULL, realview_pbx_smsc911x_resources);
 	platform_device_register(&realview_i2c_device);
 	platform_device_register(&realview_cf_device);
+	platform_device_register(&realview_leds_device);
 	realview_usb_register(realview_pbx_isp1761_resources);
 
 	for (i = 0; i < ARRAY_SIZE(amba_devs); i++) {
arch/arm/mach-versatile/core.c +16 -0
index a335126ae18f..b31878570a00 100644
@@ -310,6 +310,21 @@ static struct platform_device char_lcd_device = {
 	.resource       =       char_lcd_resources,
 };
 
+static struct resource leds_resources[] = {
+	{
+		.start	= VERSATILE_SYS_BASE + VERSATILE_SYS_LED_OFFSET,
+		.end	= VERSATILE_SYS_BASE + VERSATILE_SYS_LED_OFFSET + 4,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device leds_device = {
+	.name		= "versatile-leds",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(leds_resources),
+	.resource	= leds_resources,
+};
+
 /*
  * Clock handling
  */
@@ -795,6 +810,7 @@ void __init versatile_init(void)
 	platform_device_register(&versatile_i2c_device);
 	platform_device_register(&smc91x_device);
 	platform_device_register(&char_lcd_device);
+	platform_device_register(&leds_device);
 
 	for (i = 0; i < ARRAY_SIZE(amba_devs); i++) {
 		struct amba_device *d = amba_devs[i];
arch/arm/plat-versatile/leds.c +28 -21

94 lines not on this page · raw message

Signature · 1 line
1.8.5.3
#1 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

Cc Bryan Wu <[email protected]>
Cc Richard Purdie <[email protected]>
Cc Russell King <[email protected]>
Cc Pawel Moll <[email protected]>

---
Bryan: can you provide an ACK for this patch so I can funnel this through the ARM SoC tree?

---
 arch/arm/plat-versatile/Kconfig  |   6 ---
 arch/arm/plat-versatile/Makefile |   1 -
 arch/arm/plat-versatile/leds.c   | 110 ---------------------------------------
 drivers/leds/Kconfig             |   8 +++
 drivers/leds/Makefile            |   1 +
 drivers/leds/leds-versatile.c    | 110 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 119 insertions(+), 117 deletions(-)
 delete mode 100644 arch/arm/plat-versatile/leds.c
 create mode 100644 drivers/leds/leds-versatile.c
arch/arm/plat-versatile/Kconfig +0 -6
index 2c4332b9f948..fce41e93b6a4 100644
@@ -6,12 +6,6 @@ config PLAT_VERSATILE_CLOCK
 config PLAT_VERSATILE_CLCD
 	bool
 
-config PLAT_VERSATILE_LEDS
-	def_bool y if NEW_LEDS
-	depends on ARCH_REALVIEW || ARCH_VERSATILE
-	select LEDS_CLASS
-	select LEDS_TRIGGERS
-
 config PLAT_VERSATILE_SCHED_CLOCK
 	def_bool y
 
arch/arm/plat-versatile/Makefile +0 -1
index f88d448b629c..2e0c472958ae 100644
@@ -2,6 +2,5 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include
 
 obj-$(CONFIG_PLAT_VERSATILE_CLOCK) += clock.o
 obj-$(CONFIG_PLAT_VERSATILE_CLCD) += clcd.o
-obj-$(CONFIG_PLAT_VERSATILE_LEDS) += leds.o
 obj-$(CONFIG_PLAT_VERSATILE_SCHED_CLOCK) += sched-clock.o
 obj-$(CONFIG_SMP) += headsmp.o platsmp.o
arch/arm/plat-versatile/leds.c deleted +0 -110

110 lines not on this page · raw message

drivers/leds/Kconfig +8 -0
index 72156c123033..93235f7378ba 100644
@@ -487,6 +487,14 @@ config LEDS_BLINKM
 	  This option enables support for the BlinkM RGB LED connected
 	  through I2C. Say Y to enable support for the BlinkM LED.
 
+config LEDS_VERSATILE
+	bool "LED support for the ARM Versatile and RealView"
+	depends on ARCH_REALVIEW || ARCH_VERSATILE
+	depends on LEDS_CLASS
+	help
+	  This option enabled support for the LEDs on the ARM Versatile
+	  and RealView boards. Say Y to enabled these.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
drivers/leds/Makefile +1 -0
index 3cd76dbd9be2..8b4c956e11ba 100644
@@ -54,6 +54,7 @@ obj-$(CONFIG_LEDS_ASIC3)		+= leds-asic3.o
 obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
 obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
+obj-$(CONFIG_LEDS_VERSATILE)		+= leds-versatile.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o
drivers/leds/leds-versatile.c new file +110 -0

110 lines not on this page · raw message

Signature · 1 line
1.8.5.3
#2 lore raw

Re: [PATCH 2/3] ARM/leds: move ARM Versatile LED driver to leds subsystem

???????, 27 ??????? 2014, 14:58 +01:00 ?? Linus Walleij <[email protected]>:

Quoted 3 lines

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

In fact, it can be converted to use basic-mmio-gpio => leds-gpio.

---

#3 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

Quoted 6 lines · Alexander Shiyan

???????, 27 ??????? 2014, 14:58 +01:00 ?? Linus Walleij <[email protected]>:

Quoted 3 lines

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

In fact, it can be converted to use basic-mmio-gpio => leds-gpio.

Hm, yeah I see what you mean. However this register is not described as a GPIO register, and on all Versatile/RealView boards these signals are soldered to LEDs, so they are not general purpose at all.

On some systems without "real" GPIO the above would lead to compiling in the entire gpiolib (149 KB) just to do this.

I would agree more with inventing something like leds-mmio as a separate refactoring after this, i.e. a driver for any memory-mapped LED, which should cover a few cases. What do you think about this idea?

Yours,
Linus Walleij

#4 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

Quoted 12 lines · Linus Walleij
Quoted 6 lines · Alexander Shiyan

???????, 27 ??????? 2014, 14:58 +01:00 ?? Linus Walleij <[email protected]>:

Quoted 3 lines

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

In fact, it can be converted to use basic-mmio-gpio => leds-gpio.

Hm, yeah I see what you mean. However this register is not described as a GPIO register, and on all Versatile/RealView boards these signals are soldered to LEDs, so they are not general purpose at all.

Quoted 7 lines

On some systems without "real" GPIO the above would lead to compiling in the entire gpiolib (149 KB) just to do this.

I would agree more with inventing something like leds-mmio as a separate refactoring after this, i.e. a driver for any memory-mapped LED, which should cover a few cases. What do you think about this idea?

Something like this?
http://thread.gmane.org/gmane.linux.kernel/1387110 ;-)

But seriously speaking, I did exactly what Alexander mentioned on VE, considering the SYS_LED & alikes as "pseudo-gpios".

Pawel

#5 lore raw

Re: [PATCH 2/3] ARM/leds: move ARM Versatile LED driver to leds subsystem

???????, 11 ????? 2014, 12:23 UTC ?? Pawel Moll <[email protected]>:

Quoted 21 lines · Linus Walleij
Quoted 12 lines · Alexander Shiyan
Quoted 6 lines

???????, 27 ??????? 2014, 14:58 +01:00 ?? Linus Walleij <[email protected]>:

Quoted 3 lines

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

In fact, it can be converted to use basic-mmio-gpio => leds-gpio.

Hm, yeah I see what you mean. However this register is not described as a GPIO register, and on all Versatile/RealView boards these signals are soldered to LEDs, so they are not general purpose at all.

Quoted 7 lines

On some systems without "real" GPIO the above would lead to compiling in the entire gpiolib (149 KB) just to do this.

I would agree more with inventing something like leds-mmio as a separate refactoring after this, i.e. a driver for any memory-mapped LED, which should cover a few cases. What do you think about this idea?

Quoted 5 lines

Something like this?
http://thread.gmane.org/gmane.linux.kernel/1387110 ;-)

But seriously speaking, I did exactly what Alexander mentioned on VE, considering the SYS_LED & alikes as "pseudo-gpios".

It turns out everything has already been invented :)

---

#6 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

In reply to Pawel Moll
Quoted 5 lines · Pawel Moll
Quoted 3 lines · Linus Walleij

On some systems without "real" GPIO the above would lead to compiling in the entire gpiolib (149 KB) just to do this.

Quoted 7 lines
Quoted 4 lines

I would agree more with inventing something like leds-mmio as a separate refactoring after this, i.e. a driver for any memory-mapped LED, which should cover a few cases. What do you think about this idea?

Something like this?
http://thread.gmane.org/gmane.linux.kernel/1387110 ;-)

Yes.

The current GPIO maintainer is not aligned with the statements that this is "some kind of GPIO".

Also the fact that this adds some hundred KB of code just to do this simple thing wasn't brought up.

Quoted 2 lines

But seriously speaking, I did exactly what Alexander mentioned on VE, considering the SYS_LED & alikes as "pseudo-gpios".

I just don't agree with that. They aren't GPIO's until used as such, and push in a huge abstraction layer inbetween the driver and its hardware for no good reason whatsoever I think. This is not keeping things simple, it is making things obscure.

Yours,
Linus Walleij

#7 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

In reply to Linus Walleij
Quoted 4 lines · Linus Walleij

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

Quoted 8 lines
Cc Bryan Wu <[email protected]>
Cc Richard Purdie <[email protected]>
Cc Russell King <[email protected]>
Cc Pawel Moll <[email protected]>

---
Bryan: can you provide an ACK for this patch so I can funnel this through the ARM SoC tree?

Ping on this.

Yours,
Linus Walleij

#8 lore raw

PATCH 2/3 ARM/leds: move ARM Versatile LED driver to leds subsystem

Quoted 15 lines · Linus Walleij
Quoted 4 lines · Linus Walleij

Now that we have converted this driver to a real platform device module-based thing, we move the driver down into the LEDs subsystem and rename the config option to LEDS_VERSATILE.

Quoted 8 lines
Cc Bryan Wu <[email protected]>
Cc Richard Purdie <[email protected]>
Cc Russell King <[email protected]>
Cc Pawel Moll <[email protected]>

---
Bryan: can you provide an ACK for this patch so I can funnel this through the ARM SoC tree?

Quoted 2 lines

Ping on this.

Apologize for this delay, this patch email didn't show up in my inbox. Please go ahead with my ack

#9 lore raw

PATCH 3/3 ARM: plat-versatile: update defconfigs for Versatile LEDs

In reply to Linus Walleij

As we moved the Versatile LEDs to the LEDs subsystem and made it a module, the config options are no more default-selected. Make sure all users still experience LEDs properly by updating all RealView and Versatile defconfigs.

Cc Bryan Wu <[email protected]>
Cc Richard Purdie <[email protected]>
Cc Russell King <[email protected]>
Cc Pawel Moll <[email protected]>
---
 arch/arm/configs/realview-smp_defconfig | 2 ++
 arch/arm/configs/realview_defconfig     | 2 ++
 arch/arm/configs/versatile_defconfig    | 3 +++
 3 files changed, 7 insertions(+)
arch/arm/configs/realview-smp_defconfig +2 -0
index abe61bf379d2..1da5d9e48224 100644
@@ -76,8 +76,10 @@ CONFIG_MMC=y
 CONFIG_MMC_ARMMMCI=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_VERSATILE=y
 CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_CPU=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_DS1307=y
 CONFIG_RTC_DRV_PL031=y
arch/arm/configs/realview_defconfig +2 -0
index 7079cbe898a8..d02e9d911bb7 100644
@@ -75,8 +75,10 @@ CONFIG_MMC=y
 CONFIG_MMC_ARMMMCI=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_VERSATILE=y
 CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_CPU=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_DS1307=y
 CONFIG_RTC_DRV_PL031=y
arch/arm/configs/versatile_defconfig +3 -0
index 073541a50e23..d52b4ffe2012 100644
@@ -61,6 +61,9 @@ CONFIG_SND_ARMAACI=m
 CONFIG_MMC=y
 CONFIG_MMC_ARMMMCI=m
 CONFIG_NEW_LEDS=y
+CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_VERSATILE=y
+CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_LEDS_TRIGGER_CPU=y
 CONFIG_EXT2_FS=y
Signature · 1 line
1.8.5.3