arm: ep93xx: use gpio_led_register_device
PATCH 1/1 arm: ep93xx: use gpio_led_register_device
Use gpio_led_register_device to register the two leds connected to the ep93xx.
Add a SOC_EP93XX Kconfig option for common options needed by ep93xx and use that option to select LEDS_GPIO_REGISTER.
---
arch/arm/mach-ep93xx/Kconfig | 12 ++++++++++++ arch/arm/mach-ep93xx/core.c | 16 ++++------------ 2 files changed, 16 insertions(+), 12 deletions(-)
arch/arm/mach-ep93xx/Kconfig +12 -0
@@ -2,6 +2,10 @@ if ARCH_EP93XX menu "Cirrus EP93xx Implementation Options" +config SOC_EP93XX + bool + select LEDS_GPIO_REGISTER + config CRUNCH bool "Support for MaverickCrunch" help
@@ -48,12 +52,14 @@ endchoice config MACH_ADSSPHERE bool "Support ADS Sphere" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the ADS Sphere board. config MACH_EDB93XX bool + select SOC_EP93XX config MACH_EDB9301 bool "Support Cirrus Logic EDB9301"
@@ -122,12 +128,14 @@ config MACH_EDB9315A config MACH_GESBC9312 depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET bool "Support Glomation GESBC-9312-sx" + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Glomation GESBC-9312-sx board. config MACH_MICRO9 bool + select SOC_EP93XX config MACH_MICRO9H bool "Support Contec Micro9-High"
@@ -164,6 +172,7 @@ config MACH_MICRO9S config MACH_SIM_ONE bool "Support Simplemachines Sim.One board" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Simplemachines Sim.One board.
@@ -171,6 +180,7 @@ config MACH_SIM_ONE config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Bluewater Systems Snapper CL15 Module.
@@ -178,6 +188,7 @@ config MACH_SNAPPER_CL15 config MACH_TS72XX bool "Support Technologic Systems TS-72xx SBC" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Technologic Systems TS-72xx board.
@@ -185,6 +196,7 @@ config MACH_TS72XX config MACH_VISION_EP9307 bool "Support Vision Engraving Systems EP9307 SoM" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Vision Engraving Systems EP9307 SoM.
arch/arm/mach-ep93xx/core.c +4 -12
@@ -513,7 +513,7 @@ void __init ep93xx_register_spi(struct ep93xx_spi_info *info,/************************************************************************* * EP93xx LEDs *************************************************************************/ -static struct gpio_led ep93xx_led_pins[] = { +static const struct gpio_led ep93xx_led_pins[]__initconst = { { .name ="platform:grled" , .gpio =EP93XX_GPIO_LINE_GRLED ,
@@ -523,20 +523,11 @@ static struct gpio_led ep93xx_led_pins[] = { }, }; -static struct gpio_led_platform_data ep93xx_led_data = { +static const struct gpio_led_platform_data ep93xx_led_data__initconst = { .num_leds =ARRAY_SIZE (ep93xx_led_pins), .leds = ep93xx_led_pins, }; -static struct platform_device ep93xx_leds = { - .name ="leds-gpio" , - .id =-1 , - .dev = { - .platform_data = &ep93xx_led_data, - }, -}; - -/************************************************************************* * EP93xx pwm peripheral handling *************************************************************************/
@@ -889,8 +880,9 @@ void __init ep93xx_init_devices(void)platform_device_register (&ep93xx_rtc_device);platform_device_register (&ep93xx_ohci_device); -platform_device_register (&ep93xx_leds);platform_device_register (&ep93xx_wdt_device); + +gpio_led_register_device (-1 , &ep93xx_led_data); }void ep93xx_restart (char mode,const char *cmd)
PATCH 1/1 arm: ep93xx: use gpio_led_register_device
Quoted 8 lines · H Hartley Sweeten
Use
gpio_led_register_deviceto register the two leds connected to the ep93xx.Add a
SOC_EP93XXKconfigoption for common options needed by ep93xx and use that option to selectLEDS_GPIO_REGISTER.Signed-off-by Hartley Sweeten <[email protected]>Cc Ryan Mallon <[email protected]>
Hi Hartley,
Just a couple of comments below.
~Ryan
Quoted 11 lines
---
arch/arm/mach-ep93xx/Kconfig | 12 ++++++++++++ arch/arm/mach-ep93xx/core.c | 16 ++++------------ 2 files changed, 16 insertions(+), 12 deletions(-)arch/arm/mach-ep93xx/Kconfig +0
-0
arch/arm/mach-ep93xx/Kconfig if ARCH_EP93XX · line 8
Quoted 7 lines
arch/arm/mach-ep93xx/Kconfig +4
-0@@ -2,6 +2,10 @@ if ARCH_EP93XX menu "Cirrus EP93xx Implementation Options" +config SOC_EP93XX + bool + select LEDS_GPIO_REGISTER +
So, this option is currently just used to indirectly select LEDS_GPIO_REGISTER. Do you have plans for it to select other things? Otherwise, its just a bunch of extra Kconfig lines for not much benefit.
Quoted 34 lines
arch/arm/mach-ep93xx/Kconfig +4
-0config CRUNCH bool "Support for MaverickCrunch" help@@ -48,12 +52,14 @@ endchoice config MACH_ADSSPHERE bool "Support ADS Sphere" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the ADS Sphere board. config MACH_EDB93XX bool + select SOC_EP93XX config MACH_EDB9301 bool "Support Cirrus Logic EDB9301"@@ -122,12 +128,14 @@ config MACH_EDB9315A config MACH_GESBC9312 depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET bool "Support Glomation GESBC-9312-sx" + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Glomation GESBC-9312-sx board. config MACH_MICRO9 bool + select SOC_EP93XX config MACH_MICRO9H bool "Support Contec Micro9-High"
arch/arm/mach-ep93xx/Kconfig config MACH_MICRO9S · line 175
Quoted 4 lines
arch/arm/mach-ep93xx/Kconfig +1
-0@@ -164,6 +172,7 @@ config MACH_MICRO9S config MACH_SIM_ONE bool "Support Simplemachines Sim.One board" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX
The existing whitespace here is using spaces instead of tabs. If the result looks terrible (not aligned) then we should maybe do a separate patch to clean up the crappy whitespace.
Quoted 32 lines
arch/arm/mach-ep93xx/Kconfig +3
-0help Say 'Y' here if you want your kernel to support the Simplemachines Sim.One board.@@ -171,6 +180,7 @@ config MACH_SIM_ONE config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Bluewater Systems Snapper CL15 Module.@@ -178,6 +188,7 @@ config MACH_SNAPPER_CL15 config MACH_TS72XX bool "Support Technologic Systems TS-72xx SBC" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Technologic Systems TS-72xx board.@@ -185,6 +196,7 @@ config MACH_TS72XX config MACH_VISION_EP9307 bool "Support Vision Engraving Systems EP9307 SoM" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Vision Engraving Systems EP9307 SoM.arch/arm/mach-ep93xx/core.c +0
-0
arch/arm/mach-ep93xx/core.c void __init ep93xx_register_spi(struct ep93xx_spi_info *info, · line 516
Quoted 5 lines
arch/arm/mach-ep93xx/core.c +1
-1@@ -513,7 +513,7 @@ void __init ep93xx_register_spi(struct ep93xx_spi_info *info,/************************************************************************* * EP93xx LEDs *************************************************************************/ -static struct gpio_led ep93xx_led_pins[] = { +static const struct gpio_led ep93xx_led_pins[]__initconst = {
This fix, and related changes are not mentioned in the changelog.
Quoted 36 lines
arch/arm/mach-ep93xx/core.c +3
-11{ .name = "platform:grled", .gpio = EP93XX_GPIO_LINE_GRLED,@@ -523,20 +523,11 @@ static struct gpio_led ep93xx_led_pins[] = { }, }; -static struct gpio_led_platform_data ep93xx_led_data = { +static const struct gpio_led_platform_data ep93xx_led_data__initconst = { .num_leds =ARRAY_SIZE (ep93xx_led_pins), .leds = ep93xx_led_pins, }; -static struct platform_device ep93xx_leds = { - .name ="leds-gpio" , - .id =-1 , - .dev = { - .platform_data = &ep93xx_led_data, - }, -}; - -/************************************************************************* * EP93xx pwm peripheral handling *************************************************************************/ @@ -889,8 +880,9 @@ void __init ep93xx_init_devices(void)platform_device_register (&ep93xx_rtc_device);platform_device_register (&ep93xx_ohci_device); -platform_device_register (&ep93xx_leds);platform_device_register (&ep93xx_wdt_device); + +gpio_led_register_device (-1 , &ep93xx_led_data); }void ep93xx_restart (char mode,const char *cmd)
PATCH 1/1 arm: ep93xx: use gpio_led_register_device
Quoted 31 lines · Ryan Mallon
Quoted 8 lines · H Hartley Sweeten
Use
gpio_led_register_deviceto register the two leds connected to the ep93xx.Add a
SOC_EP93XXKconfigoption for common options needed by ep93xx and use that option to selectLEDS_GPIO_REGISTER.Signed-off-by Hartley Sweeten <[email protected]>Cc Ryan Mallon <[email protected]>Hi Hartley,
Just a couple of comments below.
~Ryan
Quoted 14 lines
---
arch/arm/mach-ep93xx/Kconfig | 12 ++++++++++++ arch/arm/mach-ep93xx/core.c | 16 ++++------------ 2 files changed, 16 insertions(+), 12 deletions(-)arch/arm/mach-ep93xx/Kconfig +0
-0@@ -2,6 +2,10 @@ if ARCH_EP93XX menu "Cirrus EP93xx Implementation Options"
Quoted 9 lines
arch/arm/mach-ep93xx/Kconfig if ARCH_EP93XX · line 8
Quoted 4 lines
arch/arm/mach-ep93xx/Kconfig +4
-0+config SOC_EP93XX + bool + select LEDS_GPIO_REGISTER +So, this option is currently just used to indirectly select
LEDS_GPIO_REGISTER. Do you have plans for it to select other things?Quoted 1 line
Otherwise, its just a bunch of extra
Kconfiglines for not much benefit.
Yes, this option will be used to indirectly select common "generic" options for the ep93xx. I think this is cleaner than putting them directly under ARCH_EP93XX in arch/arm/Kconfig.
EP93XX specific options are already handled in the various subsystems with the "depends on ARCH_EP93XX", but for generic stuff we would need to either update the defconfig or rely on the user to select the options.
Currently, with the single option being selected, it is a bit of overkill. But as more options (hopefully) get added it should be a benefit.
Quoted 31 lines
Quoted 31 lines
config CRUNCH
arch/arm/mach-ep93xx/Kconfig +4
-0bool "Support for MaverickCrunch" help@@ -48,12 +52,14 @@ endchoice config MACH_ADSSPHERE bool "Support ADS Sphere" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the ADS Sphere board. config MACH_EDB93XX bool + select SOC_EP93XX config MACH_EDB9301 bool "Support Cirrus Logic EDB9301"@@ -122,12 +128,14 @@ config MACH_EDB9315A config MACH_GESBC9312 depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET bool "Support Glomation GESBC-9312-sx" + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Glomation GESBC-9312-sx board. config MACH_MICRO9 bool + select SOC_EP93XX
Quoted 12 lines
arch/arm/mach-ep93xx/Kconfig config MACH_MICRO9S · line 175
Quoted 7 lines
arch/arm/mach-ep93xx/Kconfig +1
-0config MACH_MICRO9H bool "Support Contec Micro9-High"@@ -164,6 +172,7 @@ config MACH_MICRO9S config MACH_SIM_ONE bool "Support Simplemachines Sim.One board" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XXThe existing whitespace here is using spaces instead of tabs. If the result looks terrible (not aligned) then we should maybe do a separate patch to clean up the crappy whitespace.
I noticed that also... Hmm... who used the spaces ;-)
I agree, a separate patch should clean up the shitespace.
Quoted 32 lines
Quoted 32 lines
arch/arm/mach-ep93xx/Kconfig +3
-0help Say 'Y' here if you want your kernel to support the Simplemachines Sim.One board.@@ -171,6 +180,7 @@ config MACH_SIM_ONE config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Bluewater Systems Snapper CL15 Module.@@ -178,6 +188,7 @@ config MACH_SNAPPER_CL15 config MACH_TS72XX bool "Support Technologic Systems TS-72xx SBC" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Technologic Systems TS-72xx board.@@ -185,6 +196,7 @@ config MACH_TS72XX config MACH_VISION_EP9307 bool "Support Vision Engraving Systems EP9307 SoM" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Vision Engraving Systems EP9307 SoM.arch/arm/mach-ep93xx/core.c +0
-0@@ -513,7 +513,7 @@ void __init ep93xx_register_spi(struct ep93xx_spi_info *info,
Quoted 8 lines
arch/arm/mach-ep93xx/core.c void __init ep93xx_register_spi(struct ep93xx_spi_info *info, · line 516
Quoted 5 lines
arch/arm/mach-ep93xx/core.c +1
-1/************************************************************************* * EP93xx LEDs *************************************************************************/ -static struct gpio_led ep93xx_led_pins[] = { +static const struct gpio_led ep93xx_led_pins[]__initconst = {This fix, and related changes are not mentioned in the changelog.
Sorry about that. From include/Linux/leds.h:
struct platform_device *gpio_led_register_device(
int id, const struct gpio_led_platform_data *pdata);
Since pdata needs to be const I changed the two relevant static variables to const. And, since nothing should modify them I also made them __initconst. If you feel this needs to be mentioned in the changelog I will resubmit the patch.
Quoted 36 lines
Quoted 36 lines
arch/arm/mach-ep93xx/core.c +3
-11{ .name = "platform:grled", .gpio = EP93XX_GPIO_LINE_GRLED,@@ -523,20 +523,11 @@ static struct gpio_led ep93xx_led_pins[] = { }, }; -static struct gpio_led_platform_data ep93xx_led_data = { +static const struct gpio_led_platform_data ep93xx_led_data__initconst = { .num_leds =ARRAY_SIZE (ep93xx_led_pins), .leds = ep93xx_led_pins, }; -static struct platform_device ep93xx_leds = { - .name ="leds-gpio" , - .id =-1 , - .dev = { - .platform_data = &ep93xx_led_data, - }, -}; - -/************************************************************************* * EP93xx pwm peripheral handling *************************************************************************/ @@ -889,8 +880,9 @@ void __init ep93xx_init_devices(void)platform_device_register (&ep93xx_rtc_device);platform_device_register (&ep93xx_ohci_device); -platform_device_register (&ep93xx_leds);platform_device_register (&ep93xx_wdt_device); + +gpio_led_register_device (-1 , &ep93xx_led_data); }
Quoted 1 line
arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 888
Quoted 1 line
arch/arm/mach-ep93xx/core.c +0
-0void ep93xx_restart (char mode,const char *cmd)
Regards,
Hartley
PATCH 1/1 arm: ep93xx: use gpio_led_register_device
Quoted 46 lines · H Hartley Sweeten
Quoted 31 lines · Ryan Mallon
Quoted 8 lines · H Hartley Sweeten
Use
gpio_led_register_deviceto register the two leds connected to the ep93xx.Add a
SOC_EP93XXKconfigoption for common options needed by ep93xx and use that option to selectLEDS_GPIO_REGISTER.Signed-off-by Hartley Sweeten <[email protected]>Cc Ryan Mallon <[email protected]>Hi Hartley,
Just a couple of comments below.
~Ryan
Quoted 14 lines
---
arch/arm/mach-ep93xx/Kconfig | 12 ++++++++++++ arch/arm/mach-ep93xx/core.c | 16 ++++------------ 2 files changed, 16 insertions(+), 12 deletions(-)arch/arm/mach-ep93xx/Kconfig +0
-0@@ -2,6 +2,10 @@ if ARCH_EP93XX menu "Cirrus EP93xx Implementation Options"Quoted 9 lines
arch/arm/mach-ep93xx/Kconfig if ARCH_EP93XX · line 8
Quoted 4 lines
arch/arm/mach-ep93xx/Kconfig +4
-0+config SOC_EP93XX + bool + select LEDS_GPIO_REGISTER +So, this option is currently just used to indirectly select
LEDS_GPIO_REGISTER. Do you have plans for it to select other things?Quoted 1 line
Otherwise, its just a bunch of extra
Kconfiglines for not much benefit.Yes, this option will be used to indirectly select common "generic" options for the ep93xx. I think this is cleaner than putting them directly under
ARCH_EP93XXinarch/arm/Kconfig.
Quoted 6 lines
EP93XX specific options are already handled in the various subsystems with the "depends on
ARCH_EP93XX", but for generic stuff we would need to either update the defconfig or rely on the user to select the options.Currently, with the single option being selected, it is a bit of overkill. But as more options (hopefully) get added it should be a benefit.
Okay, if all of the boards select it then we can do what omap does for CONFIG_OMAP2PLUS_TYPICAL and make the option default y so that we don't need to select it individually for each board.
Quoted 44 lines
Quoted 31 lines
Quoted 31 lines
config CRUNCH
arch/arm/mach-ep93xx/Kconfig +4
-0bool "Support for MaverickCrunch" help@@ -48,12 +52,14 @@ endchoice config MACH_ADSSPHERE bool "Support ADS Sphere" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the ADS Sphere board. config MACH_EDB93XX bool + select SOC_EP93XX config MACH_EDB9301 bool "Support Cirrus Logic EDB9301"@@ -122,12 +128,14 @@ config MACH_EDB9315A config MACH_GESBC9312 depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET bool "Support Glomation GESBC-9312-sx" + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Glomation GESBC-9312-sx board. config MACH_MICRO9 bool + select SOC_EP93XXQuoted 12 lines
arch/arm/mach-ep93xx/Kconfig config MACH_MICRO9S · line 175
Quoted 7 lines
arch/arm/mach-ep93xx/Kconfig +1
-0config MACH_MICRO9H bool "Support Contec Micro9-High"@@ -164,6 +172,7 @@ config MACH_MICRO9S config MACH_SIM_ONE bool "Support Simplemachines Sim.One board" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XXThe existing whitespace here is using spaces instead of tabs. If the result looks terrible (not aligned) then we should maybe do a separate patch to clean up the crappy whitespace.
Quoted 3 lines
I noticed that also... Hmm... who used the spaces ;-)
I agree, a separate patch should clean up the shitespace.
I'm only really bothered if the mixed whitespace makes things look horrible. If you make the CONFIG_SOC_EP93XX option default y then this is no longer an issue anyway.
Quoted 41 lines
Quoted 32 lines
Quoted 32 lines
arch/arm/mach-ep93xx/Kconfig +3
-0help Say 'Y' here if you want your kernel to support the Simplemachines Sim.One board.@@ -171,6 +180,7 @@ config MACH_SIM_ONE config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Bluewater Systems Snapper CL15 Module.@@ -178,6 +188,7 @@ config MACH_SNAPPER_CL15 config MACH_TS72XX bool "Support Technologic Systems TS-72xx SBC" depends on EP93XX_SDCE3_SYNC_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Technologic Systems TS-72xx board.@@ -185,6 +196,7 @@ config MACH_TS72XX config MACH_VISION_EP9307 bool "Support Vision Engraving Systems EP9307 SoM" depends on EP93XX_SDCE0_PHYS_OFFSET + select SOC_EP93XX help Say 'Y' here if you want your kernel to support the Vision Engraving Systems EP9307 SoM.arch/arm/mach-ep93xx/core.c +0
-0@@ -513,7 +513,7 @@ void __init ep93xx_register_spi(struct ep93xx_spi_info *info,Quoted 8 lines
arch/arm/mach-ep93xx/core.c void __init ep93xx_register_spi(struct ep93xx_spi_info *info, · line 516
Quoted 5 lines
arch/arm/mach-ep93xx/core.c +1
-1/************************************************************************* * EP93xx LEDs *************************************************************************/ -static struct gpio_led ep93xx_led_pins[] = { +static const struct gpio_led ep93xx_led_pins[]__initconst = {This fix, and related changes are not mentioned in the changelog.
Quoted 8 lines
Sorry about that. From
include/Linux/leds.h:struct
platform_device*gpio_led_register_device(int id, const struct gpio_led_platform_data *pdata);Since pdata needs to be const I changed the two relevant static variables to const. And, since nothing should modify them I also made them
__initconst. If you feel this needs to be mentioned in the changelog I will resubmit the patch.
Yeah, please mention it in the changelog.
Thanks,
~Ryan