2007-09-13 14:57:46

by Yoichi Yuasa

[permalink] [raw]
Subject: [PATCH][1/2] led: add Cobalt Raq LEDs support

Add Cobalt Raq LEDs support.

Signed-off-by: Yoichi Yuasa <[email protected]>

diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/Kconfig mips/drivers/leds/Kconfig
--- mips-orig/drivers/leds/Kconfig 2007-09-12 12:01:58.027251750 +0900
+++ mips/drivers/leds/Kconfig 2007-09-12 12:04:34.021000750 +0900
@@ -87,11 +87,18 @@ config LEDS_H1940
help
This option enables support for the LEDs on the h1940.

-config LEDS_COBALT
- tristate "LED Support for Cobalt Server front LED"
+config LEDS_COBALT_QUBE
+ tristate "LED Support for Cobalt Qube front LED"
depends on LEDS_CLASS && MIPS_COBALT
help
- This option enables support for the front LED on Cobalt Server
+ This option enables support for the front LEDs on Cobalt Qube/Qube2.
+
+config LEDS_COBALT_RAQ
+ bool "LED Support for Cobalt Raq series"
+ depends on LEDS_CLASS && MIPS_COBALT
+ select LEDS_TRIGGERS
+ help
+ This option enables support for LEDs on the Cobalt Raq series.

config LEDS_GPIO
tristate "LED Support for GPIO connected LEDs"
diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/Makefile mips/drivers/leds/Makefile
--- mips-orig/drivers/leds/Makefile 2007-09-12 12:01:58.035252250 +0900
+++ mips/drivers/leds/Makefile 2007-09-12 12:04:34.021000750 +0900
@@ -15,7 +15,8 @@ obj-$(CONFIG_LEDS_AMS_DELTA) += leds-am
obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
obj-$(CONFIG_LEDS_H1940) += leds-h1940.o
-obj-$(CONFIG_LEDS_COBALT) += leds-cobalt.o
+obj-$(CONFIG_LEDS_COBALT_QUBE) += leds-cobalt-qube.o
+obj-$(CONFIG_LEDS_COBALT_RAQ) += leds-cobalt-raq.o
obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o

