Add LED support for the HTC ASIC3. Underlying support is provided by the mfd/asic3 and leds/leds-asic3 drivers. An example configuration is provided by the pxa/hx4700 platform.
Signed-off-by: Paul Parsons <[email protected]>
---
diff -uprN clean-2.6.39-rc5/drivers/mfd/asic3.c linux-2.6.39-rc5/drivers/mfd/asic3.c
--- clean-2.6.39-rc5/drivers/mfd/asic3.c 2011-04-28 11:18:15.622582797 +0100
+++ linux-2.6.39-rc5/drivers/mfd/asic3.c 2011-04-30 15:44:30.288890098 +0100
@@ -88,19 +88,19 @@ struct asic3 {
static int asic3_gpio_get(struct gpio_chip *chip, unsigned offset);
-static inline void asic3_write_register(struct asic3 *asic,
- unsigned int reg, u32 value)
+void asic3_write_register(struct asic3 *asic, unsigned int reg, u32 value)
{
iowrite16(value, asic->mapping +
(reg >> asic->bus_shift));
}
+EXPORT_SYMBOL_GPL(asic3_write_register);
-static inline u32 asic3_read_register(struct asic3 *asic,
- unsigned int reg)
+u32 asic3_read_register(struct asic3 *asic, unsigned int reg)
{
return ioread16(asic->mapping +
(reg >> asic->bus_shift));
}
+EXPORT_SYMBOL_GPL(asic3_read_register);
static void asic3_set_register(struct asic3 *asic, u32 reg, u32 bits, bool set)
{
@@ -584,7 +584,7 @@ static int asic3_gpio_remove(struct plat
return gpiochip_remove(&asic->gpio);
}
-static int asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
+static void asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
{
unsigned long flags;
u32 cdex;
@@ -596,8 +596,6 @@ static int asic3_clk_enable(struct asic3
asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
}
spin_unlock_irqrestore(&asic->lock, flags);
-
- return 0;
}
static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
@@ -782,7 +780,55 @@ static struct mfd_cell asic3_cell_mmc =
.resources = asic3_mmc_resources,
};
+static const int clock_ledn[ASIC3_NUM_LEDS] = {
+ [0] = ASIC3_CLOCK_LED0,
+ [1] = ASIC3_CLOCK_LED1,
+ [2] = ASIC3_CLOCK_LED2,
+};
+
+static int asic3_leds_enable(struct platform_device *pdev)
+{
+ const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+
+ asic3_clk_enable(asic, &asic->clocks[clock_ledn[cell->id]]);
+
+ return 0;
+}
+
+static int asic3_leds_disable(struct platform_device *pdev)
+{
+ const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+
+ asic3_clk_disable(asic, &asic->clocks[clock_ledn[cell->id]]);
+
+ return 0;
+}
+
+static struct mfd_cell asic3_cell_leds[ASIC3_NUM_LEDS] = {
+ [0] = {
+ .name = "leds-asic3",
+ .id = 0,
+ .enable = asic3_leds_enable,
+ .disable = asic3_leds_disable,
+ },
+ [1] = {
+ .name = "leds-asic3",
+ .id = 1,
+ .enable = asic3_leds_enable,
+ .disable = asic3_leds_disable,
+ },
+ [2] = {
+ .name = "leds-asic3",
+ .id = 2,
+ .enable = asic3_leds_enable,
+ .disable = asic3_leds_disable,
+ },
+};
+
static int __init asic3_mfd_probe(struct platform_device *pdev,
+ struct asic3_platform_data *pdata,
struct resource *mem)
{
struct asic3 *asic = platform_get_drvdata(pdev);
@@ -820,9 +866,20 @@ static int __init asic3_mfd_probe(struct
if (ret < 0)
goto out;
- if (mem_sdio && (irq >= 0))
+ if (mem_sdio && (irq >= 0)) {
ret = mfd_add_devices(&pdev->dev, pdev->id,
&asic3_cell_mmc, 1, mem_sdio, irq);
+ if (ret < 0)
+ goto out;
+ }
+
+ if (pdata->leds) {
+ asic3_cell_leds[0].mfd_data = &pdata->leds[0];
+ asic3_cell_leds[1].mfd_data = &pdata->leds[1];
+ asic3_cell_leds[2].mfd_data = &pdata->leds[2];
+ ret = mfd_add_devices(&pdev->dev, 0,
+ asic3_cell_leds, ASIC3_NUM_LEDS, NULL, 0);
+ }
out:
return ret;
@@ -883,6 +940,7 @@ static int __init asic3_probe(struct pla
goto out_unmap;
}
+ asic->gpio.label = "asic3";
asic->gpio.base = pdata->gpio_base;
asic->gpio.ngpio = ASIC3_NUM_GPIOS;
asic->gpio.get = asic3_gpio_get;
@@ -903,7 +961,7 @@ static int __init asic3_probe(struct pla
*/
memcpy(asic->clocks, asic3_clk_init, sizeof(asic3_clk_init));
- asic3_mfd_probe(pdev, mem);
+ asic3_mfd_probe(pdev, pdata, mem);
dev_info(asic->dev, "ASIC3 Core driver\n");
diff -uprN clean-2.6.39-rc5/include/linux/mfd/asic3.h linux-2.6.39-rc5/include/linux/mfd/asic3.h
--- clean-2.6.39-rc5/include/linux/mfd/asic3.h 2011-03-15 01:20:32.000000000 +0000
+++ linux-2.6.39-rc5/include/linux/mfd/asic3.h 2011-04-30 16:06:11.893286349 +0100
@@ -16,6 +16,13 @@
#include <linux/types.h>
+struct led_classdev;
+struct asic3_led {
+ const char *name;
+ const char *default_trigger;
+ struct led_classdev *cdev;
+};
+
struct asic3_platform_data {
u16 *gpio_config;
unsigned int gpio_config_num;
@@ -23,6 +30,8 @@ struct asic3_platform_data {
unsigned int irq_base;
unsigned int gpio_base;
+
+ struct asic3_led *leds;
};
#define ASIC3_NUM_GPIO_BANKS 4
@@ -111,9 +120,9 @@ struct asic3_platform_data {
#define ASIC3_GPIOA11_PWM0 ASIC3_CONFIG_GPIO(11, 1, 1, 0)
#define ASIC3_GPIOA12_PWM1 ASIC3_CONFIG_GPIO(12, 1, 1, 0)
#define ASIC3_GPIOA15_CONTROL_CX ASIC3_CONFIG_GPIO(15, 1, 1, 0)
-#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 1, 0)
-#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 1, 0)
-#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 1, 0)
+#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 0, 0)
+#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 0, 0)
+#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 0, 0)
#define ASIC3_GPIOC3_SPI_RXD ASIC3_CONFIG_GPIO(35, 1, 0, 0)
#define ASIC3_GPIOC4_CF_nCD ASIC3_CONFIG_GPIO(36, 1, 0, 0)
#define ASIC3_GPIOC4_SPI_TXD ASIC3_CONFIG_GPIO(36, 1, 1, 0)
@@ -152,6 +161,7 @@ struct asic3_platform_data {
#define PWM_TIMEBASE_VALUE(x) ((x)&0xf) /* Low 4 bits sets time base */
#define PWM_TIMEBASE_ENABLE (1 << 4) /* Enable clock */
+#define ASIC3_NUM_LEDS 3
#define ASIC3_LED_0_Base 0x0700
#define ASIC3_LED_1_Base 0x0800
#define ASIC3_LED_2_Base 0x0900
@@ -293,4 +303,10 @@ struct asic3_platform_data {
#define ASIC3_MAP_SIZE_32BIT 0x2000
#define ASIC3_MAP_SIZE_16BIT 0x1000
+/* Functions needed by leds-asic3 */
+
+struct asic3;
+extern void asic3_write_register(struct asic3 *asic, unsigned int reg, u32 val);
+extern u32 asic3_read_register(struct asic3 *asic, unsigned int reg);
+
#endif /* __ASIC3_H__ */
Hi Paul,
On Sat, Apr 30, 2011 at 05:59:25PM +0000, Paul Parsons wrote:
> Add LED support for the HTC ASIC3. Underlying support is provided by the mfd/asic3 and leds/leds-asic3 drivers.
>
Where is the LED driver ?
> -static int asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
> +static void asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
> {
> unsigned long flags;
> u32 cdex;
> @@ -596,8 +596,6 @@ static int asic3_clk_enable(struct asic3
> asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
> }
> spin_unlock_irqrestore(&asic->lock, flags);
> -
> - return 0;
> }
Although a valid change, this is unrelated to this patch.
> @@ -883,6 +940,7 @@ static int __init asic3_probe(struct pla
> goto out_unmap;
> }
>
> + asic->gpio.label = "asic3";
Ditto.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
Hi Samuel,
--- On Mon, 2/5/11, Samuel Ortiz <[email protected]> wrote:
> Where is the LED driver ?
It was posted separately - [PATCH 2/2] leds: Add ASIC3 LED support - since drivers/mfd and drivers/leds are maintained separately. Sorry, I should have cross copied them.
> > -static int asic3_clk_enable(struct asic3 *asic,
> struct asic3_clk *clk)
> > +static void asic3_clk_enable(struct asic3 *asic,
> struct asic3_clk *clk)
> Although a valid change, this is unrelated to this patch.
Not completely unrelated; I call asic3_clk_enable() and found it simpler to remove the unnecessary return value rather than check it.
> > +??? asic->gpio.label = "asic3";
> Ditto.
Agreed; that was left in by mistake.
Should I resubmit?
Regards,
Paul
Hi Paul,
On Mon, May 02, 2011 at 11:16:23PM +0100, Paul Parsons wrote:
> Hi Samuel,
>
> --- On Mon, 2/5/11, Samuel Ortiz <[email protected]> wrote:
> > Where is the LED driver ?
> It was posted separately - [PATCH 2/2] leds: Add ASIC3 LED support - since drivers/mfd and drivers/leds are maintained separately. Sorry, I should have cross copied them.
>
I believe the LED driver is dependent on the asic3.h changes this patches
carry. So both patches should go in at the same time, probably through my
tree.
> > > -static int asic3_clk_enable(struct asic3 *asic,
> > struct asic3_clk *clk)
> > > +static void asic3_clk_enable(struct asic3 *asic,
> > struct asic3_clk *clk)
> > Although a valid change, this is unrelated to this patch.
> Not completely unrelated; I call asic3_clk_enable() and found it simpler to remove the unnecessary return value rather than check it.
>
I understood that. And I am fine with the change, but I would like it to be a
separate patch.
> > > +??? asic->gpio.label = "asic3";
> > Ditto.
> Agreed; that was left in by mistake.
>
> Should I resubmit?
Yes, please. Also, I'm deprecating the mfd_data usage in my mfd-2.6 tree, so
you should fix your code according to that.
Thanks in advance.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
Am Samstag, den 30.04.2011, 17:59 +0000 schrieb Paul Parsons:
> Add LED support for the HTC ASIC3. Underlying support is provided by the mfd/asic3 and leds/leds-asic3 drivers. An example configuration is provided by the pxa/hx4700 platform.
>
> Signed-off-by: Paul Parsons <[email protected]>
> ---
> diff -uprN clean-2.6.39-rc5/drivers/mfd/asic3.c linux-2.6.39-rc5/drivers/mfd/asic3.c
> --- clean-2.6.39-rc5/drivers/mfd/asic3.c 2011-04-28 11:18:15.622582797 +0100
> +++ linux-2.6.39-rc5/drivers/mfd/asic3.c 2011-04-30 15:44:30.288890098 +0100
> @@ -88,19 +88,19 @@ struct asic3 {
>
> static int asic3_gpio_get(struct gpio_chip *chip, unsigned offset);
>
> -static inline void asic3_write_register(struct asic3 *asic,
> - unsigned int reg, u32 value)
> +void asic3_write_register(struct asic3 *asic, unsigned int reg, u32 value)
> {
> iowrite16(value, asic->mapping +
> (reg >> asic->bus_shift));
> }
> +EXPORT_SYMBOL_GPL(asic3_write_register);
I'd like to avoid this export. Could you inform the cell driver of
mapping and bus shift via io resources as it is done in ds1wm?
> -static inline u32 asic3_read_register(struct asic3 *asic,
> - unsigned int reg)
> +u32 asic3_read_register(struct asic3 *asic, unsigned int reg)
> {
> return ioread16(asic->mapping +
> (reg >> asic->bus_shift));
> }
> +EXPORT_SYMBOL_GPL(asic3_read_register);
Same for this one.
> static void asic3_set_register(struct asic3 *asic, u32 reg, u32 bits, bool set)
> {
> @@ -584,7 +584,7 @@ static int asic3_gpio_remove(struct plat
> return gpiochip_remove(&asic->gpio);
> }
>
> -static int asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
> +static void asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
> {
> unsigned long flags;
> u32 cdex;
> @@ -596,8 +596,6 @@ static int asic3_clk_enable(struct asic3
> asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
> }
> spin_unlock_irqrestore(&asic->lock, flags);
> -
> - return 0;
> }
Ok for now, I guess. But it's unrelated to led support. And once we have
a common struct clk and can use clk_enable in the led driver, there's
going to be error checking anyway...
> static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
> @@ -782,7 +780,55 @@ static struct mfd_cell asic3_cell_mmc =
> .resources = asic3_mmc_resources,
> };
>
> +static const int clock_ledn[ASIC3_NUM_LEDS] = {
> + [0] = ASIC3_CLOCK_LED0,
> + [1] = ASIC3_CLOCK_LED1,
> + [2] = ASIC3_CLOCK_LED2,
> +};
> +
> +static int asic3_leds_enable(struct platform_device *pdev)
> +{
> + const struct mfd_cell *cell = mfd_get_cell(pdev);
> + struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> +
> + asic3_clk_enable(asic, &asic->clocks[clock_ledn[cell->id]]);
> +
> + return 0;
> +}
> +
> +static int asic3_leds_disable(struct platform_device *pdev)
> +{
> + const struct mfd_cell *cell = mfd_get_cell(pdev);
> + struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> +
> + asic3_clk_disable(asic, &asic->clocks[clock_ledn[cell->id]]);
> +
> + return 0;
> +}
> +
> +static struct mfd_cell asic3_cell_leds[ASIC3_NUM_LEDS] = {
> + [0] = {
> + .name = "leds-asic3",
> + .id = 0,
> + .enable = asic3_leds_enable,
> + .disable = asic3_leds_disable,
> + },
> + [1] = {
> + .name = "leds-asic3",
> + .id = 1,
> + .enable = asic3_leds_enable,
> + .disable = asic3_leds_disable,
> + },
> + [2] = {
> + .name = "leds-asic3",
> + .id = 2,
> + .enable = asic3_leds_enable,
> + .disable = asic3_leds_disable,
> + },
> +};
> +
> static int __init asic3_mfd_probe(struct platform_device *pdev,
> + struct asic3_platform_data *pdata,
> struct resource *mem)
> {
> struct asic3 *asic = platform_get_drvdata(pdev);
> @@ -820,9 +866,20 @@ static int __init asic3_mfd_probe(struct
> if (ret < 0)
> goto out;
>
> - if (mem_sdio && (irq >= 0))
> + if (mem_sdio && (irq >= 0)) {
> ret = mfd_add_devices(&pdev->dev, pdev->id,
> &asic3_cell_mmc, 1, mem_sdio, irq);
> + if (ret < 0)
> + goto out;
> + }
> +
> + if (pdata->leds) {
> + asic3_cell_leds[0].mfd_data = &pdata->leds[0];
> + asic3_cell_leds[1].mfd_data = &pdata->leds[1];
> + asic3_cell_leds[2].mfd_data = &pdata->leds[2];
As Samuel pointed out, better use .platform_data and .platform_size here
right away.
> + ret = mfd_add_devices(&pdev->dev, 0,
> + asic3_cell_leds, ASIC3_NUM_LEDS, NULL, 0);
> + }
>
> out:
> return ret;
> @@ -883,6 +940,7 @@ static int __init asic3_probe(struct pla
> goto out_unmap;
> }
>
> + asic->gpio.label = "asic3";
Separate patch?
> asic->gpio.base = pdata->gpio_base;
> asic->gpio.ngpio = ASIC3_NUM_GPIOS;
> asic->gpio.get = asic3_gpio_get;
> @@ -903,7 +961,7 @@ static int __init asic3_probe(struct pla
> */
> memcpy(asic->clocks, asic3_clk_init, sizeof(asic3_clk_init));
>
> - asic3_mfd_probe(pdev, mem);
> + asic3_mfd_probe(pdev, pdata, mem);
>
> dev_info(asic->dev, "ASIC3 Core driver\n");
>
> diff -uprN clean-2.6.39-rc5/include/linux/mfd/asic3.h linux-2.6.39-rc5/include/linux/mfd/asic3.h
> --- clean-2.6.39-rc5/include/linux/mfd/asic3.h 2011-03-15 01:20:32.000000000 +0000
> +++ linux-2.6.39-rc5/include/linux/mfd/asic3.h 2011-04-30 16:06:11.893286349 +0100
> @@ -16,6 +16,13 @@
>
> #include <linux/types.h>
>
> +struct led_classdev;
> +struct asic3_led {
> + const char *name;
> + const char *default_trigger;
> + struct led_classdev *cdev;
> +};
> +
> struct asic3_platform_data {
> u16 *gpio_config;
> unsigned int gpio_config_num;
> @@ -23,6 +30,8 @@ struct asic3_platform_data {
> unsigned int irq_base;
>
> unsigned int gpio_base;
> +
> + struct asic3_led *leds;
> };
>
> #define ASIC3_NUM_GPIO_BANKS 4
> @@ -111,9 +120,9 @@ struct asic3_platform_data {
> #define ASIC3_GPIOA11_PWM0 ASIC3_CONFIG_GPIO(11, 1, 1, 0)
> #define ASIC3_GPIOA12_PWM1 ASIC3_CONFIG_GPIO(12, 1, 1, 0)
> #define ASIC3_GPIOA15_CONTROL_CX ASIC3_CONFIG_GPIO(15, 1, 1, 0)
> -#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 1, 0)
> -#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 1, 0)
> -#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 1, 0)
> +#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 0, 0)
> +#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 0, 0)
> +#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 0, 0)
As I said, not convinced about that one. I'll follow up with a patch to
test on top of yours.
> #define ASIC3_GPIOC3_SPI_RXD ASIC3_CONFIG_GPIO(35, 1, 0, 0)
> #define ASIC3_GPIOC4_CF_nCD ASIC3_CONFIG_GPIO(36, 1, 0, 0)
> #define ASIC3_GPIOC4_SPI_TXD ASIC3_CONFIG_GPIO(36, 1, 1, 0)
> @@ -152,6 +161,7 @@ struct asic3_platform_data {
> #define PWM_TIMEBASE_VALUE(x) ((x)&0xf) /* Low 4 bits sets time base */
> #define PWM_TIMEBASE_ENABLE (1 << 4) /* Enable clock */
>
> +#define ASIC3_NUM_LEDS 3
> #define ASIC3_LED_0_Base 0x0700
> #define ASIC3_LED_1_Base 0x0800
> #define ASIC3_LED_2_Base 0x0900
> @@ -293,4 +303,10 @@ struct asic3_platform_data {
> #define ASIC3_MAP_SIZE_32BIT 0x2000
> #define ASIC3_MAP_SIZE_16BIT 0x1000
>
> +/* Functions needed by leds-asic3 */
> +
> +struct asic3;
> +extern void asic3_write_register(struct asic3 *asic, unsigned int reg, u32 val);
> +extern u32 asic3_read_register(struct asic3 *asic, unsigned int reg);
> +
> #endif /* __ASIC3_H__ */
regards
Philipp
Hi Paul,
This patch reverts your ASIC3_GPIOC?_LED? changes to keep the gpios as
outputs and informs the led driver via struct asic3_led of the gpio to
toggle in the brightness_set and blink_set callbacks.
Does this work for you?
regards
Philipp
---
drivers/leds/leds-asic3.c | 19 +++++++++++++++++++
drivers/mfd/asic3.c | 3 +++
include/linux/mfd/asic3.h | 8 +++++---
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/leds-asic3.c b/drivers/leds/leds-asic3.c
index abda364..62b35dc 100644
--- a/drivers/leds/leds-asic3.c
+++ b/drivers/leds/leds-asic3.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/platform_device.h>
+#include <linux/gpio.h>
#include <linux/leds.h>
#include <linux/slab.h>
@@ -41,10 +42,14 @@ static void brightness_set(struct led_classdev *cdev,
{
struct platform_device *pdev = to_platform_device(cdev->dev->parent);
const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3_led *led = mfd_get_data(pdev);
struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
u32 timebase;
unsigned int base;
+ if (value != LED_OFF)
+ gpio_set_value(led->gpio, 1);
+
timebase = (value == LED_OFF) ? 0 : (LED_EN|0x4);
base = led_n_base[cell->id];
@@ -52,6 +57,9 @@ static void brightness_set(struct led_classdev *cdev,
asic3_write_register(asic, (base + ASIC3_LED_DutyTime), 32);
asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
asic3_write_register(asic, (base + ASIC3_LED_TimeBase), timebase);
+
+ if (value == LED_OFF)
+ gpio_set_value(led->gpio, 0);
}
static int blink_set(struct led_classdev *cdev,
@@ -60,6 +68,7 @@ static int blink_set(struct led_classdev *cdev,
{
struct platform_device *pdev = to_platform_device(cdev->dev->parent);
const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3_led *led = mfd_get_data(pdev);
struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
u32 on;
u32 off;
@@ -79,6 +88,8 @@ static int blink_set(struct led_classdev *cdev,
return -EINVAL;
}
+ gpio_set_value(led->gpio, 1);
+
base = led_n_base[cell->id];
asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), (on + off));
asic3_write_register(asic, (base + ASIC3_LED_DutyTime), on);
@@ -96,6 +107,10 @@ static int __devinit asic3_led_probe(struct platform_device *pdev)
struct asic3_led *led = mfd_get_data(pdev);
int ret;
+ ret = gpio_request(led->gpio, "leds-asic3");
+ if (ret < 0)
+ goto ret_gpio;
+
ret = mfd_cell_enable(pdev);
if (ret < 0)
goto ret0;
@@ -122,6 +137,8 @@ ret2:
ret1:
(void) mfd_cell_disable(pdev);
ret0:
+ gpio_free(led->gpio);
+ret_gpio:
return ret;
}
@@ -135,6 +152,8 @@ static int __devexit asic3_led_remove(struct platform_device *pdev)
(void) mfd_cell_disable(pdev);
+ gpio_free(led->gpio);
+
return 0;
}
diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
index 77c10b7..5e6758d 100644
--- a/drivers/mfd/asic3.c
+++ b/drivers/mfd/asic3.c
@@ -889,6 +889,9 @@ static int __init asic3_mfd_probe(struct platform_device *pdev,
}
if (pdata->leds) {
+ pdata->leds[0].gpio = asic->gpio.base + 32; /* ASIC3_GPIOC0_LED0 */
+ pdata->leds[1].gpio = asic->gpio.base + 33; /* ASIC3_GPIOC1_LED1 */
+ pdata->leds[2].gpio = asic->gpio.base + 34; /* ASIC3_GPIOC2_LED2 */
asic3_cell_leds[0].mfd_data = &pdata->leds[0];
asic3_cell_leds[1].mfd_data = &pdata->leds[1];
asic3_cell_leds[2].mfd_data = &pdata->leds[2];
diff --git a/include/linux/mfd/asic3.h b/include/linux/mfd/asic3.h
index d0dd3eb..f1944ef 100644
--- a/include/linux/mfd/asic3.h
+++ b/include/linux/mfd/asic3.h
@@ -21,6 +21,8 @@ struct asic3_led {
const char *name;
const char *default_trigger;
struct led_classdev *cdev;
+
+ int gpio; /* set by the asic3 mfd driver itself */
};
struct asic3_platform_data {
@@ -120,9 +122,9 @@ struct asic3_platform_data {
#define ASIC3_GPIOA11_PWM0 ASIC3_CONFIG_GPIO(11, 1, 1, 0)
#define ASIC3_GPIOA12_PWM1 ASIC3_CONFIG_GPIO(12, 1, 1, 0)
#define ASIC3_GPIOA15_CONTROL_CX ASIC3_CONFIG_GPIO(15, 1, 1, 0)
-#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 0, 0)
-#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 0, 0)
-#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 0, 0)
+#define ASIC3_GPIOC0_LED0 ASIC3_CONFIG_GPIO(32, 1, 1, 0)
+#define ASIC3_GPIOC1_LED1 ASIC3_CONFIG_GPIO(33, 1, 1, 0)
+#define ASIC3_GPIOC2_LED2 ASIC3_CONFIG_GPIO(34, 1, 1, 0)
#define ASIC3_GPIOC3_SPI_RXD ASIC3_CONFIG_GPIO(35, 1, 0, 0)
#define ASIC3_GPIOC4_CF_nCD ASIC3_CONFIG_GPIO(36, 1, 0, 0)
#define ASIC3_GPIOC4_SPI_TXD ASIC3_CONFIG_GPIO(36, 1, 1, 0)
--
1.7.4.4
Hi Philipp,
--- On Tue, 3/5/11, Philipp Zabel <[email protected]> wrote:
> This patch reverts your ASIC3_GPIOC?_LED? changes to keep
> the gpios as
> outputs and informs the led driver via struct asic3_led of
> the gpio to
> toggle in the brightness_set and blink_set callbacks.
>
> Does this work for you?
Well, it works in the sense that the LEDs still operate as before. In other words, calling gpio_set_value() has made no difference to the driver's behaviour.
So I'm confused as to what the patch is intended to demonstrate. Surely we want to use the LED GPIOs instead of the TimeBase enable bit, not in addition to it.
As a quick experiment, I modified the driver again to clear the TimeBase enable bit:
--- leds-asic3.c 2011-05-03 15:58:18.372196585 +0100
+++ drivers/leds/leds-asic3.c 2011-05-03 16:12:33.832902946 +0100
@@ -50,7 +50,7 @@ static void brightness_set(struct led_cl
if (value != LED_OFF)
gpio_set_value(led->gpio, 1);
- timebase = (value == LED_OFF) ? 0 : (LED_EN|0x4);
+ timebase = (value == LED_OFF) ? 0 : (0|0x4);
base = led_n_base[cell->id];
asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), 32);
@@ -94,7 +94,7 @@ static int blink_set(struct led_classdev
asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), (on + off));
asic3_write_register(asic, (base + ASIC3_LED_DutyTime), on);
asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
- asic3_write_register(asic, (base + ASIC3_LED_TimeBase), (LED_EN|0x4));
+ asic3_write_register(asic, (base + ASIC3_LED_TimeBase), (0|0x4));
*delay_on = CLK_TO_MS(on);
*delay_off = CLK_TO_MS(off);
This was to check whether the LED GPIOs performed the same function as the TimeBase enable bit, which would be a useful result. But they didn't; the LEDs stopped working.
So I must conclude that there is still no evidence that the LED GPIOs work as outputs. They do work as inputs however, per my previous observations.
Maybe there exists an ASIC3 control register which can change the direction of the LED GPIOs from inputs to outputs?
Regards,
Paul
Hi Philipp,
--- On Tue, 3/5/11, Philipp Zabel <[email protected]> wrote:
> I'd like to avoid this export. Could you inform the cell
> driver of
> mapping and bus shift via io resources as it is done in
> ds1wm?
> Same for this one.
The LED registers are already mapped by the mfd driver, and the functions to access them are already provided. My feeling is that replicating all of that seems a high price to pay to avoid exporting a couple of functions.
> Ok for now, I guess. But it's unrelated to led support. And
> once we have
> a common struct clk and can use clk_enable in the led
> driver, there's
> going to be error checking anyway...
Yes, that's a separate patch if at all.
> As Samuel pointed out, better use .platform_data and
> .platform_size here
> right away.
Will do.
> Separate patch?
Separate patch.
> As I said, not convinced about that one. I'll follow up
> with a patch to
> test on top of yours.
OK.
Regards,
Paul