| Message ID | 1470679911-13669-2-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon 08 Aug 11:11 PDT 2016, Linus Walleij wrote: > This adds a driver for the six PM8058 LEDs, three ordinary LEDs, > two "flash" LEDs and one "keypad" LED. > > The "keypad" and "flash" LEDs are not really hard-wired to these > usecases: for example on the APQ8060 Dragonboard, the "keypad" > LED is instead used to drive an IR LED used for the proximity > sensor. The "flash" LEDs are just ordinary high-current LED > drivers. > > Cc: linux-arm-msm@vger.kernel.org > Cc: Andy Gross <andy.gross@linaro.org> > Cc: Stephen Boyd <sboyd@codeaurora.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- [..] > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c [..] > +#include <linux/init.h> I don't see any uses of this. > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pm.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> Dito > +#include <linux/spinlock.h> Dito > + > +#define PM8058_LED_TYPE_COMMON 0x00 > +#define PM8058_LED_TYPE_KEYPAD 0x01 > +#define PM8058_LED_TYPE_FLASH 0x02 > + > +#define PM8058_LED_TYPE_COMMON_MASK 0xf8 > +#define PM8058_LED_TYPE_KEYPAD_MASK 0xf0 > +#define PM8058_LED_TYPE_COMMON_SHIFT 3 > +#define PM8058_LED_TYPE_KEYPAD_SHIFT 4 > + > +struct pm8058_led { > + struct regmap *map; > + unsigned int reg; reg should be u32, as you pass it to of_property_read_u32() > + u32 ledtype; > + struct led_classdev cdev; > +}; > + > +static void pm8058_led_set(struct led_classdev *cled, > + enum led_brightness value) > +{ > + struct pm8058_led *led; > + int ret = 0; > + > + led = container_of(cled, struct pm8058_led, cdev); > + switch (led->ledtype) { > + case PM8058_LED_TYPE_COMMON: > + ret = regmap_update_bits(led->map, led->reg, > + PM8058_LED_TYPE_COMMON_MASK, > + value << PM8058_LED_TYPE_COMMON_SHIFT); > + break; > + case PM8058_LED_TYPE_KEYPAD: > + case PM8058_LED_TYPE_FLASH: > + ret = regmap_update_bits(led->map, led->reg, > + PM8058_LED_TYPE_KEYPAD_MASK, > + value << PM8058_LED_TYPE_KEYPAD_SHIFT); > + break; > + default: "default" shouldn't ever happen, so I think you should have the switch massage value and pick a mask and then move the regmap_update_bits() after the switch. > + break; > + } > + if (ret) > + pr_err("Failed to set LED brightness\n"); > +} > + > +static enum led_brightness pm8058_led_get(struct led_classdev *cled) > +{ > + struct pm8058_led *led; > + int ret; > + u32 val; regmap_read takes an unsigned int. > + > + led = container_of(cled, struct pm8058_led, cdev); > + switch (led->ledtype) { > + case PM8058_LED_TYPE_COMMON: > + ret = regmap_read(led->map, led->reg, &val); > + if (ret) { > + pr_err("Failed to get LED brightness\n"); > + return LED_OFF; > + } > + return ((val & PM8058_LED_TYPE_COMMON_MASK) >> > + PM8058_LED_TYPE_COMMON_SHIFT); > + case PM8058_LED_TYPE_KEYPAD: > + case PM8058_LED_TYPE_FLASH: > + ret = regmap_read(led->map, led->reg, &val); > + if (ret) { > + pr_err("Failed to get LED brightness\n"); > + return LED_OFF; > + } > + return ((val & PM8058_LED_TYPE_KEYPAD_MASK) >> > + PM8058_LED_TYPE_KEYPAD_SHIFT); > + default: "default" should never happen. > + break; > + } I think you should move the read above the switch statement, then have the cases massage the value and have a common return here. regmap_read(&val) if (error) ... switch (type) case ...: val &= MASK; val >>= SHIFT; break; case ...; ... return val; Would look cleaner. > + > + return LED_OFF; > +} > + > +static const struct of_device_id pm8058_leds_id_table[] = { > + { > + .compatible = "qcom,pm8058-led", > + .data = (void *)PM8058_LED_TYPE_COMMON > + }, > + { > + .compatible = "qcom,pm8058-keypad-led", > + .data = (void *)PM8058_LED_TYPE_KEYPAD > + }, > + { > + .compatible = "qcom,pm8058-flash-led", > + .data = (void *)PM8058_LED_TYPE_FLASH > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, pm8058_leds_id_table); > + > +static int pm8058_led_probe(struct platform_device *pdev) > +{ > + struct pm8058_led *led; > + struct device_node *np = pdev->dev.of_node; > + int ret; > + struct regmap *map; > + const struct of_device_id *match; > + const char *state; > + > + led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); > + if (led == NULL) if (!led) > + return -ENOMEM; > + > + match = of_match_node(pm8058_leds_id_table, np); > + if (!match) > + return -ENXIO; > + led->ledtype = (u32)match->data; Use of_device_get_match_data() instead, then you can keep the pm8058_leds_id_table at the bottom of the file. > + > + map = dev_get_regmap(pdev->dev.parent, NULL); > + if (!map) { > + dev_err(&pdev->dev, "Parent regmap unavailable.\n"); > + return -ENXIO; > + } > + led->map = map; > + > + ret = of_property_read_u32(np, "reg", &led->reg); > + if (ret) { > + dev_err(&pdev->dev, "no register offset specified\n"); > + return -EINVAL; > + } > + > + /* Use label else node name */ > + led->cdev.name = of_get_property(np, "label", NULL) ? : np->name; > + led->cdev.default_trigger = > + of_get_property(np, "linux,default-trigger", NULL); > + led->cdev.brightness_set = pm8058_led_set; > + led->cdev.brightness_get = pm8058_led_get; > + led->cdev.max_brightness = 15; For the "common" LED you have 5 bits of precision. > + > + state = of_get_property(np, "default-state", NULL); > + if (state) { > + if (!strcmp(state, "keep")) { > + led->cdev.brightness = pm8058_led_get(&led->cdev); > + } else if (!strcmp(state, "on")) { > + led->cdev.brightness = LED_FULL; > + pm8058_led_set(&led->cdev, LED_FULL); It's kinda ugly to let regmap_update_bits() shave off the upper 3 or 4 bits of LED_FULL. Perhaps pass max_brightness here instead? There's no need to limit the paramter inside pm8058_led_set(), as subsequent calls to set_brightness() will pass through a min(value, max). > + } else { > + led->cdev.brightness = LED_OFF; > + pm8058_led_set(&led->cdev, LED_OFF); > + } > + } > + led->cdev.flags = LED_CORE_SUSPENDRESUME; I can see this being a feature for the keyboard and flash, but for the "common" LEDs I don't think this is the right thing to do. > + > + ret = devm_led_classdev_register(&pdev->dev, &led->cdev); > + if (ret) { > + dev_err(&pdev->dev, "unable to register led \"%s\"\n", > + led->cdev.name); > + return ret; > + } > + platform_set_drvdata(pdev, led); You don't retrieve the drvdata, so no need to set it. > + > + return 0; > +} > + > +static struct platform_driver pm8058_led_driver = { > + .probe = pm8058_led_probe, > + .driver = { > + .name = "pm8058-leds", > + .of_match_table = pm8058_leds_id_table, > + }, > +}; > +module_platform_driver(pm8058_led_driver); > + > +MODULE_DESCRIPTION("PM8058 LEDs driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:pm8058-leds"); Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 9dcc9b13d495..f499db5a8552 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -631,6 +631,14 @@ config LEDS_VERSATILE This option enabled support for the LEDs on the ARM Versatile and RealView boards. Say Y to enabled these. +config LEDS_PM8058 + tristate "LED Support for the Qualcomm PM8058 PMIC" + depends on MFD_PM8921_CORE + depends on LEDS_CLASS + help + Choose this option if you want to use the LED drivers in + the Qualcomm PM8058 PMIC. + comment "LED Triggers" source "drivers/leds/trigger/Kconfig" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 0684c865a1c0..220860c970ca 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o +obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o # LED SPI Drivers obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c new file mode 100644 index 000000000000..70a6a48496cc --- /dev/null +++ b/drivers/leds/leds-pm8058.c @@ -0,0 +1,188 @@ +/* Copyright (c) 2010, 2011, 2016 The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * 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. + */ +#include <linux/init.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +#define PM8058_LED_TYPE_COMMON 0x00 +#define PM8058_LED_TYPE_KEYPAD 0x01 +#define PM8058_LED_TYPE_FLASH 0x02 + +#define PM8058_LED_TYPE_COMMON_MASK 0xf8 +#define PM8058_LED_TYPE_KEYPAD_MASK 0xf0 +#define PM8058_LED_TYPE_COMMON_SHIFT 3 +#define PM8058_LED_TYPE_KEYPAD_SHIFT 4 + +struct pm8058_led { + struct regmap *map; + unsigned int reg; + u32 ledtype; + struct led_classdev cdev; +}; + +static void pm8058_led_set(struct led_classdev *cled, + enum led_brightness value) +{ + struct pm8058_led *led; + int ret = 0; + + led = container_of(cled, struct pm8058_led, cdev); + switch (led->ledtype) { + case PM8058_LED_TYPE_COMMON: + ret = regmap_update_bits(led->map, led->reg, + PM8058_LED_TYPE_COMMON_MASK, + value << PM8058_LED_TYPE_COMMON_SHIFT); + break; + case PM8058_LED_TYPE_KEYPAD: + case PM8058_LED_TYPE_FLASH: + ret = regmap_update_bits(led->map, led->reg, + PM8058_LED_TYPE_KEYPAD_MASK, + value << PM8058_LED_TYPE_KEYPAD_SHIFT); + break; + default: + break; + } + if (ret) + pr_err("Failed to set LED brightness\n"); +} + +static enum led_brightness pm8058_led_get(struct led_classdev *cled) +{ + struct pm8058_led *led; + int ret; + u32 val; + + led = container_of(cled, struct pm8058_led, cdev); + switch (led->ledtype) { + case PM8058_LED_TYPE_COMMON: + ret = regmap_read(led->map, led->reg, &val); + if (ret) { + pr_err("Failed to get LED brightness\n"); + return LED_OFF; + } + return ((val & PM8058_LED_TYPE_COMMON_MASK) >> + PM8058_LED_TYPE_COMMON_SHIFT); + case PM8058_LED_TYPE_KEYPAD: + case PM8058_LED_TYPE_FLASH: + ret = regmap_read(led->map, led->reg, &val); + if (ret) { + pr_err("Failed to get LED brightness\n"); + return LED_OFF; + } + return ((val & PM8058_LED_TYPE_KEYPAD_MASK) >> + PM8058_LED_TYPE_KEYPAD_SHIFT); + default: + break; + } + + return LED_OFF; +} + +static const struct of_device_id pm8058_leds_id_table[] = { + { + .compatible = "qcom,pm8058-led", + .data = (void *)PM8058_LED_TYPE_COMMON + }, + { + .compatible = "qcom,pm8058-keypad-led", + .data = (void *)PM8058_LED_TYPE_KEYPAD + }, + { + .compatible = "qcom,pm8058-flash-led", + .data = (void *)PM8058_LED_TYPE_FLASH + }, + { }, +}; +MODULE_DEVICE_TABLE(of, pm8058_leds_id_table); + +static int pm8058_led_probe(struct platform_device *pdev) +{ + struct pm8058_led *led; + struct device_node *np = pdev->dev.of_node; + int ret; + struct regmap *map; + const struct of_device_id *match; + const char *state; + + led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); + if (led == NULL) + return -ENOMEM; + + match = of_match_node(pm8058_leds_id_table, np); + if (!match) + return -ENXIO; + led->ledtype = (u32)match->data; + + map = dev_get_regmap(pdev->dev.parent, NULL); + if (!map) { + dev_err(&pdev->dev, "Parent regmap unavailable.\n"); + return -ENXIO; + } + led->map = map; + + ret = of_property_read_u32(np, "reg", &led->reg); + if (ret) { + dev_err(&pdev->dev, "no register offset specified\n"); + return -EINVAL; + } + + /* Use label else node name */ + led->cdev.name = of_get_property(np, "label", NULL) ? : np->name; + led->cdev.default_trigger = + of_get_property(np, "linux,default-trigger", NULL); + led->cdev.brightness_set = pm8058_led_set; + led->cdev.brightness_get = pm8058_led_get; + led->cdev.max_brightness = 15; + + state = of_get_property(np, "default-state", NULL); + if (state) { + if (!strcmp(state, "keep")) { + led->cdev.brightness = pm8058_led_get(&led->cdev); + } else if (!strcmp(state, "on")) { + led->cdev.brightness = LED_FULL; + pm8058_led_set(&led->cdev, LED_FULL); + } else { + led->cdev.brightness = LED_OFF; + pm8058_led_set(&led->cdev, LED_OFF); + } + } + led->cdev.flags = LED_CORE_SUSPENDRESUME; + + ret = devm_led_classdev_register(&pdev->dev, &led->cdev); + if (ret) { + dev_err(&pdev->dev, "unable to register led \"%s\"\n", + led->cdev.name); + return ret; + } + platform_set_drvdata(pdev, led); + + return 0; +} + +static struct platform_driver pm8058_led_driver = { + .probe = pm8058_led_probe, + .driver = { + .name = "pm8058-leds", + .of_match_table = pm8058_leds_id_table, + }, +}; +module_platform_driver(pm8058_led_driver); + +MODULE_DESCRIPTION("PM8058 LEDs driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:pm8058-leds");
This adds a driver for the six PM8058 LEDs, three ordinary LEDs, two "flash" LEDs and one "keypad" LED. The "keypad" and "flash" LEDs are not really hard-wired to these usecases: for example on the APQ8060 Dragonboard, the "keypad" LED is instead used to drive an IR LED used for the proximity sensor. The "flash" LEDs are just ordinary high-current LED drivers. Cc: linux-arm-msm@vger.kernel.org Cc: Andy Gross <andy.gross@linaro.org> Cc: Stephen Boyd <sboyd@codeaurora.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v2->v3: - No changes, just resending. ChangeLog v1->v2: - Arrange #includes in alphabetical order - Use #define'd mask and shift and abstain from magic values - Drop surplus break; statements in switch clause - Put in the missed terminator {} in the OF match table - Use the devm_led_classdev_register() call to get managed resources - As a result drop the whole redundant .remove() clause --- drivers/leds/Kconfig | 8 ++ drivers/leds/Makefile | 1 + drivers/leds/leds-pm8058.c | 188 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+) create mode 100644 drivers/leds/leds-pm8058.c -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html