# LED Triggers
diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt-qube.c mips/drivers/leds/leds-cobalt-qube.c
--- mips-orig/drivers/leds/leds-cobalt-qube.c 1970-01-01 09:00:00.000000000 +0900
+++ mips/drivers/leds/leds-cobalt-qube.c 2007-09-12 13:29:20.088085000 +0900
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2006 - Florian Fainelli <[email protected]>
+ *
+ * Control the Cobalt Qube/RaQ front LED
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/leds.h>
+#include <asm/mach-cobalt/cobalt.h>
+
+static void cobalt_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ if (brightness)
+ COBALT_LED_PORT = COBALT_LED_BAR_LEFT | COBALT_LED_BAR_RIGHT;
+ else
+ COBALT_LED_PORT = 0;
+}
+
+static struct led_classdev cobalt_led = {
+ .name = "cobalt-front-led",
+ .brightness_set = cobalt_led_set,
+ .default_trigger = "ide-disk",
+};
+
+static int __init cobalt_led_init(void)
+{
+ return led_classdev_register(NULL, &cobalt_led);
+}
+
+static void __exit cobalt_led_exit(void)
+{
+ led_classdev_unregister(&cobalt_led);
+}
+
+module_init(cobalt_led_init);
+module_exit(cobalt_led_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Front LED support for Cobalt Server");
+MODULE_AUTHOR("Florian Fainelli <[email protected]>");
diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt-raq.c mips/drivers/leds/leds-cobalt-raq.c
--- mips-orig/drivers/leds/leds-cobalt-raq.c 1970-01-01 09:00:00.000000000 +0900
+++ mips/drivers/leds/leds-cobalt-raq.c 2007-09-12 12:04:34.021000750 +0900
@@ -0,0 +1,135 @@
+/*
+ * Cobalt Raq/Raq2 LED driver.
+ *
+ * Copyright (C) 2007 Yoichi Yuasa <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <asm/io.h>
+
+#define LED_WEB 0x04
+#define LED_POWER_OFF 0x08
+
+static void __iomem *led_port;
+static u8 led_value;
+DEFINE_SPINLOCK(raq_led_lock);
+
+static void raq_web_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ spin_lock_irq(&raq_led_lock);
+
+ if (brightness)
+ led_value |= LED_WEB;
+ else
+ led_value &= ~LED_WEB;
+ writeb(led_value, led_port);
+
+ spin_unlock_irq(&raq_led_lock);
+}
+
+static struct led_classdev raq_web_led = {
+ .name = "raq-web-led",
+ .brightness_set = raq_web_led_set,
+};
+
+static void raq_power_off_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ spin_lock_irq(&raq_led_lock);
+
+ if (brightness)
+ led_value |= LED_POWER_OFF;
+ else
+ led_value &= ~LED_POWER_OFF;
+ writeb(led_value, led_port);
+
+ spin_unlock_irq(&raq_led_lock);
+}
+
+static struct led_classdev raq_power_off_led = {
+ .name = "raq-power-off-led",
+ .brightness_set = raq_power_off_led_set,
+ .default_trigger = "power-off",
+};
+
+static int __devinit cobalt_raq_led_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int retval;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EBUSY;
+
+ led_port = ioremap(res->start, res->end - res->start + 1);
+ if (!led_port)
+ return -ENOMEM;
+
+ retval = led_classdev_register(&pdev->dev, &raq_power_off_led);
+ if (retval)
+ goto err_iounmap;
+
+ retval = led_classdev_register(&pdev->dev, &raq_web_led);
+ if (retval)
+ goto err_unregister;
+
+ return 0;
+
+err_unregister:
+ led_classdev_unregister(&raq_power_off_led);
+
+err_iounmap:
+ iounmap(led_port);
+ led_port = NULL;
+
+ return retval;
+}
+
+static int __devexit cobalt_raq_led_remove(struct platform_device *pdev)
+{
+ if (led_port) {
+ iounmap(led_port);
+ led_port = NULL;
+ }
+
+ led_classdev_unregister(&raq_power_off_led);
+ led_classdev_unregister(&raq_web_led);
+
+ return 0;
+}
+
+static struct platform_driver cobalt_raq_led_driver = {
+ .probe = cobalt_raq_led_probe,
+ .remove = __devexit_p(cobalt_raq_led_remove),
+ .driver = {
+ .name = "Cobalt Raq LEDs",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init cobalt_raq_led_init(void)
+{
+ return platform_driver_register(&cobalt_raq_led_driver);
+}
+
+module_init(cobalt_raq_led_init);
diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt.c mips/drivers/leds/leds-cobalt.c
--- mips-orig/drivers/leds/leds-cobalt.c 2007-09-12 12:01:58.083255250 +0900
+++ mips/drivers/leds/leds-cobalt.c 1970-01-01 09:00:00.000000000 +0900
@@ -1,43 +0,0 @@
-/*
- * Copyright 2006 - Florian Fainelli <[email protected]>
- *
- * Control the Cobalt Qube/RaQ front LED
- */
-
-#include <linux/module.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/device.h>
-#include <linux/leds.h>
-#include <asm/mach-cobalt/cobalt.h>
-
-static void cobalt_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
-{
- if (brightness)
- COBALT_LED_PORT = COBALT_LED_BAR_LEFT | COBALT_LED_BAR_RIGHT;
- else
- COBALT_LED_PORT = 0;
-}
-
-static struct led_classdev cobalt_led = {
- .name = "cobalt-front-led",
- .brightness_set = cobalt_led_set,
- .default_trigger = "ide-disk",
-};
-
-static int __init cobalt_led_init(void)
-{
- return led_classdev_register(NULL, &cobalt_led);
-}
-
-static void __exit cobalt_led_exit(void)
-{
- led_classdev_unregister(&cobalt_led);
-}
-
-module_init(cobalt_led_init);
-module_exit(cobalt_led_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Front LED support for Cobalt Server");
-MODULE_AUTHOR("Florian Fainelli <[email protected]>");


2007-09-13 14:57:34

by Yoichi Yuasa

[permalink] [raw]
Subject: [PATCH][2/2] led: update Cobalt Qube LED support

Update Cobalt Qube front LED support.

Signed-off-by: Yoichi Yuasa <[email protected]>

diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt-qube.c mips/drivers/leds/leds-cobalt-qube.c
--- mips-orig/drivers/leds/leds-cobalt-qube.c 2007-08-24 19:35:51.689587500 +0900
+++ mips/drivers/leds/leds-cobalt-qube.c 2007-08-24 19:57:45.391688750 +0900
@@ -1,43 +1,103 @@
/*
* Copyright 2006 - Florian Fainelli <[email protected]>
*
- * Control the Cobalt Qube/RaQ front LED
+ * Control the Cobalt Qube/Qube2 front LED
*/
-
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/leds.h>
#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/device.h>
-#include <linux/leds.h>
-#include <asm/mach-cobalt/cobalt.h>

-static void cobalt_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+#include <asm/io.h>
+
+#define LED_BAR_LEFT 0x01
+#define LED_BAR_RIGHT 0x02
+
+static void __iomem *led_port;
+static u8 led_value;
+
+static void qube_bar_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
{
if (brightness)
- COBALT_LED_PORT = COBALT_LED_BAR_LEFT | COBALT_LED_BAR_RIGHT;
+ led_value = LED_BAR_LEFT | LED_BAR_RIGHT;
else
- COBALT_LED_PORT = 0;
+ led_value = ~(LED_BAR_LEFT | LED_BAR_RIGHT);
+ writeb(led_value, led_port);
+}
+
+static struct led_classdev qube_bar_led = {
+ .name = "qube-bar-led",
+ .brightness = LED_FULL,
+ .brightness_set = qube_bar_led_set,
+ .default_trigger = "ide-disk",
+};
+
+static int __devinit cobalt_qube_led_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int retval;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EBUSY;
+
+ led_port = ioremap(res->start, res->end - res->start + 1);
+ if (!led_port)
+ return -ENOMEM;
+
+ led_value = LED_BAR_LEFT | LED_BAR_RIGHT;
+ writeb(led_value, led_port);
+
+ retval = led_classdev_register(&pdev->dev, &qube_bar_led);
+ if (retval)
+ goto err_iounmap;
+
+ return 0;
+
+err_iounmap:
+ iounmap(led_port);
+ led_port = NULL;
+
+ return retval;
+}
+
+static int __devexit cobalt_qube_led_remove(struct platform_device *pdev)
+{
+ if (led_port) {
+ iounmap(led_port);
+ led_port = NULL;
+ }
+
+ led_classdev_unregister(&qube_bar_led);
+
+ return 0;
}

-static struct led_classdev cobalt_led = {
- .name = "cobalt-front-led",
- .brightness_set = cobalt_led_set,
- .default_trigger = "ide-disk",
+static struct platform_driver cobalt_qube_led_driver = {
+ .probe = cobalt_qube_led_probe,
+ .remove = __devexit_p(cobalt_qube_led_remove),
+ .driver = {
+ .name = "Cobalt Qube LEDs",
+ .owner = THIS_MODULE,
+ },
};

-static int __init cobalt_led_init(void)
+static int __init cobalt_qube_led_init(void)
{
- return led_classdev_register(NULL, &cobalt_led);
+ return platform_driver_register(&cobalt_qube_led_driver);
}

-static void __exit cobalt_led_exit(void)
+static void __exit cobalt_qube_led_exit(void)
{
- led_classdev_unregister(&cobalt_led);
+ platform_driver_unregister(&cobalt_qube_led_driver);
}

-module_init(cobalt_led_init);
-module_exit(cobalt_led_exit);
+module_init(cobalt_qube_led_init);
+module_exit(cobalt_qube_led_exit);

MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Front LED support for Cobalt Server");
+MODULE_DESCRIPTION("Front LED support for Cobalt Qube");
MODULE_AUTHOR("Florian Fainelli <[email protected]>");

2007-09-13 21:32:34

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH][1/2] led: add Cobalt Raq LEDs support

On Thu, 2007-09-13 at 23:54 +0900, Yoichi Yuasa wrote:
> Add Cobalt Raq LEDs support.
>
> Signed-off-by: Yoichi Yuasa <[email protected]>

Not the clearest patch I've ever seen or the most helpful patch
description. The rename could probably be split from the additional new
driver at least...

> diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt-raq.c mips/drivers/leds/leds-cobalt-raq.c
> --- mips-orig/drivers/leds/leds-cobalt-raq.c 1970-01-01 09:00:00.000000000 +0900
> +++ mips/drivers/leds/leds-cobalt-raq.c 2007-09-12 12:04:34.021000750 +0900
[...]
> +static void raq_web_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + spin_lock_irq(&raq_led_lock);
> +
> + if (brightness)
> + led_value |= LED_WEB;
> + else
> + led_value &= ~LED_WEB;
> + writeb(led_value, led_port);
> +
> + spin_unlock_irq(&raq_led_lock);
> +}

