On Thu, 17 Mar 2011 09:44:29 +0000
Ed W <[email protected]> wrote:
> Hi, please be gentle, first kernel patch. I have already posted this
> to the
> kernel mailing list, but I didn't obviously get any response.
It's best to Cc maintainers directly, as many of them (such as myself)
don't follow lkml all that closely.
> Grateful if someone could please take a look and guide me as to what
> adjustments might be required and also the correct person to address
> this to? Rather hoping that this might make 2.6.39?
Comments below. BTW, Grant is the new GPIO maintainer, so I've cc'd
him as well.
>
> The patch basically simplifies the Alix LED driver by making it use
> the leds_gpio instead of driving GPIOs directly. This allows us to
> use the normal kernel gpio facilities to access the rest of the board
> normally (Alix has a number of user controllable GPIOs)
>
> I have already mailed the original authors of the old alix led code,
> but without a response.
>
> It's hard to know, before pressing send, if my mail client will mangle
> my patch? Copy to the kernel mailing list direct from git is here:
> http://marc.info/?l=linux-kernel&m=129943074113312&w=2
>
> Thanks for any feedback
>
> Ed W
>
> -------- Original Message --------
> Subject: [PATCH] leds: New PCEngines Alix LED driver using
> gpio interface Date: Sun, 6 Mar 2011 16:51:25 +0000
> From: [email protected]
> To: [email protected]
> CC: [email protected], [email protected], [email protected],
> [email protected], Ed Wildgoose <[email protected]>
>
>
>
> From: Ed Wildgoose <[email protected]>
>
> This new driver replaces the old PCEngines Alix 2/3 LED driver with
> a new driver that controls the LEDs through the leds-gpio driver.
> The old driver accessed GPIOs directly, which created a conflict
> and prevented also loading the cs5535-gio driver to read other
> GPIOs on the Alix board. With this new driver, both are loaded
> and therefore it's possible to access both the LEDs and onboard GPIOs
>
> This driver is based on leds-net5501.c
> by: Alessandro Zummo <[email protected]>
> Additionally it relies on parts of the patch: 7f131cf3ed
> by: Daniel Mack <[email protected]> to perform detection of the Alix
> board
>
> Signed-off-by: Ed Wildgoose <[email protected]>
> ---
> :100644 100644 6f190f4... 25f0285... M drivers/leds/Kconfig
> :100644 100644 f59ffad... 62c8fff... M
> drivers/leds/leds-alix2.c drivers/leds/Kconfig | 2 +-
> drivers/leds/leds-alix2.c | 196
> ++++++++++---------------------------------- 2 files changed, 46
> insertions(+), 152 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 6f190f4..25f0285 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -100,7 +100,7 @@ config LEDS_WRAP
> config LEDS_ALIX2
> tristate "LED Support for ALIX.2 and ALIX.3 series"
> depends on LEDS_CLASS
> - depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
> + depends on X86 && LEDS_GPIO_PLATFORM && GPIO_CS5535
> help
> This option enables support for the PCEngines ALIX.2 and
> ALIX.3 LEDs. You have to set leds-alix2.force=1 for boards with Award
> BIOS. diff --git a/drivers/leds/leds-alix2.c
> b/drivers/leds/leds-alix2.c index f59ffad..62c8fff 100644
> --- a/drivers/leds/leds-alix2.c
> +++ b/drivers/leds/leds-alix2.c
> @@ -1,119 +1,66 @@
> /*
> * LEDs driver for PCEngines ALIX.2 and ALIX.3
> *
> - * Copyright (C) 2008 Constantin Baranov <[email protected]>
This copyright line should not be removed, so long as parts of the
original driver (such as alix_present) remain.
> + * Copyright (C) 2011 Nippy Networks Ltd
> + * Written by Ed Wildgoose <[email protected]>
> + * Based on leds-net5501.c by Alessandro Zummo <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> */
>
> -#include <linux/err.h>
> -#include <linux/io.h>
> #include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/string.h>
> #include <linux/leds.h>
> -#include <linux/module.h>
> #include <linux/platform_device.h>
> -#include <linux/string.h>
> -#include <linux/pci.h>
> +#include <linux/gpio.h>
> +
> +#include <asm/geode.h>
>
> static int force = 0;
> module_param(force, bool, 0444);
> MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style
> LEDs");
> -#define MSR_LBAR_GPIO 0x5140000C
> -#define CS5535_GPIO_SIZE 256
> -
> -static u32 gpio_base;
> -
> -static struct pci_device_id divil_pci[] = {
> - { PCI_DEVICE(PCI_VENDOR_ID_NS,
> PCI_DEVICE_ID_NS_CS5535_ISA) },
> - { PCI_DEVICE(PCI_VENDOR_ID_AMD,
> PCI_DEVICE_ID_AMD_CS5536_ISA) },
> - { } /* NULL entry */
> -};
> -MODULE_DEVICE_TABLE(pci, divil_pci);
> -
> -struct alix_led {
> - struct led_classdev cdev;
> - unsigned short port;
> - unsigned int on_value;
> - unsigned int off_value;
> -};
> -
> -static void alix_led_set(struct led_classdev *led_cdev,
> - enum led_brightness brightness)
> -{
> - struct alix_led *led_dev =
> - container_of(led_cdev, struct alix_led, cdev);
> -
> - if (brightness)
> - outl(led_dev->on_value, gpio_base + led_dev->port);
> - else
> - outl(led_dev->off_value, gpio_base + led_dev->port);
> -}
> -
> -static struct alix_led alix_leds[] = {
> +static struct gpio_led alix_leds[] = {
> {
> - .cdev = {
> - .name = "alix:1",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x00,
> - .on_value = 1 << 22,
> - .off_value = 1 << 6,
> + .name = "alix:1",
> + .gpio = 6,
> + .default_trigger = "default-on",
> + .active_low = 1,
> },
> {
> - .cdev = {
> - .name = "alix:2",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x80,
> - .on_value = 1 << 25,
> - .off_value = 1 << 9,
> + .name = "alix:2",
> + .gpio = 25,
> + .default_trigger = "default-off",
> + .active_low = 1,
> },
> {
> - .cdev = {
> - .name = "alix:3",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x80,
> - .on_value = 1 << 27,
> - .off_value = 1 << 11,
> + .name = "alix:3",
> + .gpio = 27,
> + .default_trigger = "default-off",
> + .active_low = 1,
> },
> };
>
> -static int __init alix_led_probe(struct platform_device *pdev)
> -{
> - int i;
> - int ret;
> -
> - for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
> - alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
> - ret = led_classdev_register(&pdev->dev,
> &alix_leds[i].cdev);
> - if (ret < 0)
> - goto fail;
> - }
> - return 0;
> +static struct gpio_led_platform_data alix_leds_data = {
> + .num_leds = ARRAY_SIZE(alix_leds),
> + .leds = alix_leds,
> +};
>
> -fail:
> - while (--i >= 0)
> - led_classdev_unregister(&alix_leds[i].cdev);
> - return ret;
> -}
> +static struct platform_device alix_leds_dev = {
> + .name = "leds-gpio",
This should probably be more unique; something like "leds-alix2".
> + .id = -1,
> + .dev.platform_data = &alix_leds_data,
> +};
>
> -static int alix_led_remove(struct platform_device *pdev)
> +static void __init register_alix(void)
> {
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
> - led_classdev_unregister(&alix_leds[i].cdev);
> - return 0;
> + platform_device_register(&alix_leds_dev);
> }
>
> -static struct platform_driver alix_led_driver = {
> - .remove = alix_led_remove,
> - .driver = {
> - .name = KBUILD_MODNAME,
> - .owner = THIS_MODULE,
> - },
> -};
> -
> static int __init alix_present(unsigned long bios_phys,
> const char *alix_sig,
> size_t alix_sig_len)
> @@ -164,76 +111,23 @@ static int __init alix_present(unsigned long
> bios_phys, return 0;
> }
>
> -static struct platform_device *pdev;
> -
> -static int __init alix_pci_led_init(void)
> -{
> - u32 low, hi;
> -
> - if (pci_dev_present(divil_pci) == 0) {
> - printk(KERN_WARNING KBUILD_MODNAME": DIVIL not
> found\n");
> - return -ENODEV;
> - }
> -
> - /* Grab the GPIO I/O range */
> - rdmsr(MSR_LBAR_GPIO, low, hi);
> -
> - /* Check the mask and whether GPIO is enabled (sanity check)
> */
> - if (hi != 0x0000f001) {
> - printk(KERN_WARNING KBUILD_MODNAME": GPIO not
> enabled\n");
> - return -ENODEV;
> - }
> -
> - /* Mask off the IO base address */
> - gpio_base = low & 0x0000ff00;
> -
> - if (!request_region(gpio_base, CS5535_GPIO_SIZE,
> KBUILD_MODNAME)) {
> - printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O
> for GPIO\n");
> - return -ENODEV;
> - }
> -
> - /* Set GPIO function to output */
> - outl(1 << 6, gpio_base + 0x04);
> - outl(1 << 9, gpio_base + 0x84);
> - outl(1 << 11, gpio_base + 0x84);
> -
> - return 0;
> -}
> -
> -static int __init alix_led_init(void)
> +static int __init alix_init(void)
> {
> - int ret = -ENODEV;
> const char tinybios_sig[] = "PC Engines ALIX.";
> const char coreboot_sig[] = "PC Engines\0ALIX.";
>
> + if (!is_geode())
> + return 0;
> +
Presumably you want to return -ENODEV here, especially since your
driver has no method for unloading.
> if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig)
> - 1) || alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
> - ret = alix_pci_led_init();
> + register_alix();
Ditto.
>
> - if (ret < 0)
> - return ret;
> -
> - pdev = platform_device_register_simple(KBUILD_MODNAME, -1,
> NULL, 0);
> - if (!IS_ERR(pdev)) {
> - ret = platform_driver_probe(&alix_led_driver,
> alix_led_probe);
> - if (ret)
> - platform_device_unregister(pdev);
> - } else
> - ret = PTR_ERR(pdev);
> -
> - return ret;
> -}
> -
> -static void __exit alix_led_exit(void)
> -{
> - platform_device_unregister(pdev);
> - platform_driver_unregister(&alix_led_driver);
> - release_region(gpio_base, CS5535_GPIO_SIZE);
> + return 0;
> }
>
> -module_init(alix_led_init);
> -module_exit(alix_led_exit);
> +arch_initcall(alix_init);
Why is this arch_initcall rather than module_init? If possible, it
would be good to have an unload hook as well.
>
> -MODULE_AUTHOR("Constantin Baranov <[email protected]>");
> +MODULE_AUTHOR("Ed Wildgoose <[email protected]>");
> MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
> MODULE_LICENSE("GPL");
On Thu, Mar 17, 2011 at 9:43 AM, Andres Salomon <[email protected]> wrote:
> On Thu, 17 Mar 2011 09:44:29 +0000
> Ed W <[email protected]> wrote:
>
>> Hi, please be gentle, first kernel patch. ?I have already posted this
>> to the
>> kernel mailing list, but I didn't obviously get any response.
>
> It's best to Cc maintainers directly, as many of them (such as myself)
> don't follow lkml all that closely.
>
>> Grateful if someone could please take a look and guide me as to what
>> adjustments might be required and also the correct person to address
>> this to? Rather hoping that this might make 2.6.39?
>
> Comments below. ?BTW, Grant is the new GPIO maintainer, so I've cc'd
> him as well.
Thanks Andres.
Hi Ed. Thanks for digging in to clean up this driver. It's
considered very bad form for a driver to both register a device and
then register the driver that binds against it, so this cleanup is
most welcome.
Actually, it looks like with your changes this isn't even a driver
anymore. It is merely code to register a device on a specific
platform. Is there any other alix-specific initialization code in the
kernel? If so, you should consider relocating the device registration
with the rest of the alix setup code.
More comments below...
>
>>
>> The patch basically simplifies the Alix LED driver by making it use
>> the leds_gpio instead of driving GPIOs directly. ?This allows us to
>> use the normal kernel gpio facilities to access the rest of the board
>> normally (Alix has a number of user controllable GPIOs)
>>
>> I have already mailed the original authors of the old alix led code,
>> but without a response.
>>
>> It's hard to know, before pressing send, if my mail client will mangle
>> my patch? Copy to the kernel mailing list direct from git is here:
>> ? ? http://marc.info/?l=linux-kernel&m=129943074113312&w=2
>>
>> Thanks for any feedback
>>
>> Ed W
>>
>> -------- Original Message --------
>> Subject: ? ? ?[PATCH] leds: New PCEngines Alix LED driver using
>> gpio interface Date: ?Sun, 6 Mar 2011 16:51:25 +0000
>> From: ? ? ? ? [email protected]
>> To: ? [email protected]
>> CC: ? [email protected], [email protected], [email protected],
>> [email protected], Ed Wildgoose <[email protected]>
>>
>>
>>
>> From: Ed Wildgoose <[email protected]>
>>
>> This new driver replaces the old PCEngines Alix 2/3 LED driver with
>> a new driver that controls the LEDs through the leds-gpio driver.
>> The old driver accessed GPIOs directly, which created a conflict
>> and prevented also loading the cs5535-gio driver to read other
>> GPIOs on the Alix board. With this new driver, both are loaded
>> and therefore it's possible to access both the LEDs and onboard GPIOs
>>
>> This driver is based on leds-net5501.c
>> by: Alessandro Zummo <[email protected]>
>> Additionally it relies on parts of the patch: 7f131cf3ed
>> by: Daniel Mack <[email protected]> to perform detection of the Alix
>> board
>>
>> Signed-off-by: Ed Wildgoose <[email protected]>
>> ---
>> :100644 100644 6f190f4... 25f0285... M ? ? ? ?drivers/leds/Kconfig
>> :100644 100644 f59ffad... 62c8fff... M
>> drivers/leds/leds-alix2.c drivers/leds/Kconfig ? ? ?| ? ?2 +-
>> ?drivers/leds/leds-alix2.c | ?196
>> ++++++++++---------------------------------- 2 files changed, 46
>> insertions(+), 152 deletions(-)
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 6f190f4..25f0285 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -100,7 +100,7 @@ config LEDS_WRAP
>> ?config LEDS_ALIX2
>> ? ? ? tristate "LED Support for ALIX.2 and ALIX.3 series"
>> ? ? ? depends on LEDS_CLASS
>> - ? ? depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
>> + ? ? depends on X86 && LEDS_GPIO_PLATFORM && GPIO_CS5535
>> ? ? ? help
>> ? ? ? ? This option enables support for the PCEngines ALIX.2 and
>> ALIX.3 LEDs. You have to set leds-alix2.force=1 for boards with Award
>> BIOS. diff --git a/drivers/leds/leds-alix2.c
>> b/drivers/leds/leds-alix2.c index f59ffad..62c8fff 100644
>> --- a/drivers/leds/leds-alix2.c
>> +++ b/drivers/leds/leds-alix2.c
>> @@ -1,119 +1,66 @@
>> ?/*
>> ? * LEDs driver for PCEngines ALIX.2 and ALIX.3
>> ? *
>> - * Copyright (C) 2008 Constantin Baranov <[email protected]>
>
> This copyright line should not be removed, so long as parts of the
> original driver (such as alix_present) remain.
>
>
>> + * Copyright (C) 2011 Nippy Networks Ltd
>> + * Written by Ed Wildgoose <[email protected]>
>> + * Based on leds-net5501.c by Alessandro Zummo <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation.
>> ? */
>>
>> -#include <linux/err.h>
>> -#include <linux/io.h>
>> ?#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/string.h>
>> ?#include <linux/leds.h>
>> -#include <linux/module.h>
>> ?#include <linux/platform_device.h>
>> -#include <linux/string.h>
>> -#include <linux/pci.h>
>> +#include <linux/gpio.h>
>> +
>> +#include <asm/geode.h>
>>
>> ?static int force = 0;
>> ?module_param(force, bool, 0444);
>> ?MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style
>> LEDs");
>> -#define MSR_LBAR_GPIO ? ? ? ? ? ? ? ?0x5140000C
>> -#define CS5535_GPIO_SIZE ? ? 256
>> -
>> -static u32 gpio_base;
>> -
>> -static struct pci_device_id divil_pci[] = {
>> - ? ? { PCI_DEVICE(PCI_VENDOR_ID_NS,
>> PCI_DEVICE_ID_NS_CS5535_ISA) },
>> - ? ? { PCI_DEVICE(PCI_VENDOR_ID_AMD,
>> PCI_DEVICE_ID_AMD_CS5536_ISA) },
>> - ? ? { } /* NULL entry */
>> -};
>> -MODULE_DEVICE_TABLE(pci, divil_pci);
>> -
>> -struct alix_led {
>> - ? ? struct led_classdev cdev;
>> - ? ? unsigned short port;
>> - ? ? unsigned int on_value;
>> - ? ? unsigned int off_value;
>> -};
>> -
>> -static void alix_led_set(struct led_classdev *led_cdev,
>> - ? ? ? ? ? ? ? ? ? ? ?enum led_brightness brightness)
>> -{
>> - ? ? struct alix_led *led_dev =
>> - ? ? ? ? ? ? container_of(led_cdev, struct alix_led, cdev);
>> -
>> - ? ? if (brightness)
>> - ? ? ? ? ? ? outl(led_dev->on_value, gpio_base + led_dev->port);
>> - ? ? else
>> - ? ? ? ? ? ? outl(led_dev->off_value, gpio_base + led_dev->port);
>> -}
>> -
>> -static struct alix_led alix_leds[] = {
>> +static struct gpio_led alix_leds[] = {
>> ? ? ? {
>> - ? ? ? ? ? ? .cdev = {
>> - ? ? ? ? ? ? ? ? ? ? .name = "alix:1",
>> - ? ? ? ? ? ? ? ? ? ? .brightness_set = alix_led_set,
>> - ? ? ? ? ? ? },
>> - ? ? ? ? ? ? .port = 0x00,
>> - ? ? ? ? ? ? .on_value = 1 << 22,
>> - ? ? ? ? ? ? .off_value = 1 << 6,
>> + ? ? ? ? ? ? .name = "alix:1",
>> + ? ? ? ? ? ? .gpio = 6,
>> + ? ? ? ? ? ? .default_trigger = "default-on",
>> + ? ? ? ? ? ? .active_low = 1,
>> ? ? ? },
>> ? ? ? {
>> - ? ? ? ? ? ? .cdev = {
>> - ? ? ? ? ? ? ? ? ? ? .name = "alix:2",
>> - ? ? ? ? ? ? ? ? ? ? .brightness_set = alix_led_set,
>> - ? ? ? ? ? ? },
>> - ? ? ? ? ? ? .port = 0x80,
>> - ? ? ? ? ? ? .on_value = 1 << 25,
>> - ? ? ? ? ? ? .off_value = 1 << 9,
>> + ? ? ? ? ? ? .name = "alix:2",
>> + ? ? ? ? ? ? .gpio = 25,
>> + ? ? ? ? ? ? .default_trigger = "default-off",
>> + ? ? ? ? ? ? .active_low = 1,
>> ? ? ? },
>> ? ? ? {
>> - ? ? ? ? ? ? .cdev = {
>> - ? ? ? ? ? ? ? ? ? ? .name = "alix:3",
>> - ? ? ? ? ? ? ? ? ? ? .brightness_set = alix_led_set,
>> - ? ? ? ? ? ? },
>> - ? ? ? ? ? ? .port = 0x80,
>> - ? ? ? ? ? ? .on_value = 1 << 27,
>> - ? ? ? ? ? ? .off_value = 1 << 11,
>> + ? ? ? ? ? ? .name = "alix:3",
>> + ? ? ? ? ? ? .gpio = 27,
>> + ? ? ? ? ? ? .default_trigger = "default-off",
>> + ? ? ? ? ? ? .active_low = 1,
>> ? ? ? },
>> ?};
>>
>> -static int __init alix_led_probe(struct platform_device *pdev)
>> -{
>> - ? ? int i;
>> - ? ? int ret;
>> -
>> - ? ? for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
>> - ? ? ? ? ? ? alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
>> - ? ? ? ? ? ? ret = led_classdev_register(&pdev->dev,
>> &alix_leds[i].cdev);
>> - ? ? ? ? ? ? if (ret < 0)
>> - ? ? ? ? ? ? ? ? ? ? goto fail;
>> - ? ? }
>> - ? ? return 0;
>> +static struct gpio_led_platform_data alix_leds_data = {
>> + ? ? .num_leds = ARRAY_SIZE(alix_leds),
>> + ? ? .leds = alix_leds,
>> +};
>>
>> -fail:
>> - ? ? while (--i >= 0)
>> - ? ? ? ? ? ? led_classdev_unregister(&alix_leds[i].cdev);
>> - ? ? return ret;
>> -}
>> +static struct platform_device alix_leds_dev = {
>> + ? ? .name = "leds-gpio",
>
> This should probably be more unique; something like "leds-alix2".
He is pretty much required to use "leds-gpio" so it will get bound to
the leds-gpio driver.
>
>> + ? ? .id = -1,
>> + ? ? .dev.platform_data = &alix_leds_data,
>> +};
>>
>> -static int alix_led_remove(struct platform_device *pdev)
>> +static void __init register_alix(void)
>> ?{
>> - ? ? int i;
>> -
>> - ? ? for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
>> - ? ? ? ? ? ? led_classdev_unregister(&alix_leds[i].cdev);
>> - ? ? return 0;
>> + ? ? platform_device_register(&alix_leds_dev);
>> ?}
>>
>> -static struct platform_driver alix_led_driver = {
>> - ? ? .remove = alix_led_remove,
>> - ? ? .driver = {
>> - ? ? ? ? ? ? .name = KBUILD_MODNAME,
>> - ? ? ? ? ? ? .owner = THIS_MODULE,
>> - ? ? },
>> -};
>> -
>> ?static int __init alix_present(unsigned long bios_phys,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *alix_sig,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? size_t alix_sig_len)
>> @@ -164,76 +111,23 @@ static int __init alix_present(unsigned long
>> bios_phys, return 0;
>> ?}
>>
>> -static struct platform_device *pdev;
>> -
>> -static int __init alix_pci_led_init(void)
>> -{
>> - ? ? u32 low, hi;
>> -
>> - ? ? if (pci_dev_present(divil_pci) == 0) {
>> - ? ? ? ? ? ? printk(KERN_WARNING KBUILD_MODNAME": DIVIL not
>> found\n");
>> - ? ? ? ? ? ? return -ENODEV;
>> - ? ? }
>> -
>> - ? ? /* Grab the GPIO I/O range */
>> - ? ? rdmsr(MSR_LBAR_GPIO, low, hi);
>> -
>> - ? ? /* Check the mask and whether GPIO is enabled (sanity check)
>> */
>> - ? ? if (hi != 0x0000f001) {
>> - ? ? ? ? ? ? printk(KERN_WARNING KBUILD_MODNAME": GPIO not
>> enabled\n");
>> - ? ? ? ? ? ? return -ENODEV;
>> - ? ? }
>> -
>> - ? ? /* Mask off the IO base address */
>> - ? ? gpio_base = low & 0x0000ff00;
>> -
>> - ? ? if (!request_region(gpio_base, CS5535_GPIO_SIZE,
>> KBUILD_MODNAME)) {
>> - ? ? ? ? ? ? printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O
>> for GPIO\n");
>> - ? ? ? ? ? ? return -ENODEV;
>> - ? ? }
>> -
>> - ? ? /* Set GPIO function to output */
>> - ? ? outl(1 << 6, gpio_base + 0x04);
>> - ? ? outl(1 << 9, gpio_base + 0x84);
>> - ? ? outl(1 << 11, gpio_base + 0x84);
>> -
>> - ? ? return 0;
>> -}
>> -
>> -static int __init alix_led_init(void)
>> +static int __init alix_init(void)
>> ?{
>> - ? ? int ret = -ENODEV;
>> ? ? ? const char tinybios_sig[] = "PC Engines ALIX.";
>> ? ? ? const char coreboot_sig[] = "PC Engines\0ALIX.";
>>
>> + ? ? if (!is_geode())
>> + ? ? ? ? ? ? return 0;
>> +
>
> Presumably you want to return -ENODEV here, especially since your
> driver has no method for unloading.
It looks to me that this isn't actually a driver anymore. It is
merely a hunk of code that registers a platform_device. Return 0
should be just fine.
>
>
>> ? ? ? if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig)
>> - 1) || alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
>> - ? ? ? ? ? ? ret = alix_pci_led_init();
>> + ? ? ? ? ? ? register_alix();
>
> Ditto.
>
>
>>
>> - ? ? if (ret < 0)
>> - ? ? ? ? ? ? return ret;
>> -
>> - ? ? pdev = platform_device_register_simple(KBUILD_MODNAME, -1,
>> NULL, 0);
>> - ? ? if (!IS_ERR(pdev)) {
>> - ? ? ? ? ? ? ret = platform_driver_probe(&alix_led_driver,
>> alix_led_probe);
>> - ? ? ? ? ? ? if (ret)
>> - ? ? ? ? ? ? ? ? ? ? platform_device_unregister(pdev);
>> - ? ? } else
>> - ? ? ? ? ? ? ret = PTR_ERR(pdev);
>> -
>> - ? ? return ret;
>> -}
>> -
>> -static void __exit alix_led_exit(void)
>> -{
>> - ? ? platform_device_unregister(pdev);
>> - ? ? platform_driver_unregister(&alix_led_driver);
>> - ? ? release_region(gpio_base, CS5535_GPIO_SIZE);
>> + ? ? return 0;
>> ?}
>>
>> -module_init(alix_led_init);
>> -module_exit(alix_led_exit);
>> +arch_initcall(alix_init);
>
> Why is this arch_initcall rather than module_init? ? If possible, it
> would be good to have an unload hook as well.
Yes, unless you've got specific ordering constraints this should
definitely be module_init().
g.
Hi Andres
Thanks for your feedback!
>> Additionally it relies on parts of the patch: 7f131cf3ed
>> by: Daniel Mack <[email protected]> to perform detection of the Alix
>> board
..
>> - * Copyright (C) 2008 Constantin Baranov <[email protected]>
>
> This copyright line should not be removed, so long as parts of the
> original driver (such as alix_present) remain.
Thanks for guidance here.
Can I ask for further thought on this - there is very close to zero
original driver present, and although I had better do some more diffs to
be sure, I think you would see the only common code was the #includes, a
"force" param and a few other { }s?
I did deliberately reuse the "alix_present" function, but this appears
to be written by Daniel Mack and contributed in patch: 7f131cf3ed -
however, Daniel is not listed in the current copyright statement on the
module (I did copy him and Constantin in on this patch so that either
might object?). Also I have tried to show this code attribution in the
commit statement?
I have also noted in the code that this is based on leds-net5501.c - is
this a sufficient and normal attribution?
Can someone offer a final "ruling" as to how I should state the
copyright line given that the code was written by taking the
leds-net5501.c skeleton and approximately applying the commit
7f131cf3ed, to give the current code?
Many thanks
Ed W
On 17/03/2011 16:08, Grant Likely wrote:
> Actually, it looks like with your changes this isn't even a driver
> anymore. It is merely code to register a device on a specific
> platform. Is there any other alix-specific initialization code in the
> kernel? If so, you should consider relocating the device registration
> with the rest of the alix setup code.
Agreed. I confess that I don't understand the linux driver structure
enough to shift the code further though
What I observe is that there is a lot of arch specific setup for ARM,
etc, however, this is not currently done at all for x86 (which is Alix),
so at the moment this would seem to sit slightly awkwardly with current
x86 arch code?
Instead I found leds-net5501.c, which is for a very similar platform to
the Alix (not quite similar enough that I could combine the files) and I
used that as my prototype for this driver.
I think given that we already have a similar driver in the leds area
which does platform alike setup, this gives some justification for doing
the same with the Alix leds? Additionally if we ever find we need Alix
specific setup code then the code is ready to be used as is by the
platform code?
>>> -module_init(alix_led_init);
>>> -module_exit(alix_led_exit);
>>> +arch_initcall(alix_init);
>>
>> Why is this arch_initcall rather than module_init? If possible, it
>> would be good to have an unload hook as well.
>
> Yes, unless you've got specific ordering constraints this should
> definitely be module_init().
I'm out of my depth here. I would be very happy to resubmit either way?
However, is there not a potential ordering issue if leds-alix2 is loaded
*before* leds-gpio? Is this not the reason for making it an arch_initcall?
Also the same code is used in leds-5501.c - would you like me to submit
a patch to change that also (if you confirm it should become a
module_init call?).
Thanks for final confirmation on this and I will quickly resubmit the patch?
Ed W
On Thu, 17 Mar 2011 17:24:55 +0000
Ed W <[email protected]> wrote:
> On 17/03/2011 16:08, Grant Likely wrote:
> > Actually, it looks like with your changes this isn't even a driver
> > anymore. It is merely code to register a device on a specific
> > platform. Is there any other alix-specific initialization code in
> > the kernel? If so, you should consider relocating the device
> > registration with the rest of the alix setup code.
>
> Agreed. I confess that I don't understand the linux driver structure
> enough to shift the code further though
>
> What I observe is that there is a lot of arch specific setup for ARM,
> etc, however, this is not currently done at all for x86 (which is
> Alix), so at the moment this would seem to sit slightly awkwardly
> with current x86 arch code?
>
> Instead I found leds-net5501.c, which is for a very similar platform
> to the Alix (not quite similar enough that I could combine the files)
> and I used that as my prototype for this driver.
>
OLPC stuff lives in arch/x86/platform/olpc; if there was more
alix-specific stuff, I'd suggest moving it into something similar.
However, I didn't find any. Maybe an arch/x86/platform/geode as a
place to collect platform drivers for the various geode-based machines
out there (alix, soekris, etc)? Though honestly, I'm not that
interested in doing the work to migrate stuff over to there.
> I think given that we already have a similar driver in the leds area
> which does platform alike setup, this gives some justification for
> doing the same with the Alix leds? Additionally if we ever find we
> need Alix specific setup code then the code is ready to be used as is
> by the platform code?
>
>
> >>> -module_init(alix_led_init);
> >>> -module_exit(alix_led_exit);
> >>> +arch_initcall(alix_init);
> >>
> >> Why is this arch_initcall rather than module_init? If possible,
> >> it would be good to have an unload hook as well.
> >
> > Yes, unless you've got specific ordering constraints this should
> > definitely be module_init().
>
> I'm out of my depth here. I would be very happy to resubmit either
> way?
>
> However, is there not a potential ordering issue if leds-alix2 is
> loaded *before* leds-gpio? Is this not the reason for making it an
> arch_initcall?
>
> Also the same code is used in leds-5501.c - would you like me to
> submit a patch to change that also (if you confirm it should become a
> module_init call?).
Yes, it should be module_init. There shouldn't be any issues with
leds-gpio; the driver will only bind once a device is added (so long as
nothing else named leds-gpio comes along before leds-alix2).
>
> Thanks for final confirmation on this and I will quickly resubmit the
> patch?
>
> Ed W
On 17/03/2011 17:52, Andres Salomon wrote:
> OLPC stuff lives in arch/x86/platform/olpc; if there was more
> alix-specific stuff, I'd suggest moving it into something similar.
> However, I didn't find any. Maybe an arch/x86/platform/geode as a
> place to collect platform drivers for the various geode-based machines
> out there (alix, soekris, etc)? Though honestly, I'm not that
> interested in doing the work to migrate stuff over to there.
As this is my first stab at this, I think you are agreeing that the
current change is ok as a first step at least? (I can have a go at a
larger code re-org later...)
Can I get your final opinion on what should be in the code Copyright
statement before I resubmit with the module_init change? To recap I
don't want to alienate anyone, but the code is now based on a) leds-5501
and b) a patch from someone not currently credited in the file copyright
statement? I have stated this origin in the commit log and noted the
derivation in the code itself. What to do..?
Thanks
Ed W
On Thu, Mar 17, 2011 at 05:04:25PM +0000, Ed W wrote:
> Hi Andres
>
> Thanks for your feedback!
>
> >> Additionally it relies on parts of the patch: 7f131cf3ed
> >> by: Daniel Mack <[email protected]> to perform detection of the Alix
> >> board
>
> ..
> >> - * Copyright (C) 2008 Constantin Baranov <[email protected]>
> >
> > This copyright line should not be removed, so long as parts of the
> > original driver (such as alix_present) remain.
>
> Thanks for guidance here.
>
> Can I ask for further thought on this - there is very close to zero
> original driver present, and although I had better do some more diffs to
> be sure, I think you would see the only common code was the #includes, a
> "force" param and a few other { }s?
>
> I did deliberately reuse the "alix_present" function, but this appears
> to be written by Daniel Mack and contributed in patch: 7f131cf3ed -
> however, Daniel is not listed in the current copyright statement on the
> module (I did copy him and Constantin in on this patch so that either
> might object?). Also I have tried to show this code attribution in the
> commit statement?
>
> I have also noted in the code that this is based on leds-net5501.c - is
> this a sufficient and normal attribution?
>
> Can someone offer a final "ruling" as to how I should state the
> copyright line given that the code was written by taking the
> leds-net5501.c skeleton and approximately applying the commit
> 7f131cf3ed, to give the current code?
Use common sense. It is always safe to leave copyright notices in
place, but if it really does look like the old stuff is gone then go
ahead and drop it.
Same goes when you clone a driver. The original copyrights should not
be stripped off unless there really is nothing left.
g.
On Thu, Mar 17, 2011 at 05:24:55PM +0000, Ed W wrote:
> On 17/03/2011 16:08, Grant Likely wrote:
> > Actually, it looks like with your changes this isn't even a driver
> > anymore. It is merely code to register a device on a specific
> > platform. Is there any other alix-specific initialization code in the
> > kernel? If so, you should consider relocating the device registration
> > with the rest of the alix setup code.
>
> Agreed. I confess that I don't understand the linux driver structure
> enough to shift the code further though
>
> What I observe is that there is a lot of arch specific setup for ARM,
> etc, however, this is not currently done at all for x86 (which is Alix),
> so at the moment this would seem to sit slightly awkwardly with current
> x86 arch code?
>
> Instead I found leds-net5501.c, which is for a very similar platform to
> the Alix (not quite similar enough that I could combine the files) and I
> used that as my prototype for this driver.
>
> I think given that we already have a similar driver in the leds area
> which does platform alike setup, this gives some justification for doing
> the same with the Alix leds?
No, that's bad practise. You should have one Alix setup file,
probably in arch/x86/platform, that takes care of all Alix specific
setup registrations. Right now that only consists of gpio leds, but
it could potentially grow. Note that this is only for on-time setup
and registrations. Actual device drivers still belong under drivers/
> Additionally if we ever find we need Alix
> specific setup code then the code is ready to be used as is by the
> platform code?
>
>
> >>> -module_init(alix_led_init);
> >>> -module_exit(alix_led_exit);
> >>> +arch_initcall(alix_init);
> >>
> >> Why is this arch_initcall rather than module_init? If possible, it
> >> would be good to have an unload hook as well.
> >
> > Yes, unless you've got specific ordering constraints this should
> > definitely be module_init().
>
> I'm out of my depth here. I would be very happy to resubmit either way?
>
> However, is there not a potential ordering issue if leds-alix2 is loaded
> *before* leds-gpio? Is this not the reason for making it an arch_initcall?
Nope. It does not matter if the driver gets registered first or the
device.
> Also the same code is used in leds-5501.c - would you like me to submit
> a patch to change that also (if you confirm it should become a
> module_init call?).
Unless you've got the hardware to test, probably not.
> Thanks for final confirmation on this and I will quickly resubmit the patch?
Thanks
g.
On Thu, Mar 17, 2011 at 05:59:06PM +0000, Ed W wrote:
> On 17/03/2011 17:52, Andres Salomon wrote:
> > OLPC stuff lives in arch/x86/platform/olpc; if there was more
> > alix-specific stuff, I'd suggest moving it into something similar.
> > However, I didn't find any. Maybe an arch/x86/platform/geode as a
> > place to collect platform drivers for the various geode-based machines
> > out there (alix, soekris, etc)? Though honestly, I'm not that
> > interested in doing the work to migrate stuff over to there.
>
>
> As this is my first stab at this, I think you are agreeing that the
> current change is ok as a first step at least? (I can have a go at a
> larger code re-org later...)
It is actually really easy. You just need to move the file into
arch/x86/platform/geode, add a Makefile, and add a line to
arch/x86/platform/Makefile to include it. In this particular case,
since so much code is sliced out, the patch will actually be easier to
review if the file is moved at the same time.
g.
On Thu, 17 Mar 2011 17:59:06 +0000
Ed W <[email protected]> wrote:
> On 17/03/2011 17:52, Andres Salomon wrote:
> > OLPC stuff lives in arch/x86/platform/olpc; if there was more
> > alix-specific stuff, I'd suggest moving it into something similar.
> > However, I didn't find any. Maybe an arch/x86/platform/geode as a
> > place to collect platform drivers for the various geode-based
> > machines out there (alix, soekris, etc)? Though honestly, I'm not
> > that interested in doing the work to migrate stuff over to there.
>
>
> As this is my first stab at this, I think you are agreeing that the
> current change is ok as a first step at least? (I can have a go at a
> larger code re-org later...)
Yes.
>
> Can I get your final opinion on what should be in the code Copyright
> statement before I resubmit with the module_init change? To recap I
> don't want to alienate anyone, but the code is now based on a)
> leds-5501 and b) a patch from someone not currently credited in the
> file copyright statement? I have stated this origin in the commit
> log and noted the derivation in the code itself. What to do..?
alix_present came from commit
ec9a943ce9f6d6a8ea09587b49d29a020c418c76 , which was from Constantin.
As such, it's probably not a good idea to remove the copyright notice.
From: Ed Wildgoose <[email protected]>
This new driver replaces the old PCEngines Alix 2/3 LED driver with
a new driver that controls the LEDs through the leds-gpio driver.
The old driver accessed GPIOs directly, which created a conflict
and prevented also loading the cs5535-gpio driver to read other
GPIOs on the Alix board. With this new driver, we hook into leds-gpio
which in turn uses GPIO to control the LEDs and therefore it's
possible to control both the LEDs and access onboard GPIOs
Driver is moved to platform/geode and any other geode
initialisation modules should move here also.
This driver is inspired by leds-net5501.c
by: Alessandro Zummo <[email protected]>
Ideally, leds-net5501.c should also be moved to platform/geode.
Additionally the driver relies on parts of the patch: 7f131cf3ed
by: Daniel Mack <[email protected]> to perform detection of the Alix board
Signed-off-by: Ed Wildgoose <[email protected]>
---
:100644 100644 d5ed94d... b16ab56... M arch/x86/Kconfig
:100644 100644 021eee9... 8d87439... M arch/x86/platform/Makefile
:000000 100644 0000000... d4e599f... A arch/x86/platform/geode/Makefile
:000000 100644 0000000... bc1b3b3... A arch/x86/platform/geode/alix_leds.c
:100644 100644 6f190f4... e4573d6... M drivers/leds/Kconfig
:100644 100644 aae6989... bf5a00c... M drivers/leds/Makefile
:100644 000000 f59ffad... 0000000... D drivers/leds/leds-alix2.c
arch/x86/Kconfig | 9 ++
arch/x86/platform/Makefile | 1 +
arch/x86/platform/geode/Makefile | 1 +
arch/x86/platform/geode/alix_leds.c | 138 ++++++++++++++++++++
drivers/leds/Kconfig | 8 --
drivers/leds/Makefile | 1 -
drivers/leds/leds-alix2.c | 239 -----------------------------------
7 files changed, 149 insertions(+), 248 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d5ed94d..b16ab56 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
select OF_PROMTREE
+config ALIX_LEDS
+ bool "PCEngines ALIX.2/.3 LED Support"
+ select GPIOLIB
+ depends on LEDS_CLASS
+ depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
+ ---help---
+ This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
+ You have to set alix-leds.force=1 for boards with Award BIOS.
+
endif # X86_32
config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 021eee9..8d87439 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -1,6 +1,7 @@
# Platform specific code goes here
obj-y += ce4100/
obj-y += efi/
+obj-y += geode/
obj-y += iris/
obj-y += mrst/
obj-y += olpc/
diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
new file mode 100644
index 0000000..d4e599f
--- /dev/null
+++ b/arch/x86/platform/geode/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ALIX_LEDS) += alix-leds.o
diff --git a/arch/x86/platform/geode/alix_leds.c b/arch/x86/platform/geode/alix_leds.c
new file mode 100644
index 0000000..bc1b3b3
--- /dev/null
+++ b/arch/x86/platform/geode/alix_leds.c
@@ -0,0 +1,138 @@
+/*
+ * Configure GPIO control of LEDs on Alix.2/3 from PCEngines
+ *
+ * Copyright (C) 2008 Constantin Baranov <[email protected]>
+ * Copyright (C) 2011 Nippy Networks Ltd
+ *
+ * TODO: There are large similarities with leds-net5501.c
+ * by Alessandro Zummo <[email protected]>
+ * In the future leds-net5501.c should be migrated over to platform
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+
+#include <asm/geode.h>
+
+static int force = 0;
+module_param(force, bool, 0444);
+/* FIXME: Award bios is not automatically detected as Alix platform */
+MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3 platform");
+
+static struct gpio_led alix_leds[] = {
+ {
+ .name = "alix:1",
+ .gpio = 6,
+ .default_trigger = "default-on",
+ .active_low = 1,
+ },
+ {
+ .name = "alix:2",
+ .gpio = 25,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+ {
+ .name = "alix:3",
+ .gpio = 27,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+};
+
+static struct gpio_led_platform_data alix_leds_data = {
+ .num_leds = ARRAY_SIZE(alix_leds),
+ .leds = alix_leds,
+};
+
+static struct platform_device alix_leds_dev = {
+ .name = "leds-gpio",
+ .id = -1,
+ .dev.platform_data = &alix_leds_data,
+};
+
+static void __init register_alix(void)
+{
+ /* Setup LED control through leds-gpio driver */
+ platform_device_register(&alix_leds_dev);
+}
+
+static int __init alix_present(unsigned long bios_phys,
+ const char *alix_sig,
+ size_t alix_sig_len)
+{
+ const size_t bios_len = 0x00010000;
+ const char *bios_virt;
+ const char *scan_end;
+ const char *p;
+ char name[64];
+
+ if (force) {
+ printk(KERN_NOTICE "%s: forced to skip BIOS test, "
+ "assume system is ALIX.2/ALIX.3\n",
+ KBUILD_MODNAME);
+ return 1;
+ }
+
+ bios_virt = phys_to_virt(bios_phys);
+ scan_end = bios_virt + bios_len - (alix_sig_len + 2);
+ for (p = bios_virt; p < scan_end; p++) {
+ const char *tail;
+ char *a;
+
+ if (memcmp(p, alix_sig, alix_sig_len) != 0)
+ continue;
+
+ memcpy(name, p, sizeof(name));
+
+ /* remove the first \0 character from string */
+ a = strchr(name, '\0');
+ if (a)
+ *a = ' ';
+
+ /* cut the string at a newline */
+ a = strchr(name, '\r');
+ if (a)
+ *a = '\0';
+
+ tail = p + alix_sig_len;
+ if ((tail[0] == '2' || tail[0] == '3')) {
+ printk(KERN_INFO
+ "%s: system is recognized as \"%s\"\n",
+ KBUILD_MODNAME, name);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static int __init alix_init(void)
+{
+ const char tinybios_sig[] = "PC Engines ALIX.";
+ const char coreboot_sig[] = "PC Engines\0ALIX.";
+
+ if (!is_geode())
+ return 0;
+
+ if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
+ alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
+ register_alix();
+
+ return 0;
+}
+
+module_init(alix_init);
+
+MODULE_AUTHOR("Ed Wildgoose <[email protected]>");
+MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 System Setup");
+MODULE_LICENSE("GPL");
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6f190f4..e4573d6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -97,14 +97,6 @@ config LEDS_WRAP
help
This option enables support for the PCEngines WRAP programmable LEDs.
-config LEDS_ALIX2
- tristate "LED Support for ALIX.2 and ALIX.3 series"
- depends on LEDS_CLASS
- depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
- help
- This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
- You have to set leds-alix2.force=1 for boards with Award BIOS.
-
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index aae6989..bf5a00c 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_LEDS_AMS_DELTA) += leds-ams-delta.o
obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
obj-$(CONFIG_LEDS_NET5501) += leds-net5501.o
obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
-obj-$(CONFIG_LEDS_ALIX2) += leds-alix2.o
obj-$(CONFIG_LEDS_H1940) += leds-h1940.o
obj-$(CONFIG_LEDS_COBALT_QUBE) += leds-cobalt-qube.o
obj-$(CONFIG_LEDS_COBALT_RAQ) += leds-cobalt-raq.o
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
deleted file mode 100644
index f59ffad..0000000
--- a/drivers/leds/leds-alix2.c
+++ /dev/null
@@ -1,239 +0,0 @@
-/*
- * LEDs driver for PCEngines ALIX.2 and ALIX.3
- *
- * Copyright (C) 2008 Constantin Baranov <[email protected]>
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/leds.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/string.h>
-#include <linux/pci.h>
-
-static int force = 0;
-module_param(force, bool, 0444);
-MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style LEDs");
-
-#define MSR_LBAR_GPIO 0x5140000C
-#define CS5535_GPIO_SIZE 256
-
-static u32 gpio_base;
-
-static struct pci_device_id divil_pci[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_CS5535_ISA) },
- { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
- { } /* NULL entry */
-};
-MODULE_DEVICE_TABLE(pci, divil_pci);
-
-struct alix_led {
- struct led_classdev cdev;
- unsigned short port;
- unsigned int on_value;
- unsigned int off_value;
-};
-
-static void alix_led_set(struct led_classdev *led_cdev,
- enum led_brightness brightness)
-{
- struct alix_led *led_dev =
- container_of(led_cdev, struct alix_led, cdev);
-
- if (brightness)
- outl(led_dev->on_value, gpio_base + led_dev->port);
- else
- outl(led_dev->off_value, gpio_base + led_dev->port);
-}
-
-static struct alix_led alix_leds[] = {
- {
- .cdev = {
- .name = "alix:1",
- .brightness_set = alix_led_set,
- },
- .port = 0x00,
- .on_value = 1 << 22,
- .off_value = 1 << 6,
- },
- {
- .cdev = {
- .name = "alix:2",
- .brightness_set = alix_led_set,
- },
- .port = 0x80,
- .on_value = 1 << 25,
- .off_value = 1 << 9,
- },
- {
- .cdev = {
- .name = "alix:3",
- .brightness_set = alix_led_set,
- },
- .port = 0x80,
- .on_value = 1 << 27,
- .off_value = 1 << 11,
- },
-};
-
-static int __init alix_led_probe(struct platform_device *pdev)
-{
- int i;
- int ret;
-
- for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
- alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
- ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev);
- if (ret < 0)
- goto fail;
- }
- return 0;
-
-fail:
- while (--i >= 0)
- led_classdev_unregister(&alix_leds[i].cdev);
- return ret;
-}
-
-static int alix_led_remove(struct platform_device *pdev)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
- led_classdev_unregister(&alix_leds[i].cdev);
- return 0;
-}
-
-static struct platform_driver alix_led_driver = {
- .remove = alix_led_remove,
- .driver = {
- .name = KBUILD_MODNAME,
- .owner = THIS_MODULE,
- },
-};
-
-static int __init alix_present(unsigned long bios_phys,
- const char *alix_sig,
- size_t alix_sig_len)
-{
- const size_t bios_len = 0x00010000;
- const char *bios_virt;
- const char *scan_end;
- const char *p;
- char name[64];
-
- if (force) {
- printk(KERN_NOTICE "%s: forced to skip BIOS test, "
- "assume system has ALIX.2 style LEDs\n",
- KBUILD_MODNAME);
- return 1;
- }
-
- bios_virt = phys_to_virt(bios_phys);
- scan_end = bios_virt + bios_len - (alix_sig_len + 2);
- for (p = bios_virt; p < scan_end; p++) {
- const char *tail;
- char *a;
-
- if (memcmp(p, alix_sig, alix_sig_len) != 0)
- continue;
-
- memcpy(name, p, sizeof(name));
-
- /* remove the first \0 character from string */
- a = strchr(name, '\0');
- if (a)
- *a = ' ';
-
- /* cut the string at a newline */
- a = strchr(name, '\r');
- if (a)
- *a = '\0';
-
- tail = p + alix_sig_len;
- if ((tail[0] == '2' || tail[0] == '3')) {
- printk(KERN_INFO
- "%s: system is recognized as \"%s\"\n",
- KBUILD_MODNAME, name);
- return 1;
- }
- }
-
- return 0;
-}
-
-static struct platform_device *pdev;
-
-static int __init alix_pci_led_init(void)
-{
- u32 low, hi;
-
- if (pci_dev_present(divil_pci) == 0) {
- printk(KERN_WARNING KBUILD_MODNAME": DIVIL not found\n");
- return -ENODEV;
- }
-
- /* Grab the GPIO I/O range */
- rdmsr(MSR_LBAR_GPIO, low, hi);
-
- /* Check the mask and whether GPIO is enabled (sanity check) */
- if (hi != 0x0000f001) {
- printk(KERN_WARNING KBUILD_MODNAME": GPIO not enabled\n");
- return -ENODEV;
- }
-
- /* Mask off the IO base address */
- gpio_base = low & 0x0000ff00;
-
- if (!request_region(gpio_base, CS5535_GPIO_SIZE, KBUILD_MODNAME)) {
- printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O for GPIO\n");
- return -ENODEV;
- }
-
- /* Set GPIO function to output */
- outl(1 << 6, gpio_base + 0x04);
- outl(1 << 9, gpio_base + 0x84);
- outl(1 << 11, gpio_base + 0x84);
-
- return 0;
-}
-
-static int __init alix_led_init(void)
-{
- int ret = -ENODEV;
- const char tinybios_sig[] = "PC Engines ALIX.";
- const char coreboot_sig[] = "PC Engines\0ALIX.";
-
- if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
- alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
- ret = alix_pci_led_init();
-
- if (ret < 0)
- return ret;
-
- pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
- if (!IS_ERR(pdev)) {
- ret = platform_driver_probe(&alix_led_driver, alix_led_probe);
- if (ret)
- platform_device_unregister(pdev);
- } else
- ret = PTR_ERR(pdev);
-
- return ret;
-}
-
-static void __exit alix_led_exit(void)
-{
- platform_device_unregister(pdev);
- platform_driver_unregister(&alix_led_driver);
- release_region(gpio_base, CS5535_GPIO_SIZE);
-}
-
-module_init(alix_led_init);
-module_exit(alix_led_exit);
-
-MODULE_AUTHOR("Constantin Baranov <[email protected]>");
-MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
-MODULE_LICENSE("GPL");
--
1.7.4
Hi, Thanks for "mentoring" me over this. I appreciate that this
consumes your valuable time
I have reposted what I hope is close to the correct code for this new
driver. I'm somewhat apprehensive, so to draw attention to the things I
have most likely "done wrong":
> :000000 100644 0000000... bc1b3b3... A arch/x86/platform/geode/alix_leds.c
...
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d5ed94d..b16ab56 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
> default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
> select OF_PROMTREE
>
> +config ALIX_LEDS
> + bool "PCEngines ALIX.2/.3 LED Support"
> + select GPIOLIB
> + depends on LEDS_CLASS
> + depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
> + ---help---
> + This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
> + You have to set alix-leds.force=1 for boards with Award BIOS.
> +
> endif # X86_32
Driver and KConfig show this being specifically a driver for the Alix
LEDs rather than being a general Alix plaform initialisation module?
I was unsure if the trend was to have one module which initialised all
Alix platform stuff (whatever it needs), or to split by function?
Looking at other platform modules they seem to be somewhat fine grained
so I went with a specific "ALIX Led Module" approach?
Additionally I am unsure how strictly to set dependencies for my module?
It clearly requires all the LED_GPIO_PLATFORM, GPIO_CS5535 dependencies
to do anything, but equally it doesn't seem to *break* anything if those
dependencies aren't compiled in? Listing all the dependencies seems to
make it hard for users to find the option to enable it since it's not
even listed in menuconfig until your deps are met? Please correct me as
to what level of deps should be listed?
On the topic of dependencies, Andres has changed cs5535-gpio.c to depend
on a new module cs5535-mfd - however, apart from the commit log message
this is not obvious to see? OK, I'm an idiot, but it took me most of
this afternoon to understand why my GPIOs stopped working after moving
to 2.6.38? Q: Should the new -mfd module be listed as a dep of -gpio?
Or perhaps -gpio should "select" -mfd? At least it would be helpful to
list the dependency in the Kconfig message?
Happy to submit a change - please advise on what (if anything) would be
the best solution?
Many thanks for your help and guidance so far
Ed W
On Fri, Mar 18, 2011 at 12:32 PM, Ed W <[email protected]> wrote:
> Hi, Thanks for "mentoring" me over this. ?I appreciate that this
> consumes your valuable time
>
> I have reposted what I hope is close to the correct code for this new
> driver. ?I'm somewhat apprehensive, so to draw attention to the things I
> have most likely "done wrong":
>
>
>> :000000 100644 0000000... bc1b3b3... A ? ? ? ?arch/x86/platform/geode/alix_leds.c
>
> ...
>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index d5ed94d..b16ab56 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
>> ? ? ? default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
>> ? ? ? select OF_PROMTREE
>>
>> +config ALIX_LEDS
>> + ? ? bool "PCEngines ALIX.2/.3 LED Support"
>> + ? ? select GPIOLIB
>> + ? ? depends on LEDS_CLASS
>> + ? ? depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
>> + ? ? ---help---
>> + ? ? ? This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
>> + ? ? ? You have to set alix-leds.force=1 for boards with Award BIOS.
>> +
>> ?endif # X86_32
>
>
> Driver and KConfig show this being specifically a driver for the Alix
> LEDs rather than being a general Alix plaform initialisation module?
>
> I was unsure if the trend was to have one module which initialised all
> Alix platform stuff (whatever it needs), or to split by function?
> Looking at other platform modules they seem to be somewhat fine grained
> so I went with a specific "ALIX Led Module" approach?
That's very much up to the board maintainer. Because it is just
device registrations, I would lump them all into one file. If they
were actual drivers then I tend to split them up.
> Additionally I am unsure how strictly to set dependencies for my module?
> ?It clearly requires all the LED_GPIO_PLATFORM, GPIO_CS5535 dependencies
> to do anything, but equally it doesn't seem to *break* anything if those
> dependencies aren't compiled in? ?Listing all the dependencies seems to
> make it hard for users to find the option to enable it since it's not
> even listed in menuconfig until your deps are met? ?Please correct me as
> to what level of deps should be listed?
Correct, you don't need to depend on the driver because this is just a
device registration. Who cares if it never gets bound to the driver?
The device registration will happily sit in the device model
unregistered.
> On the topic of dependencies, Andres has changed cs5535-gpio.c to depend
> on a new module cs5535-mfd - however, apart from the commit log message
> this is not obvious to see? ?OK, I'm an idiot, but it took me most of
> this afternoon to understand why my GPIOs stopped working after moving
> to 2.6.38? ?Q: Should the new -mfd module be listed as a dep of -gpio?
> Or perhaps -gpio should "select" -mfd? At least it would be helpful to
> list the dependency in the Kconfig message?
select is a little touchy because selecting another symbol bypasses
and dependences the other symbol has. In this case, it could skip
CONFIG_MFD_CORE which may be bad. -gpio should be a dependancy of
-mfd in this case I think.
g.
From: Ed Wildgoose <[email protected]>
This new driver replaces the old PCEngines Alix 2/3 LED driver with
a new driver that controls the LEDs through the leds-gpio driver.
The old driver accessed GPIOs directly, which created a conflict
and prevented also loading the cs5535-gpio driver to read other
GPIOs on the Alix board. With this new driver, we hook into leds-gpio
which in turn uses GPIO to control the LEDs and therefore it's
possible to control both the LEDs and access onboard GPIOs
Driver is moved to platform/geode and any other geode
initialisation modules should move here also.
This driver is inspired by leds-net5501.c
by: Alessandro Zummo <[email protected]>
Ideally, leds-net5501.c should also be moved to platform/geode.
Additionally the driver relies on parts of the patch: 7f131cf3ed
by: Daniel Mack <[email protected]> to perform detection of the Alix board
Signed-off-by: Ed Wildgoose <[email protected]>
---
:100644 100644 d5ed94d... a10556d... M arch/x86/Kconfig
:100644 100644 021eee9... 8d87439... M arch/x86/platform/Makefile
:000000 100644 0000000... 07c9cd0... A arch/x86/platform/geode/Makefile
:000000 100644 0000000... f3f01e2... A arch/x86/platform/geode/alix.c
:100644 100644 6f190f4... e4573d6... M drivers/leds/Kconfig
:100644 100644 aae6989... bf5a00c... M drivers/leds/Makefile
:100644 000000 f59ffad... 0000000... D drivers/leds/leds-alix2.c
arch/x86/Kconfig | 14 +++
arch/x86/platform/Makefile | 1 +
arch/x86/platform/geode/Makefile | 1 +
arch/x86/platform/geode/alix.c | 141 ++++++++++++++++++++++
drivers/leds/Kconfig | 8 --
drivers/leds/Makefile | 1 -
drivers/leds/leds-alix2.c | 239 --------------------------------------
7 files changed, 157 insertions(+), 248 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d5ed94d..a10556d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2094,6 +2094,20 @@ config OLPC_OPENFIRMWARE_DT
default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
select OF_PROMTREE
+config ALIX
+ bool "PCEngines ALIX System Support (LED setup)"
+ select GPIOLIB
+ ---help---
+ This option enables system support for the PCEngines ALIX.
+ At present this just sets up LEDs for GPIO control on
+ ALIX2/3/6 boards. However, other system specific setup should
+ get added here.
+
+ Note: You must still enable the drivers for GPIO and LED support
+ (GPIO_CS5535 & LEDS_GPIO) to actually use the LEDs
+
+ Note: You have to set alix.force=1 for boards with Award BIOS.
+
endif # X86_32
config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 021eee9..8d87439 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -1,6 +1,7 @@
# Platform specific code goes here
obj-y += ce4100/
obj-y += efi/
+obj-y += geode/
obj-y += iris/
obj-y += mrst/
obj-y += olpc/
diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
new file mode 100644
index 0000000..07c9cd0
--- /dev/null
+++ b/arch/x86/platform/geode/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ALIX) += alix.o
diff --git a/arch/x86/platform/geode/alix.c b/arch/x86/platform/geode/alix.c
new file mode 100644
index 0000000..f3f01e2
--- /dev/null
+++ b/arch/x86/platform/geode/alix.c
@@ -0,0 +1,141 @@
+/*
+ * System Specific setup for PCEngines ALIX.
+ * At the moment this means setup of GPIO control of LEDs
+ * on Alix.2/3/6 boards.
+ *
+ *
+ * Copyright (C) 2008 Constantin Baranov <[email protected]>
+ * Copyright (C) 2011 Ed Wildgoose <[email protected]>
+ *
+ * TODO: There are large similarities with leds-net5501.c
+ * by Alessandro Zummo <[email protected]>
+ * In the future leds-net5501.c should be migrated over to platform
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+
+#include <asm/geode.h>
+
+static int force = 0;
+module_param(force, bool, 0444);
+/* FIXME: Award bios is not automatically detected as Alix platform */
+MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3 platform");
+
+static struct gpio_led alix_leds[] = {
+ {
+ .name = "alix:1",
+ .gpio = 6,
+ .default_trigger = "default-on",
+ .active_low = 1,
+ },
+ {
+ .name = "alix:2",
+ .gpio = 25,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+ {
+ .name = "alix:3",
+ .gpio = 27,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+};
+
+static struct gpio_led_platform_data alix_leds_data = {
+ .num_leds = ARRAY_SIZE(alix_leds),
+ .leds = alix_leds,
+};
+
+static struct platform_device alix_leds_dev = {
+ .name = "leds-gpio",
+ .id = -1,
+ .dev.platform_data = &alix_leds_data,
+};
+
+static void __init register_alix(void)
+{
+ /* Setup LED control through leds-gpio driver */
+ platform_device_register(&alix_leds_dev);
+}
+
+static int __init alix_present(unsigned long bios_phys,
+ const char *alix_sig,
+ size_t alix_sig_len)
+{
+ const size_t bios_len = 0x00010000;
+ const char *bios_virt;
+ const char *scan_end;
+ const char *p;
+ char name[64];
+
+ if (force) {
+ printk(KERN_NOTICE "%s: forced to skip BIOS test, "
+ "assume system is ALIX.2/ALIX.3\n",
+ KBUILD_MODNAME);
+ return 1;
+ }
+
+ bios_virt = phys_to_virt(bios_phys);
+ scan_end = bios_virt + bios_len - (alix_sig_len + 2);
+ for (p = bios_virt; p < scan_end; p++) {
+ const char *tail;
+ char *a;
+
+ if (memcmp(p, alix_sig, alix_sig_len) != 0)
+ continue;
+
+ memcpy(name, p, sizeof(name));
+
+ /* remove the first \0 character from string */
+ a = strchr(name, '\0');
+ if (a)
+ *a = ' ';
+
+ /* cut the string at a newline */
+ a = strchr(name, '\r');
+ if (a)
+ *a = '\0';
+
+ tail = p + alix_sig_len;
+ if ((tail[0] == '2' || tail[0] == '3')) {
+ printk(KERN_INFO
+ "%s: system is recognized as \"%s\"\n",
+ KBUILD_MODNAME, name);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static int __init alix_init(void)
+{
+ const char tinybios_sig[] = "PC Engines ALIX.";
+ const char coreboot_sig[] = "PC Engines\0ALIX.";
+
+ if (!is_geode())
+ return 0;
+
+ if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
+ alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
+ register_alix();
+
+ return 0;
+}
+
+module_init(alix_init);
+
+MODULE_AUTHOR("Ed Wildgoose <[email protected]>");
+MODULE_DESCRIPTION("PCEngines ALIX System Setup");
+MODULE_LICENSE("GPL");
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6f190f4..e4573d6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -97,14 +97,6 @@ config LEDS_WRAP
help
This option enables support for the PCEngines WRAP programmable LEDs.
-config LEDS_ALIX2
- tristate "LED Support for ALIX.2 and ALIX.3 series"
- depends on LEDS_CLASS
- depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
- help
- This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
- You have to set leds-alix2.force=1 for boards with Award BIOS.
-
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index aae6989..bf5a00c 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_LEDS_AMS_DELTA) += leds-ams-delta.o
obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
obj-$(CONFIG_LEDS_NET5501) += leds-net5501.o
obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
-obj-$(CONFIG_LEDS_ALIX2) += leds-alix2.o
obj-$(CONFIG_LEDS_H1940) += leds-h1940.o
obj-$(CONFIG_LEDS_COBALT_QUBE) += leds-cobalt-qube.o
obj-$(CONFIG_LEDS_COBALT_RAQ) += leds-cobalt-raq.o
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
deleted file mode 100644
index f59ffad..0000000
--- a/drivers/leds/leds-alix2.c
+++ /dev/null
@@ -1,239 +0,0 @@
-/*
- * LEDs driver for PCEngines ALIX.2 and ALIX.3
- *
- * Copyright (C) 2008 Constantin Baranov <[email protected]>
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/leds.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/string.h>
-#include <linux/pci.h>
-
-static int force = 0;
-module_param(force, bool, 0444);
-MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style LEDs");
-
-#define MSR_LBAR_GPIO 0x5140000C
-#define CS5535_GPIO_SIZE 256
-
-static u32 gpio_base;
-
-static struct pci_device_id divil_pci[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_CS5535_ISA) },
- { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
- { } /* NULL entry */
-};
-MODULE_DEVICE_TABLE(pci, divil_pci);
-
-struct alix_led {
- struct led_classdev cdev;
- unsigned short port;
- unsigned int on_value;
- unsigned int off_value;
-};
-
-static void alix_led_set(struct led_classdev *led_cdev,
- enum led_brightness brightness)
-{
- struct alix_led *led_dev =
- container_of(led_cdev, struct alix_led, cdev);
-
- if (brightness)
- outl(led_dev->on_value, gpio_base + led_dev->port);
- else
- outl(led_dev->off_value, gpio_base + led_dev->port);
-}
-
-static struct alix_led alix_leds[] = {
- {
- .cdev = {
- .name = "alix:1",
- .brightness_set = alix_led_set,
- },
- .port = 0x00,
- .on_value = 1 << 22,
- .off_value = 1 << 6,
- },
- {
- .cdev = {
- .name = "alix:2",
- .brightness_set = alix_led_set,
- },
- .port = 0x80,
- .on_value = 1 << 25,
- .off_value = 1 << 9,
- },
- {
- .cdev = {
- .name = "alix:3",
- .brightness_set = alix_led_set,
- },
- .port = 0x80,
- .on_value = 1 << 27,
- .off_value = 1 << 11,
- },
-};
-
-static int __init alix_led_probe(struct platform_device *pdev)
-{
- int i;
- int ret;
-
- for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
- alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
- ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev);
- if (ret < 0)
- goto fail;
- }
- return 0;
-
-fail:
- while (--i >= 0)
- led_classdev_unregister(&alix_leds[i].cdev);
- return ret;
-}
-
-static int alix_led_remove(struct platform_device *pdev)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
- led_classdev_unregister(&alix_leds[i].cdev);
- return 0;
-}
-
-static struct platform_driver alix_led_driver = {
- .remove = alix_led_remove,
- .driver = {
- .name = KBUILD_MODNAME,
- .owner = THIS_MODULE,
- },
-};
-
-static int __init alix_present(unsigned long bios_phys,
- const char *alix_sig,
- size_t alix_sig_len)
-{
- const size_t bios_len = 0x00010000;
- const char *bios_virt;
- const char *scan_end;
- const char *p;
- char name[64];
-
- if (force) {
- printk(KERN_NOTICE "%s: forced to skip BIOS test, "
- "assume system has ALIX.2 style LEDs\n",
- KBUILD_MODNAME);
- return 1;
- }
-
- bios_virt = phys_to_virt(bios_phys);
- scan_end = bios_virt + bios_len - (alix_sig_len + 2);
- for (p = bios_virt; p < scan_end; p++) {
- const char *tail;
- char *a;
-
- if (memcmp(p, alix_sig, alix_sig_len) != 0)
- continue;
-
- memcpy(name, p, sizeof(name));
-
- /* remove the first \0 character from string */
- a = strchr(name, '\0');
- if (a)
- *a = ' ';
-
- /* cut the string at a newline */
- a = strchr(name, '\r');
- if (a)
- *a = '\0';
-
- tail = p + alix_sig_len;
- if ((tail[0] == '2' || tail[0] == '3')) {
- printk(KERN_INFO
- "%s: system is recognized as \"%s\"\n",
- KBUILD_MODNAME, name);
- return 1;
- }
- }
-
- return 0;
-}
-
-static struct platform_device *pdev;
-
-static int __init alix_pci_led_init(void)
-{
- u32 low, hi;
-
- if (pci_dev_present(divil_pci) == 0) {
- printk(KERN_WARNING KBUILD_MODNAME": DIVIL not found\n");
- return -ENODEV;
- }
-
- /* Grab the GPIO I/O range */
- rdmsr(MSR_LBAR_GPIO, low, hi);
-
- /* Check the mask and whether GPIO is enabled (sanity check) */
- if (hi != 0x0000f001) {
- printk(KERN_WARNING KBUILD_MODNAME": GPIO not enabled\n");
- return -ENODEV;
- }
-
- /* Mask off the IO base address */
- gpio_base = low & 0x0000ff00;
-
- if (!request_region(gpio_base, CS5535_GPIO_SIZE, KBUILD_MODNAME)) {
- printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O for GPIO\n");
- return -ENODEV;
- }
-
- /* Set GPIO function to output */
- outl(1 << 6, gpio_base + 0x04);
- outl(1 << 9, gpio_base + 0x84);
- outl(1 << 11, gpio_base + 0x84);
-
- return 0;
-}
-
-static int __init alix_led_init(void)
-{
- int ret = -ENODEV;
- const char tinybios_sig[] = "PC Engines ALIX.";
- const char coreboot_sig[] = "PC Engines\0ALIX.";
-
- if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
- alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
- ret = alix_pci_led_init();
-
- if (ret < 0)
- return ret;
-
- pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
- if (!IS_ERR(pdev)) {
- ret = platform_driver_probe(&alix_led_driver, alix_led_probe);
- if (ret)
- platform_device_unregister(pdev);
- } else
- ret = PTR_ERR(pdev);
-
- return ret;
-}
-
-static void __exit alix_led_exit(void)
-{
- platform_device_unregister(pdev);
- platform_driver_unregister(&alix_led_driver);
- release_region(gpio_base, CS5535_GPIO_SIZE);
-}
-
-module_init(alix_led_init);
-module_exit(alix_led_exit);
-
-MODULE_AUTHOR("Constantin Baranov <[email protected]>");
-MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
-MODULE_LICENSE("GPL");
--
1.7.4
On 19/03/2011 16:51, [email protected] wrote:
> From: Ed Wildgoose <[email protected]>
>
> This new driver replaces the old PCEngines Alix 2/3 LED driver with
> a new driver that controls the LEDs through the leds-gpio driver.
> The old driver accessed GPIOs directly, which created a conflict
> and prevented also loading the cs5535-gpio driver to read other
> GPIOs on the Alix board. With this new driver, we hook into leds-gpio
> which in turn uses GPIO to control the LEDs and therefore it's
> possible to control both the LEDs and access onboard GPIOs
>
> Driver is moved to platform/geode and any other geode
> initialisation modules should move here also.
>
> This driver is inspired by leds-net5501.c
> by: Alessandro Zummo <[email protected]>
> Ideally, leds-net5501.c should also be moved to platform/geode.
> Additionally the driver relies on parts of the patch: 7f131cf3ed
> by: Daniel Mack <[email protected]> to perform detection of the Alix board
> Signed-off-by: Ed Wildgoose <[email protected]>
Hi Grant
I *think* this patch should now be satisfactory and I believe to be
implemented as we discussed:
- I have made the driver look like a straightforward platform
initialisation routine and so further platform setup could be added here
if required.
My only thought is that it's quite tucked away and hard to find here,
given that at present it only enables LEDs on this platform (and seems
reasonably unlikely to grow beyond that). Would it be
acceptable/sensible to put the Kconfig entry under LEDs for the time
being (but the code under /platform/)? Obviously if it starts to do more
initialisation/setup then the KConfig can be moved back to /arch/x86/ ?
However, grateful if you could please now review this for inclusion?
I will submit a second patch momentarily which adds the extra dependency
of -gpio on -mfd as we discussed in the previous email
Many thanks
Ed W
From: Ed Wildgoose <[email protected]>
cs5535-gpio.c has been split into two, with various setup moved
into cs5535-mfd.c. Given that cs5535-gpio will not load without
the -mfd part, lets make that dependency explicit in Kconfig
Signed-off-by: Ed Wildgoose <[email protected]>
---
:100644 100644 664660e... 3246ad4... M drivers/gpio/Kconfig
drivers/gpio/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 664660e..3246ad4 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -295,7 +295,7 @@ comment "PCI GPIO expanders:"
config GPIO_CS5535
tristate "AMD CS5535/CS5536 GPIO support"
- depends on PCI && X86 && !CS5535_GPIO
+ depends on PCI && X86 && !CS5535_GPIO && MFD_CS5535
help
The AMD CS5535 and CS5536 southbridges support 28 GPIO pins that
can be used for quite a number of things. The CS5535/6 is found on
--
1.7.4
On Sat, 19 Mar 2011 17:46:25 +0000
[email protected] wrote:
> From: Ed Wildgoose <[email protected]>
>
> cs5535-gpio.c has been split into two, with various setup moved
> into cs5535-mfd.c. Given that cs5535-gpio will not load without
> the -mfd part, lets make that dependency explicit in Kconfig
Thanks!
Acked-by: Andres Salomon <[email protected]>
>
> Signed-off-by: Ed Wildgoose <[email protected]>
> ---
> :100644 100644 664660e... 3246ad4... M drivers/gpio/Kconfig
> drivers/gpio/Kconfig | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 664660e..3246ad4 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -295,7 +295,7 @@ comment "PCI GPIO expanders:"
>
> config GPIO_CS5535
> tristate "AMD CS5535/CS5536 GPIO support"
> - depends on PCI && X86 && !CS5535_GPIO
> + depends on PCI && X86 && !CS5535_GPIO && MFD_CS5535
> help
> The AMD CS5535 and CS5536 southbridges support 28 GPIO
> pins that can be used for quite a number of things. The CS5535/6 is
> found on
On Sat, Mar 19, 2011 at 04:51:06PM +0000, [email protected] wrote:
> From: Ed Wildgoose <[email protected]>
>
> This new driver replaces the old PCEngines Alix 2/3 LED driver with
> a new driver that controls the LEDs through the leds-gpio driver.
> The old driver accessed GPIOs directly, which created a conflict
> and prevented also loading the cs5535-gpio driver to read other
> GPIOs on the Alix board. With this new driver, we hook into leds-gpio
> which in turn uses GPIO to control the LEDs and therefore it's
> possible to control both the LEDs and access onboard GPIOs
>
> Driver is moved to platform/geode and any other geode
> initialisation modules should move here also.
>
> This driver is inspired by leds-net5501.c
> by: Alessandro Zummo <[email protected]>
> Ideally, leds-net5501.c should also be moved to platform/geode.
> Additionally the driver relies on parts of the patch: 7f131cf3ed
> by: Daniel Mack <[email protected]> to perform detection of the Alix board
> Signed-off-by: Ed Wildgoose <[email protected]>
> ---
[grafting in comments from 2nd email]
>
> Hi Grant
>
> I *think* this patch should now be satisfactory and I believe to be
> implemented as we discussed:
>
> - I have made the driver look like a straightforward platform
> initialisation routine and so further platform setup could be added here
> if required.
>
> My only thought is that it's quite tucked away and hard to find here,
> given that at present it only enables LEDs on this platform (and seems
> reasonably unlikely to grow beyond that). Would it be
> acceptable/sensible to put the Kconfig entry under LEDs for the time
> being (but the code under /platform/)? Obviously if it starts to do more
> initialisation/setup then the KConfig can be moved back to /arch/x86/ ?
>
> However, grateful if you could please now review this for inclusion?
>
> I will submit a second patch momentarily which adds the extra dependency
> of -gpio on -mfd as we discussed in the previous email
Hi Ed,
Reviewed-by: Grant Likely <[email protected]>
You'll need to get one of the x86 maintainers to pick this patch up.
Also, a few notes on patch submission etiquette...
(this is not a chastisement, just some things to remember for the next
patch you submit. You do not need to resubmit your patch)
When following up with new versions of a patch, it is common to
include the version number in the subject line in; something like
'[PATCH v2]', and to also include a changelog in the commit text that
describes what has changed. It makes it easier to know with patch a
reviewer should be looking at when there are multiple in the INBOX at
one time. The revision history also helps after a patch is committed
to know at a later date exactly which version actually made it into
Linux mainline.
Also if you want to add personal notes to a patch, you can simply add
them below the '---' cut line. Text above that line gets committed,
while text below it is culled when committing with git. So, instead
of replying to your own patch, you can put the text inline.
Cheers,
g.
---
> :100644 100644 d5ed94d... a10556d... M arch/x86/Kconfig
> :100644 100644 021eee9... 8d87439... M arch/x86/platform/Makefile
> :000000 100644 0000000... 07c9cd0... A arch/x86/platform/geode/Makefile
> :000000 100644 0000000... f3f01e2... A arch/x86/platform/geode/alix.c
> :100644 100644 6f190f4... e4573d6... M drivers/leds/Kconfig
> :100644 100644 aae6989... bf5a00c... M drivers/leds/Makefile
> :100644 000000 f59ffad... 0000000... D drivers/leds/leds-alix2.c
> arch/x86/Kconfig | 14 +++
> arch/x86/platform/Makefile | 1 +
> arch/x86/platform/geode/Makefile | 1 +
> arch/x86/platform/geode/alix.c | 141 ++++++++++++++++++++++
> drivers/leds/Kconfig | 8 --
> drivers/leds/Makefile | 1 -
> drivers/leds/leds-alix2.c | 239 --------------------------------------
> 7 files changed, 157 insertions(+), 248 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d5ed94d..a10556d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2094,6 +2094,20 @@ config OLPC_OPENFIRMWARE_DT
> default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
> select OF_PROMTREE
>
> +config ALIX
> + bool "PCEngines ALIX System Support (LED setup)"
> + select GPIOLIB
> + ---help---
> + This option enables system support for the PCEngines ALIX.
> + At present this just sets up LEDs for GPIO control on
> + ALIX2/3/6 boards. However, other system specific setup should
> + get added here.
> +
> + Note: You must still enable the drivers for GPIO and LED support
> + (GPIO_CS5535 & LEDS_GPIO) to actually use the LEDs
> +
> + Note: You have to set alix.force=1 for boards with Award BIOS.
> +
> endif # X86_32
>
> config AMD_NB
> diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
> index 021eee9..8d87439 100644
> --- a/arch/x86/platform/Makefile
> +++ b/arch/x86/platform/Makefile
> @@ -1,6 +1,7 @@
> # Platform specific code goes here
> obj-y += ce4100/
> obj-y += efi/
> +obj-y += geode/
> obj-y += iris/
> obj-y += mrst/
> obj-y += olpc/
> diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
> new file mode 100644
> index 0000000..07c9cd0
> --- /dev/null
> +++ b/arch/x86/platform/geode/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ALIX) += alix.o
> diff --git a/arch/x86/platform/geode/alix.c b/arch/x86/platform/geode/alix.c
> new file mode 100644
> index 0000000..f3f01e2
> --- /dev/null
> +++ b/arch/x86/platform/geode/alix.c
> @@ -0,0 +1,141 @@
> +/*
> + * System Specific setup for PCEngines ALIX.
> + * At the moment this means setup of GPIO control of LEDs
> + * on Alix.2/3/6 boards.
> + *
> + *
> + * Copyright (C) 2008 Constantin Baranov <[email protected]>
> + * Copyright (C) 2011 Ed Wildgoose <[email protected]>
> + *
> + * TODO: There are large similarities with leds-net5501.c
> + * by Alessandro Zummo <[email protected]>
> + * In the future leds-net5501.c should be migrated over to platform
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/string.h>
> +#include <linux/leds.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +
> +#include <asm/geode.h>
> +
> +static int force = 0;
> +module_param(force, bool, 0444);
> +/* FIXME: Award bios is not automatically detected as Alix platform */
> +MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3 platform");
> +
> +static struct gpio_led alix_leds[] = {
> + {
> + .name = "alix:1",
> + .gpio = 6,
> + .default_trigger = "default-on",
> + .active_low = 1,
> + },
> + {
> + .name = "alix:2",
> + .gpio = 25,
> + .default_trigger = "default-off",
> + .active_low = 1,
> + },
> + {
> + .name = "alix:3",
> + .gpio = 27,
> + .default_trigger = "default-off",
> + .active_low = 1,
> + },
> +};
> +
> +static struct gpio_led_platform_data alix_leds_data = {
> + .num_leds = ARRAY_SIZE(alix_leds),
> + .leds = alix_leds,
> +};
> +
> +static struct platform_device alix_leds_dev = {
> + .name = "leds-gpio",
> + .id = -1,
> + .dev.platform_data = &alix_leds_data,
> +};
> +
> +static void __init register_alix(void)
> +{
> + /* Setup LED control through leds-gpio driver */
> + platform_device_register(&alix_leds_dev);
> +}
> +
> +static int __init alix_present(unsigned long bios_phys,
> + const char *alix_sig,
> + size_t alix_sig_len)
> +{
> + const size_t bios_len = 0x00010000;
> + const char *bios_virt;
> + const char *scan_end;
> + const char *p;
> + char name[64];
> +
> + if (force) {
> + printk(KERN_NOTICE "%s: forced to skip BIOS test, "
> + "assume system is ALIX.2/ALIX.3\n",
> + KBUILD_MODNAME);
> + return 1;
> + }
> +
> + bios_virt = phys_to_virt(bios_phys);
> + scan_end = bios_virt + bios_len - (alix_sig_len + 2);
> + for (p = bios_virt; p < scan_end; p++) {
> + const char *tail;
> + char *a;
> +
> + if (memcmp(p, alix_sig, alix_sig_len) != 0)
> + continue;
> +
> + memcpy(name, p, sizeof(name));
> +
> + /* remove the first \0 character from string */
> + a = strchr(name, '\0');
> + if (a)
> + *a = ' ';
> +
> + /* cut the string at a newline */
> + a = strchr(name, '\r');
> + if (a)
> + *a = '\0';
> +
> + tail = p + alix_sig_len;
> + if ((tail[0] == '2' || tail[0] == '3')) {
> + printk(KERN_INFO
> + "%s: system is recognized as \"%s\"\n",
> + KBUILD_MODNAME, name);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __init alix_init(void)
> +{
> + const char tinybios_sig[] = "PC Engines ALIX.";
> + const char coreboot_sig[] = "PC Engines\0ALIX.";
> +
> + if (!is_geode())
> + return 0;
> +
> + if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
> + alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
> + register_alix();
> +
> + return 0;
> +}
> +
> +module_init(alix_init);
> +
> +MODULE_AUTHOR("Ed Wildgoose <[email protected]>");
> +MODULE_DESCRIPTION("PCEngines ALIX System Setup");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 6f190f4..e4573d6 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -97,14 +97,6 @@ config LEDS_WRAP
> help
> This option enables support for the PCEngines WRAP programmable LEDs.
>
> -config LEDS_ALIX2
> - tristate "LED Support for ALIX.2 and ALIX.3 series"
> - depends on LEDS_CLASS
> - depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
> - help
> - This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
> - You have to set leds-alix2.force=1 for boards with Award BIOS.
> -
> config LEDS_H1940
> tristate "LED Support for iPAQ H1940 device"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index aae6989..bf5a00c 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -15,7 +15,6 @@ obj-$(CONFIG_LEDS_AMS_DELTA) += leds-ams-delta.o
> obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
> obj-$(CONFIG_LEDS_NET5501) += leds-net5501.o
> obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
> -obj-$(CONFIG_LEDS_ALIX2) += leds-alix2.o
> obj-$(CONFIG_LEDS_H1940) += leds-h1940.o
> obj-$(CONFIG_LEDS_COBALT_QUBE) += leds-cobalt-qube.o
> obj-$(CONFIG_LEDS_COBALT_RAQ) += leds-cobalt-raq.o
> diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
> deleted file mode 100644
> index f59ffad..0000000
> --- a/drivers/leds/leds-alix2.c
> +++ /dev/null
> @@ -1,239 +0,0 @@
> -/*
> - * LEDs driver for PCEngines ALIX.2 and ALIX.3
> - *
> - * Copyright (C) 2008 Constantin Baranov <[email protected]>
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/kernel.h>
> -#include <linux/leds.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/string.h>
> -#include <linux/pci.h>
> -
> -static int force = 0;
> -module_param(force, bool, 0444);
> -MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style LEDs");
> -
> -#define MSR_LBAR_GPIO 0x5140000C
> -#define CS5535_GPIO_SIZE 256
> -
> -static u32 gpio_base;
> -
> -static struct pci_device_id divil_pci[] = {
> - { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_CS5535_ISA) },
> - { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
> - { } /* NULL entry */
> -};
> -MODULE_DEVICE_TABLE(pci, divil_pci);
> -
> -struct alix_led {
> - struct led_classdev cdev;
> - unsigned short port;
> - unsigned int on_value;
> - unsigned int off_value;
> -};
> -
> -static void alix_led_set(struct led_classdev *led_cdev,
> - enum led_brightness brightness)
> -{
> - struct alix_led *led_dev =
> - container_of(led_cdev, struct alix_led, cdev);
> -
> - if (brightness)
> - outl(led_dev->on_value, gpio_base + led_dev->port);
> - else
> - outl(led_dev->off_value, gpio_base + led_dev->port);
> -}
> -
> -static struct alix_led alix_leds[] = {
> - {
> - .cdev = {
> - .name = "alix:1",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x00,
> - .on_value = 1 << 22,
> - .off_value = 1 << 6,
> - },
> - {
> - .cdev = {
> - .name = "alix:2",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x80,
> - .on_value = 1 << 25,
> - .off_value = 1 << 9,
> - },
> - {
> - .cdev = {
> - .name = "alix:3",
> - .brightness_set = alix_led_set,
> - },
> - .port = 0x80,
> - .on_value = 1 << 27,
> - .off_value = 1 << 11,
> - },
> -};
> -
> -static int __init alix_led_probe(struct platform_device *pdev)
> -{
> - int i;
> - int ret;
> -
> - for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
> - alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
> - ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev);
> - if (ret < 0)
> - goto fail;
> - }
> - return 0;
> -
> -fail:
> - while (--i >= 0)
> - led_classdev_unregister(&alix_leds[i].cdev);
> - return ret;
> -}
> -
> -static int alix_led_remove(struct platform_device *pdev)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
> - led_classdev_unregister(&alix_leds[i].cdev);
> - return 0;
> -}
> -
> -static struct platform_driver alix_led_driver = {
> - .remove = alix_led_remove,
> - .driver = {
> - .name = KBUILD_MODNAME,
> - .owner = THIS_MODULE,
> - },
> -};
> -
> -static int __init alix_present(unsigned long bios_phys,
> - const char *alix_sig,
> - size_t alix_sig_len)
> -{
> - const size_t bios_len = 0x00010000;
> - const char *bios_virt;
> - const char *scan_end;
> - const char *p;
> - char name[64];
> -
> - if (force) {
> - printk(KERN_NOTICE "%s: forced to skip BIOS test, "
> - "assume system has ALIX.2 style LEDs\n",
> - KBUILD_MODNAME);
> - return 1;
> - }
> -
> - bios_virt = phys_to_virt(bios_phys);
> - scan_end = bios_virt + bios_len - (alix_sig_len + 2);
> - for (p = bios_virt; p < scan_end; p++) {
> - const char *tail;
> - char *a;
> -
> - if (memcmp(p, alix_sig, alix_sig_len) != 0)
> - continue;
> -
> - memcpy(name, p, sizeof(name));
> -
> - /* remove the first \0 character from string */
> - a = strchr(name, '\0');
> - if (a)
> - *a = ' ';
> -
> - /* cut the string at a newline */
> - a = strchr(name, '\r');
> - if (a)
> - *a = '\0';
> -
> - tail = p + alix_sig_len;
> - if ((tail[0] == '2' || tail[0] == '3')) {
> - printk(KERN_INFO
> - "%s: system is recognized as \"%s\"\n",
> - KBUILD_MODNAME, name);
> - return 1;
> - }
> - }
> -
> - return 0;
> -}
> -
> -static struct platform_device *pdev;
> -
> -static int __init alix_pci_led_init(void)
> -{
> - u32 low, hi;
> -
> - if (pci_dev_present(divil_pci) == 0) {
> - printk(KERN_WARNING KBUILD_MODNAME": DIVIL not found\n");
> - return -ENODEV;
> - }
> -
> - /* Grab the GPIO I/O range */
> - rdmsr(MSR_LBAR_GPIO, low, hi);
> -
> - /* Check the mask and whether GPIO is enabled (sanity check) */
> - if (hi != 0x0000f001) {
> - printk(KERN_WARNING KBUILD_MODNAME": GPIO not enabled\n");
> - return -ENODEV;
> - }
> -
> - /* Mask off the IO base address */
> - gpio_base = low & 0x0000ff00;
> -
> - if (!request_region(gpio_base, CS5535_GPIO_SIZE, KBUILD_MODNAME)) {
> - printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O for GPIO\n");
> - return -ENODEV;
> - }
> -
> - /* Set GPIO function to output */
> - outl(1 << 6, gpio_base + 0x04);
> - outl(1 << 9, gpio_base + 0x84);
> - outl(1 << 11, gpio_base + 0x84);
> -
> - return 0;
> -}
> -
> -static int __init alix_led_init(void)
> -{
> - int ret = -ENODEV;
> - const char tinybios_sig[] = "PC Engines ALIX.";
> - const char coreboot_sig[] = "PC Engines\0ALIX.";
> -
> - if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
> - alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
> - ret = alix_pci_led_init();
> -
> - if (ret < 0)
> - return ret;
> -
> - pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
> - if (!IS_ERR(pdev)) {
> - ret = platform_driver_probe(&alix_led_driver, alix_led_probe);
> - if (ret)
> - platform_device_unregister(pdev);
> - } else
> - ret = PTR_ERR(pdev);
> -
> - return ret;
> -}
> -
> -static void __exit alix_led_exit(void)
> -{
> - platform_device_unregister(pdev);
> - platform_driver_unregister(&alix_led_driver);
> - release_region(gpio_base, CS5535_GPIO_SIZE);
> -}
> -
> -module_init(alix_led_init);
> -module_exit(alix_led_exit);
> -
> -MODULE_AUTHOR("Constantin Baranov <[email protected]>");
> -MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
> -MODULE_LICENSE("GPL");
> --
> 1.7.4
>