| Message ID | 20190201094736.32057-10-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | mfd: add support for max77650 PMIC | expand |
Hi Bartosz, Thanks for the update. On 2/1/19 10:47 AM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > This adds basic support for LEDs for the max77650 PMIC. The device has > three current sinks for driving LEDs. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > drivers/leds/Kconfig | 6 ++ > drivers/leds/Makefile | 1 + > drivers/leds/leds-max77650.c | 147 +++++++++++++++++++++++++++++++++++ > 3 files changed, 154 insertions(+) > create mode 100644 drivers/leds/leds-max77650.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a72f97fca57b..6e7a8f51eccc 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -608,6 +608,12 @@ config LEDS_TLC591XX > This option enables support for Texas Instruments TLC59108 > and TLC59116 LED controllers. > > +config LEDS_MAX77650 > + tristate "LED support for Maxim MAX77650 PMIC" > + depends on MFD_MAX77650 > + help > + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." > + > config LEDS_MAX77693 > tristate "LED support for MAX77693 Flash" > depends on LEDS_CLASS_FLASH > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 4c1b0054f379..f48b2404dbb7 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o > obj-$(CONFIG_LEDS_NS2) += leds-ns2.o > obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o > obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o > +obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o > obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o > obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o > obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o > diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c > new file mode 100644 > index 000000000000..6b74ce9cac12 > --- /dev/null > +++ b/drivers/leds/leds-max77650.c > @@ -0,0 +1,147 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (C) 2018 BayLibre SAS > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> > +// > +// LED driver for MAXIM 77650/77651 charger/power-supply. > + > +#include <linux/i2c.h> > +#include <linux/leds.h> > +#include <linux/mfd/max77650.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#define MAX77650_LED_NUM_LEDS 3 > + > +#define MAX77650_LED_A_BASE 0x40 > +#define MAX77650_LED_B_BASE 0x43 > + > +#define MAX77650_LED_BR_MASK GENMASK(4, 0) > +#define MAX77650_LED_EN_MASK GENMASK(7, 6) > + > +#define MAX77650_LED_MAX_BRIGHTNESS MAX77650_LED_BR_MASK > + > +/* Enable EN_LED_MSTR. */ > +#define MAX77650_LED_TOP_DEFAULT BIT(0) > + > +#define MAX77650_LED_ENABLE GENMASK(7, 6) > +#define MAX77650_LED_DISABLE 0x00 > + > +#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE > +/* 100% on duty */ > +#define MAX77650_LED_B_DEFAULT GENMASK(3, 0) > + > +struct max77650_led { > + struct led_classdev cdev; > + struct regmap *map; > + unsigned int regA; > + unsigned int regB; > +}; > + > +static struct max77650_led *max77650_to_led(struct led_classdev *cdev) > +{ > + return container_of(cdev, struct max77650_led, cdev); > +} > + > +static int max77650_led_brightness_set(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct max77650_led *led = max77650_to_led(cdev); > + int val, mask; > + > + mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK; > + > + if (brightness == LED_OFF) > + val = MAX77650_LED_DISABLE; > + else > + val = MAX77650_LED_ENABLE | brightness; > + > + return regmap_update_bits(led->map, led->regA, mask, val); > +} > + > +static int max77650_led_probe(struct platform_device *pdev) > +{ > + struct device_node *of_node, *child; > + struct max77650_led *leds, *led; > + struct device *parent; > + struct device *dev; > + struct regmap *map; > + const char *label; > + int rv, num_leds; > + u32 reg; > + > + dev = &pdev->dev; > + parent = dev->parent; > + of_node = dev->of_node; > + > + if (!of_node) > + return -ENODEV; > + > + leds = devm_kcalloc(dev, sizeof(*leds), > + MAX77650_LED_NUM_LEDS, GFP_KERNEL); > + if (!leds) > + return -ENOMEM; > + > + map = dev_get_regmap(dev->parent, NULL); > + if (!map) > + return -ENODEV; > + > + num_leds = of_get_child_count(of_node); > + if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS) > + return -ENODEV; > + > + for_each_child_of_node(of_node, child) { > + rv = of_property_read_u32(child, "reg", ®); > + if (rv || reg >= MAX77650_LED_NUM_LEDS) > + return -EINVAL; > + > + led = &leds[reg]; > + led->map = map; > + led->regA = MAX77650_LED_A_BASE + reg; > + led->regB = MAX77650_LED_B_BASE + reg; > + led->cdev.brightness_set_blocking = max77650_led_brightness_set; > + led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS; > + > + label = of_get_property(child, "label", NULL); > + if (!label) { > + led->cdev.name = "max77650::"; > + } else { > + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, > + "max77650:%s", label); > + if (!led->cdev.name) > + return -ENOMEM; > + } > + > + of_property_read_string(child, "linux,default-trigger", > + &led->cdev.default_trigger); > + > + rv = devm_of_led_classdev_register(dev, child, &led->cdev); > + if (rv) > + return rv; > + > + rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT); > + if (rv) > + return rv; > + > + rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT); > + if (rv) > + return rv; > + } > + > + return regmap_write(map, > + MAX77650_REG_CNFG_LED_TOP, > + MAX77650_LED_TOP_DEFAULT); > +} > + > +static struct platform_driver max77650_led_driver = { > + .driver = { > + .name = "max77650-led", > + }, > + .probe = max77650_led_probe, > +}; > +module_platform_driver(max77650_led_driver); > + > +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver"); > +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>"); > +MODULE_LICENSE("GPL v2"); > Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Hi On 2/1/19 1:45 PM, Jacek Anaszewski wrote: > Hi Bartosz, > > Thanks for the update. > > On 2/1/19 10:47 AM, Bartosz Golaszewski wrote: >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> >> This adds basic support for LEDs for the max77650 PMIC. The device has >> three current sinks for driving LEDs. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> --- >> drivers/leds/Kconfig | 6 ++ >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-max77650.c | 147 +++++++++++++++++++++++++++++++++++ >> 3 files changed, 154 insertions(+) >> create mode 100644 drivers/leds/leds-max77650.c >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index a72f97fca57b..6e7a8f51eccc 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -608,6 +608,12 @@ config LEDS_TLC591XX >> This option enables support for Texas Instruments TLC59108 >> and TLC59116 LED controllers. >> +config LEDS_MAX77650 >> + tristate "LED support for Maxim MAX77650 PMIC" >> + depends on MFD_MAX77650 >> + help >> + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." >> + >> config LEDS_MAX77693 >> tristate "LED support for MAX77693 Flash" >> depends on LEDS_CLASS_FLASH >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile >> index 4c1b0054f379..f48b2404dbb7 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o >> obj-$(CONFIG_LEDS_NS2) += leds-ns2.o >> obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o >> obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o >> +obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o >> obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o >> obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o >> obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o >> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c >> new file mode 100644 >> index 000000000000..6b74ce9cac12 >> --- /dev/null >> +++ b/drivers/leds/leds-max77650.c >> @@ -0,0 +1,147 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// >> +// Copyright (C) 2018 BayLibre SAS >> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> +// >> +// LED driver for MAXIM 77650/77651 charger/power-supply. >> + >> +#include <linux/i2c.h> >> +#include <linux/leds.h> >> +#include <linux/mfd/max77650.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +#define MAX77650_LED_NUM_LEDS 3 >> + >> +#define MAX77650_LED_A_BASE 0x40 >> +#define MAX77650_LED_B_BASE 0x43 >> + >> +#define MAX77650_LED_BR_MASK GENMASK(4, 0) >> +#define MAX77650_LED_EN_MASK GENMASK(7, 6) >> + >> +#define MAX77650_LED_MAX_BRIGHTNESS MAX77650_LED_BR_MASK >> + >> +/* Enable EN_LED_MSTR. */ >> +#define MAX77650_LED_TOP_DEFAULT BIT(0) >> + >> +#define MAX77650_LED_ENABLE GENMASK(7, 6) >> +#define MAX77650_LED_DISABLE 0x00 >> + >> +#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE >> +/* 100% on duty */ >> +#define MAX77650_LED_B_DEFAULT GENMASK(3, 0) >> + >> +struct max77650_led { >> + struct led_classdev cdev; >> + struct regmap *map; >> + unsigned int regA; >> + unsigned int regB; Please don't use camel case. >> +}; >> + >> +static struct max77650_led *max77650_to_led(struct led_classdev *cdev) >> +{ >> + return container_of(cdev, struct max77650_led, cdev); >> +} >> + >> +static int max77650_led_brightness_set(struct led_classdev *cdev, >> + enum led_brightness brightness) >> +{ >> + struct max77650_led *led = max77650_to_led(cdev); >> + int val, mask; >> + The register value and bits are only 8 bit why an int? >> + mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK; >> + >> + if (brightness == LED_OFF) >> + val = MAX77650_LED_DISABLE; >> + else >> + val = MAX77650_LED_ENABLE | brightness; >> + >> + return regmap_update_bits(led->map, led->regA, mask, val); >> +} >> + >> +static int max77650_led_probe(struct platform_device *pdev) >> +{ >> + struct device_node *of_node, *child; >> + struct max77650_led *leds, *led; >> + struct device *parent; >> + struct device *dev; >> + struct regmap *map; >> + const char *label; >> + int rv, num_leds; >> + u32 reg; >> + >> + dev = &pdev->dev; >> + parent = dev->parent; >> + of_node = dev->of_node; >> + >> + if (!of_node) >> + return -ENODEV; >> + >> + leds = devm_kcalloc(dev, sizeof(*leds), >> + MAX77650_LED_NUM_LEDS, GFP_KERNEL); >> + if (!leds) >> + return -ENOMEM; >> + >> + map = dev_get_regmap(dev->parent, NULL); >> + if (!map) >> + return -ENODEV; >> + >> + num_leds = of_get_child_count(of_node); >> + if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS) >> + return -ENODEV; >> + >> + for_each_child_of_node(of_node, child) { >> + rv = of_property_read_u32(child, "reg", ®); Can we use the fwnode_property_read_u32 call here? And the same for all below as well? >> + if (rv || reg >= MAX77650_LED_NUM_LEDS) >> + return -EINVAL; >> + >> + led = &leds[reg]; >> + led->map = map; >> + led->regA = MAX77650_LED_A_BASE + reg; >> + led->regB = MAX77650_LED_B_BASE + reg; >> + led->cdev.brightness_set_blocking = max77650_led_brightness_set; >> + led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS; >> + >> + label = of_get_property(child, "label", NULL); >> + if (!label) { >> + led->cdev.name = "max77650::"; >> + } else { >> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, >> + "max77650:%s", label); >> + if (!led->cdev.name) >> + return -ENOMEM; >> + } >> + >> + of_property_read_string(child, "linux,default-trigger", >> + &led->cdev.default_trigger); >> + >> + rv = devm_of_led_classdev_register(dev, child, &led->cdev); >> + if (rv) >> + return rv; >> + >> + rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT); >> + if (rv) >> + return rv; >> + >> + rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT); >> + if (rv) >> + return rv; >> + } >> + >> + return regmap_write(map, >> + MAX77650_REG_CNFG_LED_TOP, >> + MAX77650_LED_TOP_DEFAULT); >> +} >> + >> +static struct platform_driver max77650_led_driver = { >> + .driver = { >> + .name = "max77650-led", >> + }, >> + .probe = max77650_led_probe, >> +}; >> +module_platform_driver(max77650_led_driver); >> + >> +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver"); >> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>"); >> +MODULE_LICENSE("GPL v2"); >> > > Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> >
pt., 1 lut 2019 o 21:15 Dan Murphy <dmurphy@ti.com> napisał(a): > > Hi > > On 2/1/19 1:45 PM, Jacek Anaszewski wrote: > > Hi Bartosz, > > > > Thanks for the update. > > > > On 2/1/19 10:47 AM, Bartosz Golaszewski wrote: > >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >> > >> This adds basic support for LEDs for the max77650 PMIC. The device has > >> three current sinks for driving LEDs. > >> > >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >> --- > >> drivers/leds/Kconfig | 6 ++ > >> drivers/leds/Makefile | 1 + > >> drivers/leds/leds-max77650.c | 147 +++++++++++++++++++++++++++++++++++ > >> 3 files changed, 154 insertions(+) > >> create mode 100644 drivers/leds/leds-max77650.c > >> > >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > >> index a72f97fca57b..6e7a8f51eccc 100644 > >> --- a/drivers/leds/Kconfig > >> +++ b/drivers/leds/Kconfig > >> @@ -608,6 +608,12 @@ config LEDS_TLC591XX > >> This option enables support for Texas Instruments TLC59108 > >> and TLC59116 LED controllers. > >> +config LEDS_MAX77650 > >> + tristate "LED support for Maxim MAX77650 PMIC" > >> + depends on MFD_MAX77650 > >> + help > >> + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." > >> + > >> config LEDS_MAX77693 > >> tristate "LED support for MAX77693 Flash" > >> depends on LEDS_CLASS_FLASH > >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > >> index 4c1b0054f379..f48b2404dbb7 100644 > >> --- a/drivers/leds/Makefile > >> +++ b/drivers/leds/Makefile > >> @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o > >> obj-$(CONFIG_LEDS_NS2) += leds-ns2.o > >> obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o > >> obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o > >> +obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o > >> obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o > >> obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o > >> obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o > >> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c > >> new file mode 100644 > >> index 000000000000..6b74ce9cac12 > >> --- /dev/null > >> +++ b/drivers/leds/leds-max77650.c > >> @@ -0,0 +1,147 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +// > >> +// Copyright (C) 2018 BayLibre SAS > >> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >> +// > >> +// LED driver for MAXIM 77650/77651 charger/power-supply. > >> + > >> +#include <linux/i2c.h> > >> +#include <linux/leds.h> > >> +#include <linux/mfd/max77650.h> > >> +#include <linux/module.h> > >> +#include <linux/platform_device.h> > >> +#include <linux/regmap.h> > >> + > >> +#define MAX77650_LED_NUM_LEDS 3 > >> + > >> +#define MAX77650_LED_A_BASE 0x40 > >> +#define MAX77650_LED_B_BASE 0x43 > >> + > >> +#define MAX77650_LED_BR_MASK GENMASK(4, 0) > >> +#define MAX77650_LED_EN_MASK GENMASK(7, 6) > >> + > >> +#define MAX77650_LED_MAX_BRIGHTNESS MAX77650_LED_BR_MASK > >> + > >> +/* Enable EN_LED_MSTR. */ > >> +#define MAX77650_LED_TOP_DEFAULT BIT(0) > >> + > >> +#define MAX77650_LED_ENABLE GENMASK(7, 6) > >> +#define MAX77650_LED_DISABLE 0x00 > >> + > >> +#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE > >> +/* 100% on duty */ > >> +#define MAX77650_LED_B_DEFAULT GENMASK(3, 0) > >> + > >> +struct max77650_led { > >> + struct led_classdev cdev; > >> + struct regmap *map; > >> + unsigned int regA; > >> + unsigned int regB; > > Please don't use camel case. > Sorry, but this is pointless nitpicking. The registers are referred to in the manual as LED[012]_A and LED[012]_B so these variable names reflect that. The difference between ThisKindOfCamelCase and regA/regB is obvious. It's much more readable than for example rega or reg_a. I also used the same approach in the regulator module and there were no complaints. > >> +}; > >> + > >> +static struct max77650_led *max77650_to_led(struct led_classdev *cdev) > >> +{ > >> + return container_of(cdev, struct max77650_led, cdev); > >> +} > >> + > >> +static int max77650_led_brightness_set(struct led_classdev *cdev, > >> + enum led_brightness brightness) > >> +{ > >> + struct max77650_led *led = max77650_to_led(cdev); > >> + int val, mask; > >> + > > The register value and bits are only 8 bit why an int? > Because regmap_update_bits() deals with 32-bit integers. There's no hurt and it's less confusing. > >> + mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK; > >> + > >> + if (brightness == LED_OFF) > >> + val = MAX77650_LED_DISABLE; > >> + else > >> + val = MAX77650_LED_ENABLE | brightness; > >> + > >> + return regmap_update_bits(led->map, led->regA, mask, val); > >> +} > >> + > >> +static int max77650_led_probe(struct platform_device *pdev) > >> +{ > >> + struct device_node *of_node, *child; > >> + struct max77650_led *leds, *led; > >> + struct device *parent; > >> + struct device *dev; > >> + struct regmap *map; > >> + const char *label; > >> + int rv, num_leds; > >> + u32 reg; > >> + > >> + dev = &pdev->dev; > >> + parent = dev->parent; > >> + of_node = dev->of_node; > >> + > >> + if (!of_node) > >> + return -ENODEV; > >> + > >> + leds = devm_kcalloc(dev, sizeof(*leds), > >> + MAX77650_LED_NUM_LEDS, GFP_KERNEL); > >> + if (!leds) > >> + return -ENOMEM; > >> + > >> + map = dev_get_regmap(dev->parent, NULL); > >> + if (!map) > >> + return -ENODEV; > >> + > >> + num_leds = of_get_child_count(of_node); > >> + if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS) > >> + return -ENODEV; > >> + > >> + for_each_child_of_node(of_node, child) { > >> + rv = of_property_read_u32(child, "reg", ®); > > Can we use the fwnode_property_read_u32 call here? > And the same for all below as well? > > Of course we can. But why? This is a low-power PMIC. Is there really a chance that a non-DT system will want to use it? If so - we will be able to convert it if needed. For now: I'd stick with of_* functions. > >> + if (rv || reg >= MAX77650_LED_NUM_LEDS) > >> + return -EINVAL; > >> + > >> + led = &leds[reg]; > >> + led->map = map; > >> + led->regA = MAX77650_LED_A_BASE + reg; > >> + led->regB = MAX77650_LED_B_BASE + reg; > >> + led->cdev.brightness_set_blocking = max77650_led_brightness_set; > >> + led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS; > >> + > >> + label = of_get_property(child, "label", NULL); > >> + if (!label) { > >> + led->cdev.name = "max77650::"; > >> + } else { > >> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, > >> + "max77650:%s", label); > >> + if (!led->cdev.name) > >> + return -ENOMEM; > >> + } > >> + > >> + of_property_read_string(child, "linux,default-trigger", > >> + &led->cdev.default_trigger); > >> + > >> + rv = devm_of_led_classdev_register(dev, child, &led->cdev); > >> + if (rv) > >> + return rv; > >> + > >> + rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT); > >> + if (rv) > >> + return rv; > >> + > >> + rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT); > >> + if (rv) > >> + return rv; > >> + } > >> + > >> + return regmap_write(map, > >> + MAX77650_REG_CNFG_LED_TOP, > >> + MAX77650_LED_TOP_DEFAULT); > >> +} > >> + > >> +static struct platform_driver max77650_led_driver = { > >> + .driver = { > >> + .name = "max77650-led", > >> + }, > >> + .probe = max77650_led_probe, > >> +}; > >> +module_platform_driver(max77650_led_driver); > >> + > >> +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver"); > >> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>"); > >> +MODULE_LICENSE("GPL v2"); > >> > > > > Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> > > > > > -- > ------------------ > Dan Murphy Best regards, Bartosz Golaszewski
Hi! > >> +static struct max77650_led *max77650_to_led(struct led_classdev *cdev) > >> +{ > >> + return container_of(cdev, struct max77650_led, cdev); > >> +} > >> + > >> +static int max77650_led_brightness_set(struct led_classdev *cdev, > >> + enum led_brightness brightness) > >> +{ > >> + struct max77650_led *led = max77650_to_led(cdev); > >> + int val, mask; > >> + > > The register value and bits are only 8 bit why an int? int is word size CPU prefers. It is ok here. If you force u8, it may actually result in slower/bigger code. Pavel
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a72f97fca57b..6e7a8f51eccc 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -608,6 +608,12 @@ config LEDS_TLC591XX > This option enables support for Texas Instruments TLC59108 > and TLC59116 LED controllers. > > +config LEDS_MAX77650 > + tristate "LED support for Maxim MAX77650 PMIC" > + depends on MFD_MAX77650 > + help > + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." > + List the chips? Remove extra "? Pavel
wt., 12 lut 2019 o 13:12 Pavel Machek <pavel@ucw.cz> napisał(a): > > > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > > index a72f97fca57b..6e7a8f51eccc 100644 > > --- a/drivers/leds/Kconfig > > +++ b/drivers/leds/Kconfig > > @@ -608,6 +608,12 @@ config LEDS_TLC591XX > > This option enables support for Texas Instruments TLC59108 > > and TLC59116 LED controllers. > > > > +config LEDS_MAX77650 > > + tristate "LED support for Maxim MAX77650 PMIC" > > + depends on MFD_MAX77650 > > + help > > + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." > > + > > List the chips? Remove extra "? > Pavel They are already listed in the core mfd Kconfig, but thanks for spotting that stray ". Bart
Hi Bartosz,
I love your patch! Yet something to improve:
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/mfd-add-support-for-max77650-PMIC/20190203-031133
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-s5-02140238 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
ld: drivers/leds/leds-max77650.o: in function `max77650_led_probe':
>> drivers/leds/leds-max77650.c:119: undefined reference to `devm_of_led_classdev_register'
vim +119 drivers/leds/leds-max77650.c
62
63 static int max77650_led_probe(struct platform_device *pdev)
64 {
65 struct device_node *of_node, *child;
66 struct max77650_led *leds, *led;
67 struct device *parent;
68 struct device *dev;
69 struct regmap *map;
70 const char *label;
71 int rv, num_leds;
72 u32 reg;
73
74 dev = &pdev->dev;
75 parent = dev->parent;
76 of_node = dev->of_node;
77
78 if (!of_node)
79 return -ENODEV;
80
81 leds = devm_kcalloc(dev, sizeof(*leds),
82 MAX77650_LED_NUM_LEDS, GFP_KERNEL);
83 if (!leds)
84 return -ENOMEM;
85
86 map = dev_get_regmap(dev->parent, NULL);
87 if (!map)
88 return -ENODEV;
89
90 num_leds = of_get_child_count(of_node);
91 if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS)
92 return -ENODEV;
93
94 for_each_child_of_node(of_node, child) {
95 rv = of_property_read_u32(child, "reg", ®);
96 if (rv || reg >= MAX77650_LED_NUM_LEDS)
97 return -EINVAL;
98
99 led = &leds[reg];
100 led->map = map;
101 led->regA = MAX77650_LED_A_BASE + reg;
102 led->regB = MAX77650_LED_B_BASE + reg;
103 led->cdev.brightness_set_blocking = max77650_led_brightness_set;
104 led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS;
105
106 label = of_get_property(child, "label", NULL);
107 if (!label) {
108 led->cdev.name = "max77650::";
109 } else {
110 led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
111 "max77650:%s", label);
112 if (!led->cdev.name)
113 return -ENOMEM;
114 }
115
116 of_property_read_string(child, "linux,default-trigger",
117 &led->cdev.default_trigger);
118
> 119 rv = devm_of_led_classdev_register(dev, child, &led->cdev);
120 if (rv)
121 return rv;
122
123 rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
124 if (rv)
125 return rv;
126
127 rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
128 if (rv)
129 return rv;
130 }
131
132 return regmap_write(map,
133 MAX77650_REG_CNFG_LED_TOP,
134 MAX77650_LED_TOP_DEFAULT);
135 }
136
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a72f97fca57b..6e7a8f51eccc 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -608,6 +608,12 @@ config LEDS_TLC591XX This option enables support for Texas Instruments TLC59108 and TLC59116 LED controllers. +config LEDS_MAX77650 + tristate "LED support for Maxim MAX77650 PMIC" + depends on MFD_MAX77650 + help + LEDs driver for MAX77650 family of PMICs from Maxim Integrated." + config LEDS_MAX77693 tristate "LED support for MAX77693 Flash" depends on LEDS_CLASS_FLASH diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 4c1b0054f379..f48b2404dbb7 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o obj-$(CONFIG_LEDS_NS2) += leds-ns2.o obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o +obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c new file mode 100644 index 000000000000..6b74ce9cac12 --- /dev/null +++ b/drivers/leds/leds-max77650.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright (C) 2018 BayLibre SAS +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> +// +// LED driver for MAXIM 77650/77651 charger/power-supply. + +#include <linux/i2c.h> +#include <linux/leds.h> +#include <linux/mfd/max77650.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#define MAX77650_LED_NUM_LEDS 3 + +#define MAX77650_LED_A_BASE 0x40 +#define MAX77650_LED_B_BASE 0x43 + +#define MAX77650_LED_BR_MASK GENMASK(4, 0) +#define MAX77650_LED_EN_MASK GENMASK(7, 6) + +#define MAX77650_LED_MAX_BRIGHTNESS MAX77650_LED_BR_MASK + +/* Enable EN_LED_MSTR. */ +#define MAX77650_LED_TOP_DEFAULT BIT(0) + +#define MAX77650_LED_ENABLE GENMASK(7, 6) +#define MAX77650_LED_DISABLE 0x00 + +#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE +/* 100% on duty */ +#define MAX77650_LED_B_DEFAULT GENMASK(3, 0) + +struct max77650_led { + struct led_classdev cdev; + struct regmap *map; + unsigned int regA; + unsigned int regB; +}; + +static struct max77650_led *max77650_to_led(struct led_classdev *cdev) +{ + return container_of(cdev, struct max77650_led, cdev); +} + +static int max77650_led_brightness_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct max77650_led *led = max77650_to_led(cdev); + int val, mask; + + mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK; + + if (brightness == LED_OFF) + val = MAX77650_LED_DISABLE; + else + val = MAX77650_LED_ENABLE | brightness; + + return regmap_update_bits(led->map, led->regA, mask, val); +} + +static int max77650_led_probe(struct platform_device *pdev) +{ + struct device_node *of_node, *child; + struct max77650_led *leds, *led; + struct device *parent; + struct device *dev; + struct regmap *map; + const char *label; + int rv, num_leds; + u32 reg; + + dev = &pdev->dev; + parent = dev->parent; + of_node = dev->of_node; + + if (!of_node) + return -ENODEV; + + leds = devm_kcalloc(dev, sizeof(*leds), + MAX77650_LED_NUM_LEDS, GFP_KERNEL); + if (!leds) + return -ENOMEM; + + map = dev_get_regmap(dev->parent, NULL); + if (!map) + return -ENODEV; + + num_leds = of_get_child_count(of_node); + if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS) + return -ENODEV; + + for_each_child_of_node(of_node, child) { + rv = of_property_read_u32(child, "reg", ®); + if (rv || reg >= MAX77650_LED_NUM_LEDS) + return -EINVAL; + + led = &leds[reg]; + led->map = map; + led->regA = MAX77650_LED_A_BASE + reg; + led->regB = MAX77650_LED_B_BASE + reg; + led->cdev.brightness_set_blocking = max77650_led_brightness_set; + led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS; + + label = of_get_property(child, "label", NULL); + if (!label) { + led->cdev.name = "max77650::"; + } else { + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, + "max77650:%s", label); + if (!led->cdev.name) + return -ENOMEM; + } + + of_property_read_string(child, "linux,default-trigger", + &led->cdev.default_trigger); + + rv = devm_of_led_classdev_register(dev, child, &led->cdev); + if (rv) + return rv; + + rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT); + if (rv) + return rv; + + rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT); + if (rv) + return rv; + } + + return regmap_write(map, + MAX77650_REG_CNFG_LED_TOP, + MAX77650_LED_TOP_DEFAULT); +} + +static struct platform_driver max77650_led_driver = { + .driver = { + .name = "max77650-led", + }, + .probe = max77650_led_probe, +}; +module_platform_driver(max77650_led_driver); + +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver"); +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>"); +MODULE_LICENSE("GPL v2");