The _irq spinlock variants are not safe there or in the other brightness
set function...

+static struct led_classdev raq_web_led = {
> + .name = "raq-web-led",
> + .brightness_set = raq_web_led_set,
> +};
> +
[...]
> +static struct led_classdev raq_power_off_led = {
> + .name = "raq-power-off-led",
> + .brightness_set = raq_power_off_led_set,
> + .default_trigger = "power-off",
> +};

Do these really need "led" in the name?

What does a power-off trigger do with an LED out of interest?

> +static int __devexit cobalt_raq_led_remove(struct platform_device *pdev)
> +{
> + if (led_port) {
> + iounmap(led_port);
> + led_port = NULL;
> + }
> +
> + led_classdev_unregister(&raq_power_off_led);
> + led_classdev_unregister(&raq_web_led);
> +
> + return 0;
> +}

You want to unregister, then iounmap...

> +static int __init cobalt_raq_led_init(void)
> +{
> + return platform_driver_register(&cobalt_raq_led_driver);
> +}
> +
> +module_init(cobalt_raq_led_init);

No module_exit in one driver for any particular reason?

Some of these comments also apply to you second patch. All minor tweaks
really though :).

Cheers,

Richard

2007-09-14 00:53:45

by Yoichi Yuasa

[permalink] [raw]
Subject: Re: [PATCH][1/2] led: add Cobalt Raq LEDs support

On Thu, 13 Sep 2007 22:32:12 +0100
Richard Purdie <[email protected]> wrote:

> On Thu, 2007-09-13 at 23:54 +0900, Yoichi Yuasa wrote:
> > Add Cobalt Raq LEDs support.
> >
> > Signed-off-by: Yoichi Yuasa <[email protected]>
>
> Not the clearest patch I've ever seen or the most helpful patch
> description. The rename could probably be split from the additional new
> driver at least...

OK, I'll split it.

> > diff -pruN -X mips/Documentation/dontdiff mips-orig/drivers/leds/leds-cobalt-raq.c mips/drivers/leds/leds-cobalt-raq.c
> > --- mips-orig/drivers/leds/leds-cobalt-raq.c 1970-01-01 09:00:00.000000000 +0900
> > +++ mips/drivers/leds/leds-cobalt-raq.c 2007-09-12 12:04:34.021000750 +0900
> [...]
> > +static void raq_web_led_set(struct led_classdev *led_cdev,
> > + enum led_brightness brightness)
> > +{
> > + spin_lock_irq(&raq_led_lock);
> > +
> > + if (brightness)
> > + led_value |= LED_WEB;
> > + else
> > + led_value &= ~LED_WEB;
> > + writeb(led_value, led_port);
> > +
> > + spin_unlock_irq(&raq_led_lock);
> > +}
>
> The _irq spinlock variants are not safe there or in the other brightness
> set function...
>
> +static struct led_classdev raq_web_led = {
> > + .name = "raq-web-led",
> > + .brightness_set = raq_web_led_set,
> > +};
> > +
> [...]
> > +static struct led_classdev raq_power_off_led = {
> > + .name = "raq-power-off-led",
> > + .brightness_set = raq_power_off_led_set,
> > + .default_trigger = "power-off",
> > +};
>
> Do these really need "led" in the name?
>
> What does a power-off trigger do with an LED out of interest?

It lights when the Cobalt Raq is possible to turn off power.

>
> > +static int __devexit cobalt_raq_led_remove(struct platform_device *pdev)
> > +{
> > + if (led_port) {
> > + iounmap(led_port);
> > + led_port = NULL;
> > + }
> > +
> > + led_classdev_unregister(&raq_power_off_led);
> > + led_classdev_unregister(&raq_web_led);
> > +
> > + return 0;
> > +}
>
> You want to unregister, then iounmap...
>
> > +static int __init cobalt_raq_led_init(void)
> > +{
> > + return platform_driver_register(&cobalt_raq_led_driver);
> > +}
> > +
> > +module_init(cobalt_raq_led_init);
>
> No module_exit in one driver for any particular reason?

This driver cares power-off LED.
Therefore, this driver is not tristate.

> Some of these comments also apply to you second patch. All minor tweaks
> really though :).

Thank you for your comments.

Yoichi