* [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block
@ 2014-12-09 0:22 Bjorn Andersson
2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Bjorn Andersson @ 2014-12-09 0:22 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Bryan Wu, Richard Purdie, Grant Likely
Cc: Courtney Cavin, devicetree, linux-kernel, linux-leds,
linux-arm-msm
From: Courtney Cavin <courtney.cavin@sonymobile.com>
This adds device tree binding documentation for the WLED ('White' LED)
block on Qualcomm's PM8941 PMICs.
Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
.../devicetree/bindings/leds/leds-pm8941-wled.txt | 43 ++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
new file mode 100644
index 0000000..a85a964
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
@@ -0,0 +1,43 @@
+Binding for Qualcomm PM8941 WLED driver
+
+Required properties:
+- compatible: should be "qcom,pm8941-wled"
+- reg: slave address
+
+Optional properties:
+- label: The label for this led
+ See Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger: Default trigger assigned to the LED
+ See Documentation/devicetree/bindings/leds/common.txt
+- qcom,cs-out: bool; enable current sink output
+- qcom,cabc: bool; enable content adaptive backlight control
+- qcom,ext-gen: bool; use externally generated modulator signal to dim
+- qcom,current-limit: mA; per-string current limit; value from 0 to 25
+ default: 20mA
+- qcom,current-boost-limit: mA; boost current limit; one of:
+ 105, 385, 525, 805, 980, 1260, 1400, 1680
+ default: 805mA
+- qcom,switching-freq: kHz; switching frequency; one of:
+ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371,
+ 1600, 1920, 2400, 3200, 4800, 9600,
+ default: 1600kHz
+- qcom,ovp: V; Over-voltage protection limit; one of:
+ 27, 29, 32, 35
+ default: 29V
+- qcom,num-strings: #; number of led strings attached; value from 1 to 3
+ default: 2
+
+Example:
+
+pm8941-wled@d800 {
+ compatible = "qcom,pm8941-wled";
+ reg = <0xd800>;
+ label = "backlight";
+
+ qcom,cs-out;
+ qcom,current-limit = <20>;
+ qcom,current-boost-limit = <805>;
+ qcom,switching-freq = <1600>;
+ qcom,ovp = <29>;
+ qcom,num-strings = <2>;
+};
--
1.8.2.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-09 0:22 [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bjorn Andersson @ 2014-12-09 0:22 ` Bjorn Andersson 2014-12-17 0:54 ` Bryan Wu 2014-12-17 1:15 ` Stephen Boyd 2014-12-10 1:19 ` [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bryan Wu 2014-12-17 1:15 ` Stephen Boyd 2 siblings, 2 replies; 11+ messages in thread From: Bjorn Andersson @ 2014-12-09 0:22 UTC (permalink / raw) To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie, Grant Likely Cc: Courtney Cavin, devicetree, linux-kernel, linux-leds, linux-arm-msm From: Courtney Cavin <courtney.cavin@sonymobile.com> This adds support for the WLED ('White' LED) block on Qualcomm's PM8941 PMICs. Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> --- drivers/leds/Kconfig | 8 + drivers/leds/Makefile | 1 + drivers/leds/leds-pm8941-wled.c | 466 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 475 insertions(+) create mode 100644 drivers/leds/leds-pm8941-wled.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a210338..9091cf4 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -505,6 +505,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_PM8941_WLED + tristate "LED support for the Qualcomm PM8941 WLED block" + depends on LEDS_CLASS + depends on REGMAP + help + This option enables support for the 'White' LED block + on Qualcomm PM8941 PMICs. + comment "LED Triggers" source "drivers/leds/trigger/Kconfig" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index a2b1647..5474afc 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o obj-$(CONFIG_LEDS_SYSCON) += leds-syscon.o obj-$(CONFIG_LEDS_VERSATILE) += leds-versatile.o obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o +obj-$(CONFIG_LEDS_PM8941_WLED) += leds-pm8941-wled.o # LED SPI Drivers obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/leds/leds-pm8941-wled.c new file mode 100644 index 0000000..aa06d0b --- /dev/null +++ b/drivers/leds/leds-pm8941-wled.c @@ -0,0 +1,466 @@ +/* Copyright (c) 2013, Sony Mobile Communications, AB. + * + * 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/leds.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/regmap.h> + +enum pm8941_wled_reg { + PM8941_WLED_REG_VAL_BASE = 0x40, +#define PM8941_WLED_REG_VAL_MAX 0xFFF + + PM8941_WLED_REG_MOD_EN = 0x46, +#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) +#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) + + PM8941_WLED_REG_SYNC = 0x47, +#define PM8941_WLED_REG_SYNC_MASK 0x07 +#define PM8941_WLED_REG_SYNC_LED1 BIT(0) +#define PM8941_WLED_REG_SYNC_LED2 BIT(1) +#define PM8941_WLED_REG_SYNC_LED3 BIT(2) +#define PM8941_WLED_REG_SYNC_ALL 0x07 +#define PM8941_WLED_REG_SYNC_CLEAR 0x00 + + PM8941_WLED_REG_FREQ = 0x4c, +#define PM8941_WLED_REG_FREQ_MASK 0x0f + + PM8941_WLED_REG_OVP = 0x4d, +#define PM8941_WLED_REG_OVP_MASK 0x03 + + PM8941_WLED_REG_BOOST = 0x4e, +#define PM8941_WLED_REG_BOOST_MASK 0x07 + + PM8941_WLED_REG_SINK = 0x4f, +#define PM8941_WLED_REG_SINK_MASK 0xe0 +#define PM8941_WLED_REG_SINK_SHFT 0x05 + +/* Per-'string' registers below */ +#define PM8941_WLED_REG_STR_OFFSET 0x10 + + PM8941_WLED_REG_STR_MOD_EN_BASE = 0x60, +#define PM8941_WLED_REG_STR_MOD_EN BIT(7) +#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) + + PM8941_WLED_REG_STR_SCALE_BASE = 0x62, +#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f + + PM8941_WLED_REG_STR_MOD_SRC_BASE = 0x63, +#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 +#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 +#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 + + PM8941_WLED_REG_STR_CABC_BASE = 0x66, +#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) +#define PM8941_WLED_REG_STR_CABC_EN BIT(7) +}; + +struct pm8941_wled_config { + u32 i_boost_limit; + u32 ovp; + u32 switch_freq; + u32 num_strings; + u32 i_limit; + bool cs_out_en; + bool ext_gen; + bool cabc_en; +}; + +struct pm8941_wled { + struct led_classdev cdev; + struct pm8941_wled_config cfg; + u16 addr; +}; + +struct pm8941_wled_context { + struct pm8941_wled wled; + struct regmap *regmap; +}; + +static int pm8941_wled_set(struct led_classdev *cdev, + enum led_brightness value) +{ + struct pm8941_wled_context *ctx; + struct pm8941_wled *wled; + u8 ctrl = 0; + u16 val; + int rc; + int i; + + wled = container_of(cdev, struct pm8941_wled, cdev); + ctx = container_of(wled, struct pm8941_wled_context, wled); + + if (value != 0) + ctrl = PM8941_WLED_REG_MOD_EN_BIT; + + val = (value * PM8941_WLED_REG_VAL_MAX) / LED_FULL; + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_MOD_EN, + PM8941_WLED_REG_MOD_EN_MASK, ctrl); + if (rc) + return rc; + + for (i = 0; i < wled->cfg.num_strings; ++i) { + u8 v[2] = { val & 0xff, (val >> 8) & 0xf }; + + rc = regmap_bulk_write(ctx->regmap, + wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i, + v, 2); + if (rc) + return rc; + } + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_SYNC, + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL); + if (rc) + return rc; + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_SYNC, + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR); + return rc; +} + +static void pm8941_wled_set_brightness(struct led_classdev *cdev, + enum led_brightness value) +{ + if (pm8941_wled_set(cdev, value)) { + dev_err(cdev->dev, "Unable to set brightness\n"); + return; + } + cdev->brightness = value; +} + +static int pm8941_wled_setup(struct pm8941_wled *wled) +{ + struct pm8941_wled_context *ctx; + int rc; + int i; + + ctx = container_of(wled, struct pm8941_wled_context, wled); + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_OVP, + PM8941_WLED_REG_OVP_MASK, wled->cfg.ovp); + if (rc) + return rc; + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_BOOST, + PM8941_WLED_REG_BOOST_MASK, wled->cfg.i_boost_limit); + if (rc) + return rc; + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_FREQ, + PM8941_WLED_REG_FREQ_MASK, wled->cfg.switch_freq); + if (rc) + return rc; + + if (wled->cfg.cs_out_en) { + u8 all = (BIT(wled->cfg.num_strings) - 1) + << PM8941_WLED_REG_SINK_SHFT; + + rc = regmap_update_bits(ctx->regmap, + wled->addr + PM8941_WLED_REG_SINK, + PM8941_WLED_REG_SINK_MASK, all); + if (rc) + return rc; + } + + for (i = 0; i < wled->cfg.num_strings; ++i) { + u16 addr = wled->addr + PM8941_WLED_REG_STR_OFFSET * i; + + rc = regmap_update_bits(ctx->regmap, + addr + PM8941_WLED_REG_STR_MOD_EN_BASE, + PM8941_WLED_REG_STR_MOD_MASK, + PM8941_WLED_REG_STR_MOD_EN); + if (rc) + return rc; + + if (wled->cfg.ext_gen) { + rc = regmap_update_bits(ctx->regmap, + addr + PM8941_WLED_REG_STR_MOD_SRC_BASE, + PM8941_WLED_REG_STR_MOD_SRC_MASK, + PM8941_WLED_REG_STR_MOD_SRC_EXT); + if (rc) + return rc; + } + + rc = regmap_update_bits(ctx->regmap, + addr + PM8941_WLED_REG_STR_SCALE_BASE, + PM8941_WLED_REG_STR_SCALE_MASK, + wled->cfg.i_limit); + if (rc) + return rc; + + rc = regmap_update_bits(ctx->regmap, + addr + PM8941_WLED_REG_STR_CABC_BASE, + PM8941_WLED_REG_STR_CABC_MASK, + wled->cfg.cabc_en ? + PM8941_WLED_REG_STR_CABC_EN : 0); + if (rc) + return rc; + } + + return 0; +} + +static const struct pm8941_wled_config pm8941_wled_config_defaults = { + .i_boost_limit = 3, + .i_limit = 20, + .ovp = 2, + .switch_freq = 5, + .num_strings = 0, + .cs_out_en = false, + .ext_gen = false, + .cabc_en = false, +}; + +struct pm8941_wled_var_cfg { + const u32 *values; + u32 (*fn)(u32); + int size; +}; + +static const u32 pm8941_wled_i_boost_limit_values[] = { + 105, 385, 525, 805, 980, 1260, 1400, 1680, +}; + +static const struct pm8941_wled_var_cfg pm8941_wled_i_boost_limit_cfg = { + .values = pm8941_wled_i_boost_limit_values, + .size = ARRAY_SIZE(pm8941_wled_i_boost_limit_values), +}; + +static const u32 pm8941_wled_ovp_values[] = { + 35, 32, 29, 27, +}; + +static const struct pm8941_wled_var_cfg pm8941_wled_ovp_cfg = { + .values = pm8941_wled_ovp_values, + .size = ARRAY_SIZE(pm8941_wled_ovp_values), +}; + +static u32 pm8941_wled_num_strings_values_fn(u32 idx) +{ + return idx + 1; +} + +static const struct pm8941_wled_var_cfg pm8941_wled_num_strings_cfg = { + .fn = pm8941_wled_num_strings_values_fn, + .size = 3, +}; + +static u32 pm8941_wled_switch_freq_values_fn(u32 idx) +{ + return 19200 / (2 * (1 + idx)); +} + +static const struct pm8941_wled_var_cfg pm8941_wled_switch_freq_cfg = { + .fn = pm8941_wled_switch_freq_values_fn, + .size = 16, +}; + +static const struct pm8941_wled_var_cfg pm8941_wled_i_limit_cfg = { + .size = 26, +}; + +static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx) +{ + if (idx >= cfg->size) + return UINT_MAX; + if (cfg->fn) + return cfg->fn(idx); + if (cfg->values) + return cfg->values[idx]; + return idx; +} + +static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) +{ + struct pm8941_wled_config *cfg = &wled->cfg; + u32 val; + int rc; + int i; + + const struct { + const char *name; + u32 *val_ptr; + const struct pm8941_wled_var_cfg *cfg; + } u32_opts[] = { + { + "qcom,current-boost-limit", &cfg->i_boost_limit, + .cfg = &pm8941_wled_i_boost_limit_cfg, + }, + { + "qcom,current-limit", &cfg->i_limit, + .cfg = &pm8941_wled_i_limit_cfg, + }, + { + "qcom,ovp", &cfg->ovp, + .cfg = &pm8941_wled_ovp_cfg, + }, + { + "qcom,switching-freq", &cfg->switch_freq, + .cfg = &pm8941_wled_switch_freq_cfg, + }, + { + "qcom,num-strings", &cfg->num_strings, + .cfg = &pm8941_wled_num_strings_cfg, + }, + }; + const struct { + const char *name; + bool *val_ptr; + } bool_opts[] = { + { "qcom,cs-out", &cfg->cs_out_en, }, + { "qcom,ext-gen", &cfg->ext_gen, }, + { "qcom,cabc", &cfg->cabc_en, }, + }; + + if (dev->of_node == NULL) { + dev_err(dev, "no OF node!\n"); + return -EINVAL; + } + + rc = of_property_read_u32(dev->of_node, "reg", &val); + if (rc || val > 0xffff) { + dev_err(dev, "invalid IO resources\n"); + return rc ? rc : -EINVAL; + } + wled->addr = val; + + rc = of_property_read_string(dev->of_node, "label", &wled->cdev.name); + if (rc) + wled->cdev.name = dev->of_node->name; + + wled->cdev.default_trigger = of_get_property(dev->of_node, + "linux,default-trigger", NULL); + + *cfg = pm8941_wled_config_defaults; + for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) { + u32 sel, c; + int j, rj; + + rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); + if (rc) { + if (rc != -EINVAL) { + dev_err(dev, "error reading '%s'\n", + u32_opts[i].name); + return rc; + } + continue; + } + + sel = UINT_MAX; + rj = -1; + c = pm8941_wled_values(u32_opts[i].cfg, 0); + for (j = 0; c != UINT_MAX; ++j) { + if (c <= val && (sel == UINT_MAX || c >= sel)) { + sel = c; + rj = j; + } + c = pm8941_wled_values(u32_opts[i].cfg, j + 1); + } + if (sel == UINT_MAX) { + dev_err(dev, "invalid value for '%s'\n", + u32_opts[i].name); + return rc; + } + if (sel != val) { + dev_warn(dev, "rounding '%s' down to %u\n", + u32_opts[i].name, sel); + } + dev_dbg(dev, "'%s' = %u\n", u32_opts[i].name, sel); + *u32_opts[i].val_ptr = rj; + }; + + for (i = 0; i < ARRAY_SIZE(bool_opts); ++i) { + if (of_property_read_bool(dev->of_node, bool_opts[i].name)) + *bool_opts[i].val_ptr = true; + } + + cfg->num_strings = cfg->num_strings + 1; + + return 0; +} + +static int pm8941_wled_probe(struct platform_device *pdev) +{ + struct pm8941_wled_context *ctx; + struct regmap *regmap; + int rc; + + regmap = dev_get_regmap(pdev->dev.parent, NULL); + if (!regmap) { + dev_err(&pdev->dev, "Unable to get regmap\n"); + return -EINVAL; + } + + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); + if (ctx == NULL) + return -ENOMEM; + + ctx->regmap = regmap; + + rc = pm8941_wled_configure(&ctx->wled, &pdev->dev); + if (rc) + return rc; + + rc = pm8941_wled_setup(&ctx->wled); + if (rc) + return rc; + + ctx->wled.cdev.brightness_set = pm8941_wled_set_brightness; + + rc = led_classdev_register(&pdev->dev, &ctx->wled.cdev); + if (rc) + return rc; + + platform_set_drvdata(pdev, ctx); + dev_info(&pdev->dev, "registered led \"%s\"\n", ctx->wled.cdev.name); + + return 0; +}; + +static int pm8941_wled_remove(struct platform_device *pdev) +{ + struct pm8941_wled_context *ctx; + + ctx = platform_get_drvdata(pdev); + led_classdev_unregister(&ctx->wled.cdev); + + return 0; +} + +static const struct of_device_id pm8941_wled_match_table[] = { + { .compatible = "qcom,pm8941-wled" }, + {} +}; + +static struct platform_driver pm8941_wled_driver = { + .probe = pm8941_wled_probe, + .remove = pm8941_wled_remove, + .driver = { + .name = "pm8941-wled", + .owner = THIS_MODULE, + .of_match_table = pm8941_wled_match_table, + }, +}; + +module_platform_driver(pm8941_wled_driver); + +MODULE_DESCRIPTION("pm8941 wled driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform: pm8941-wled"); -- 1.8.2.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson @ 2014-12-17 0:54 ` Bryan Wu 2014-12-18 23:43 ` Bjorn 2014-12-17 1:15 ` Stephen Boyd 1 sibling, 1 reply; 11+ messages in thread From: Bryan Wu @ 2014-12-17 0:54 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Richard Purdie, Grant Likely, Courtney Cavin, devicetree@vger.kernel.org, lkml, Linux LED Subsystem, linux-arm-msm On Mon, Dec 8, 2014 at 4:22 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote: > From: Courtney Cavin <courtney.cavin@sonymobile.com> > > This adds support for the WLED ('White' LED) block on Qualcomm's > PM8941 PMICs. > > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > --- > drivers/leds/Kconfig | 8 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-pm8941-wled.c | 466 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 475 insertions(+) > create mode 100644 drivers/leds/leds-pm8941-wled.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a210338..9091cf4 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -505,6 +505,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_PM8941_WLED > + tristate "LED support for the Qualcomm PM8941 WLED block" > + depends on LEDS_CLASS > + depends on REGMAP Here should be "select REGMAP" > + help > + This option enables support for the 'White' LED block > + on Qualcomm PM8941 PMICs. > + > comment "LED Triggers" > source "drivers/leds/trigger/Kconfig" > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index a2b1647..5474afc 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -56,6 +56,7 @@ obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o > obj-$(CONFIG_LEDS_SYSCON) += leds-syscon.o > obj-$(CONFIG_LEDS_VERSATILE) += leds-versatile.o > obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o > +obj-$(CONFIG_LEDS_PM8941_WLED) += leds-pm8941-wled.o > > # LED SPI Drivers > obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o > diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/leds/leds-pm8941-wled.c > new file mode 100644 > index 0000000..aa06d0b > --- /dev/null > +++ b/drivers/leds/leds-pm8941-wled.c > @@ -0,0 +1,466 @@ > +/* Copyright (c) 2013, Sony Mobile Communications, AB. > + * > + * 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/leds.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > + > +enum pm8941_wled_reg { > + PM8941_WLED_REG_VAL_BASE = 0x40, > +#define PM8941_WLED_REG_VAL_MAX 0xFFF > + > + PM8941_WLED_REG_MOD_EN = 0x46, > +#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) > +#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) > + > + PM8941_WLED_REG_SYNC = 0x47, > +#define PM8941_WLED_REG_SYNC_MASK 0x07 > +#define PM8941_WLED_REG_SYNC_LED1 BIT(0) > +#define PM8941_WLED_REG_SYNC_LED2 BIT(1) > +#define PM8941_WLED_REG_SYNC_LED3 BIT(2) > +#define PM8941_WLED_REG_SYNC_ALL 0x07 > +#define PM8941_WLED_REG_SYNC_CLEAR 0x00 > + > + PM8941_WLED_REG_FREQ = 0x4c, > +#define PM8941_WLED_REG_FREQ_MASK 0x0f > + > + PM8941_WLED_REG_OVP = 0x4d, > +#define PM8941_WLED_REG_OVP_MASK 0x03 > + > + PM8941_WLED_REG_BOOST = 0x4e, > +#define PM8941_WLED_REG_BOOST_MASK 0x07 > + > + PM8941_WLED_REG_SINK = 0x4f, > +#define PM8941_WLED_REG_SINK_MASK 0xe0 > +#define PM8941_WLED_REG_SINK_SHFT 0x05 > + > +/* Per-'string' registers below */ > +#define PM8941_WLED_REG_STR_OFFSET 0x10 > + > + PM8941_WLED_REG_STR_MOD_EN_BASE = 0x60, > +#define PM8941_WLED_REG_STR_MOD_EN BIT(7) > +#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) > + > + PM8941_WLED_REG_STR_SCALE_BASE = 0x62, > +#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f > + > + PM8941_WLED_REG_STR_MOD_SRC_BASE = 0x63, > +#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 > +#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 > +#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 > + > + PM8941_WLED_REG_STR_CABC_BASE = 0x66, > +#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) > +#define PM8941_WLED_REG_STR_CABC_EN BIT(7) > +}; > + > +struct pm8941_wled_config { > + u32 i_boost_limit; > + u32 ovp; > + u32 switch_freq; > + u32 num_strings; > + u32 i_limit; > + bool cs_out_en; > + bool ext_gen; > + bool cabc_en; > +}; > + > +struct pm8941_wled { > + struct led_classdev cdev; > + struct pm8941_wled_config cfg; > + u16 addr; > +}; > + > +struct pm8941_wled_context { > + struct pm8941_wled wled; > + struct regmap *regmap; > +}; > + I think you can just add *regmap into pm8941_wled struct and pm8941_wled_context is redundant to me. > +static int pm8941_wled_set(struct led_classdev *cdev, > + enum led_brightness value) > +{ > + struct pm8941_wled_context *ctx; > + struct pm8941_wled *wled; > + u8 ctrl = 0; > + u16 val; > + int rc; > + int i; > + > + wled = container_of(cdev, struct pm8941_wled, cdev); > + ctx = container_of(wled, struct pm8941_wled_context, wled); > + > + if (value != 0) > + ctrl = PM8941_WLED_REG_MOD_EN_BIT; > + > + val = (value * PM8941_WLED_REG_VAL_MAX) / LED_FULL; > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_MOD_EN, > + PM8941_WLED_REG_MOD_EN_MASK, ctrl); > + if (rc) > + return rc; > + > + for (i = 0; i < wled->cfg.num_strings; ++i) { > + u8 v[2] = { val & 0xff, (val >> 8) & 0xf }; > + > + rc = regmap_bulk_write(ctx->regmap, > + wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i, > + v, 2); > + if (rc) > + return rc; > + } > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_SYNC, > + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL); > + if (rc) > + return rc; > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_SYNC, > + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR); > + return rc; > +} > + > +static void pm8941_wled_set_brightness(struct led_classdev *cdev, > + enum led_brightness value) > +{ > + if (pm8941_wled_set(cdev, value)) { > + dev_err(cdev->dev, "Unable to set brightness\n"); > + return; > + } > + cdev->brightness = value; > +} > + > +static int pm8941_wled_setup(struct pm8941_wled *wled) > +{ > + struct pm8941_wled_context *ctx; > + int rc; > + int i; > + > + ctx = container_of(wled, struct pm8941_wled_context, wled); > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_OVP, > + PM8941_WLED_REG_OVP_MASK, wled->cfg.ovp); > + if (rc) > + return rc; > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_BOOST, > + PM8941_WLED_REG_BOOST_MASK, wled->cfg.i_boost_limit); > + if (rc) > + return rc; > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_FREQ, > + PM8941_WLED_REG_FREQ_MASK, wled->cfg.switch_freq); > + if (rc) > + return rc; > + > + if (wled->cfg.cs_out_en) { > + u8 all = (BIT(wled->cfg.num_strings) - 1) > + << PM8941_WLED_REG_SINK_SHFT; > + > + rc = regmap_update_bits(ctx->regmap, > + wled->addr + PM8941_WLED_REG_SINK, > + PM8941_WLED_REG_SINK_MASK, all); > + if (rc) > + return rc; > + } > + > + for (i = 0; i < wled->cfg.num_strings; ++i) { > + u16 addr = wled->addr + PM8941_WLED_REG_STR_OFFSET * i; > + > + rc = regmap_update_bits(ctx->regmap, > + addr + PM8941_WLED_REG_STR_MOD_EN_BASE, > + PM8941_WLED_REG_STR_MOD_MASK, > + PM8941_WLED_REG_STR_MOD_EN); > + if (rc) > + return rc; > + > + if (wled->cfg.ext_gen) { > + rc = regmap_update_bits(ctx->regmap, > + addr + PM8941_WLED_REG_STR_MOD_SRC_BASE, > + PM8941_WLED_REG_STR_MOD_SRC_MASK, > + PM8941_WLED_REG_STR_MOD_SRC_EXT); > + if (rc) > + return rc; > + } > + > + rc = regmap_update_bits(ctx->regmap, > + addr + PM8941_WLED_REG_STR_SCALE_BASE, > + PM8941_WLED_REG_STR_SCALE_MASK, > + wled->cfg.i_limit); > + if (rc) > + return rc; > + > + rc = regmap_update_bits(ctx->regmap, > + addr + PM8941_WLED_REG_STR_CABC_BASE, > + PM8941_WLED_REG_STR_CABC_MASK, > + wled->cfg.cabc_en ? > + PM8941_WLED_REG_STR_CABC_EN : 0); > + if (rc) > + return rc; > + } > + > + return 0; > +} > + > +static const struct pm8941_wled_config pm8941_wled_config_defaults = { > + .i_boost_limit = 3, > + .i_limit = 20, > + .ovp = 2, > + .switch_freq = 5, > + .num_strings = 0, > + .cs_out_en = false, > + .ext_gen = false, > + .cabc_en = false, > +}; > + > +struct pm8941_wled_var_cfg { > + const u32 *values; > + u32 (*fn)(u32); > + int size; > +}; > + > +static const u32 pm8941_wled_i_boost_limit_values[] = { > + 105, 385, 525, 805, 980, 1260, 1400, 1680, > +}; > + > +static const struct pm8941_wled_var_cfg pm8941_wled_i_boost_limit_cfg = { > + .values = pm8941_wled_i_boost_limit_values, > + .size = ARRAY_SIZE(pm8941_wled_i_boost_limit_values), > +}; > + > +static const u32 pm8941_wled_ovp_values[] = { > + 35, 32, 29, 27, > +}; > + > +static const struct pm8941_wled_var_cfg pm8941_wled_ovp_cfg = { > + .values = pm8941_wled_ovp_values, > + .size = ARRAY_SIZE(pm8941_wled_ovp_values), > +}; > + > +static u32 pm8941_wled_num_strings_values_fn(u32 idx) > +{ > + return idx + 1; > +} > + > +static const struct pm8941_wled_var_cfg pm8941_wled_num_strings_cfg = { > + .fn = pm8941_wled_num_strings_values_fn, > + .size = 3, > +}; > + > +static u32 pm8941_wled_switch_freq_values_fn(u32 idx) > +{ > + return 19200 / (2 * (1 + idx)); > +} > + > +static const struct pm8941_wled_var_cfg pm8941_wled_switch_freq_cfg = { > + .fn = pm8941_wled_switch_freq_values_fn, > + .size = 16, > +}; > + > +static const struct pm8941_wled_var_cfg pm8941_wled_i_limit_cfg = { > + .size = 26, > +}; > + > +static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx) > +{ > + if (idx >= cfg->size) > + return UINT_MAX; > + if (cfg->fn) > + return cfg->fn(idx); > + if (cfg->values) > + return cfg->values[idx]; > + return idx; > +} > + > +static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) > +{ > + struct pm8941_wled_config *cfg = &wled->cfg; > + u32 val; > + int rc; > + int i; > + > + const struct { > + const char *name; > + u32 *val_ptr; > + const struct pm8941_wled_var_cfg *cfg; > + } u32_opts[] = { > + { > + "qcom,current-boost-limit", &cfg->i_boost_limit, > + .cfg = &pm8941_wled_i_boost_limit_cfg, > + }, > + { > + "qcom,current-limit", &cfg->i_limit, > + .cfg = &pm8941_wled_i_limit_cfg, > + }, > + { > + "qcom,ovp", &cfg->ovp, > + .cfg = &pm8941_wled_ovp_cfg, > + }, > + { > + "qcom,switching-freq", &cfg->switch_freq, > + .cfg = &pm8941_wled_switch_freq_cfg, > + }, > + { > + "qcom,num-strings", &cfg->num_strings, > + .cfg = &pm8941_wled_num_strings_cfg, > + }, > + }; > + const struct { > + const char *name; > + bool *val_ptr; > + } bool_opts[] = { > + { "qcom,cs-out", &cfg->cs_out_en, }, > + { "qcom,ext-gen", &cfg->ext_gen, }, > + { "qcom,cabc", &cfg->cabc_en, }, > + }; > + > + if (dev->of_node == NULL) { > + dev_err(dev, "no OF node!\n"); > + return -EINVAL; > + } > + > + rc = of_property_read_u32(dev->of_node, "reg", &val); > + if (rc || val > 0xffff) { > + dev_err(dev, "invalid IO resources\n"); > + return rc ? rc : -EINVAL; > + } > + wled->addr = val; > + > + rc = of_property_read_string(dev->of_node, "label", &wled->cdev.name); > + if (rc) > + wled->cdev.name = dev->of_node->name; > + > + wled->cdev.default_trigger = of_get_property(dev->of_node, > + "linux,default-trigger", NULL); > + > + *cfg = pm8941_wled_config_defaults; > + for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) { > + u32 sel, c; > + int j, rj; > + > + rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); > + if (rc) { > + if (rc != -EINVAL) { > + dev_err(dev, "error reading '%s'\n", > + u32_opts[i].name); > + return rc; > + } > + continue; > + } > + > + sel = UINT_MAX; > + rj = -1; > + c = pm8941_wled_values(u32_opts[i].cfg, 0); > + for (j = 0; c != UINT_MAX; ++j) { > + if (c <= val && (sel == UINT_MAX || c >= sel)) { > + sel = c; > + rj = j; > + } > + c = pm8941_wled_values(u32_opts[i].cfg, j + 1); > + } > + if (sel == UINT_MAX) { > + dev_err(dev, "invalid value for '%s'\n", > + u32_opts[i].name); > + return rc; > + } > + if (sel != val) { > + dev_warn(dev, "rounding '%s' down to %u\n", > + u32_opts[i].name, sel); > + } > + dev_dbg(dev, "'%s' = %u\n", u32_opts[i].name, sel); > + *u32_opts[i].val_ptr = rj; > + }; > + > + for (i = 0; i < ARRAY_SIZE(bool_opts); ++i) { > + if (of_property_read_bool(dev->of_node, bool_opts[i].name)) > + *bool_opts[i].val_ptr = true; > + } > + > + cfg->num_strings = cfg->num_strings + 1; > + > + return 0; > +} > + > +static int pm8941_wled_probe(struct platform_device *pdev) > +{ > + struct pm8941_wled_context *ctx; > + struct regmap *regmap; > + int rc; > + > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!regmap) { > + dev_err(&pdev->dev, "Unable to get regmap\n"); > + return -EINVAL; > + } > + Is this regmap mmio registers or i2c registers? Will the regmap operation sleep? > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > + if (ctx == NULL) > + return -ENOMEM; > + > + ctx->regmap = regmap; > + > + rc = pm8941_wled_configure(&ctx->wled, &pdev->dev); > + if (rc) > + return rc; > + > + rc = pm8941_wled_setup(&ctx->wled); > + if (rc) > + return rc; > + > + ctx->wled.cdev.brightness_set = pm8941_wled_set_brightness; > + > + rc = led_classdev_register(&pdev->dev, &ctx->wled.cdev); > + if (rc) > + return rc; > + > + platform_set_drvdata(pdev, ctx); > + dev_info(&pdev->dev, "registered led \"%s\"\n", ctx->wled.cdev.name); > + > + return 0; > +}; > + > +static int pm8941_wled_remove(struct platform_device *pdev) > +{ > + struct pm8941_wled_context *ctx; > + > + ctx = platform_get_drvdata(pdev); > + led_classdev_unregister(&ctx->wled.cdev); > + > + return 0; > +} > + > +static const struct of_device_id pm8941_wled_match_table[] = { > + { .compatible = "qcom,pm8941-wled" }, > + {} > +}; > + > +static struct platform_driver pm8941_wled_driver = { > + .probe = pm8941_wled_probe, > + .remove = pm8941_wled_remove, > + .driver = { > + .name = "pm8941-wled", > + .owner = THIS_MODULE, > + .of_match_table = pm8941_wled_match_table, > + }, > +}; > + > +module_platform_driver(pm8941_wled_driver); > + > +MODULE_DESCRIPTION("pm8941 wled driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform: pm8941-wled"); No space after ":" > -- > 1.8.2.2 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-17 0:54 ` Bryan Wu @ 2014-12-18 23:43 ` Bjorn 2014-12-19 8:15 ` Ivan T. Ivanov 0 siblings, 1 reply; 11+ messages in thread From: Bjorn @ 2014-12-18 23:43 UTC (permalink / raw) To: Bryan Wu Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Richard Purdie, Grant Likely, Cavin, Courtney, devicetree@vger.kernel.org, lkml, Linux LED Subsystem, linux-arm-msm@vger.kernel.org On Tue 16 Dec 16:54 PST 2014, Bryan Wu wrote: > On Mon, Dec 8, 2014 at 4:22 PM, Bjorn Andersson [..] > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig [..] > > +config LEDS_PM8941_WLED > > + tristate "LED support for the Qualcomm PM8941 WLED block" > > + depends on LEDS_CLASS > > + depends on REGMAP > Here should be "select REGMAP" > Thanks > > + help > > + This option enables support for the 'White' LED block > > + on Qualcomm PM8941 PMICs. > > + [..] > > diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/leds/leds-pm8941-wled.c [..] > > + > > +struct pm8941_wled_context { > > + struct pm8941_wled wled; > > + struct regmap *regmap; > > +}; > > + > > I think you can just add *regmap into pm8941_wled struct and > pm8941_wled_context is redundant to me. > Looks like a development leftover, I will fold it in. [..] > > + > > +static int pm8941_wled_probe(struct platform_device *pdev) > > +{ > > + struct pm8941_wled_context *ctx; > > + struct regmap *regmap; > > + int rc; > > + > > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > > + if (!regmap) { > > + dev_err(&pdev->dev, "Unable to get regmap\n"); > > + return -EINVAL; > > + } > > + > > Is this regmap mmio registers or i2c registers? Will the regmap > operation sleep? > It's a spmi regmap, in it's current configuration it's marked as "fast" i.e. it will not sleep. I discussed this with Courtney before sending the patch out and as this is used for backlight we don't see a reason calling set_brightness() in atomic context. If this changes I would send out a follow up patch. > > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > > + if (ctx == NULL) > > + return -ENOMEM; > > + > > + ctx->regmap = regmap; > > + > > + rc = pm8941_wled_configure(&ctx->wled, &pdev->dev); > > + if (rc) > > + return rc; > > + > > + rc = pm8941_wled_setup(&ctx->wled); > > + if (rc) > > + return rc; > > + > > + ctx->wled.cdev.brightness_set = pm8941_wled_set_brightness; > > + > > + rc = led_classdev_register(&pdev->dev, &ctx->wled.cdev); > > + if (rc) > > + return rc; > > + > > + platform_set_drvdata(pdev, ctx); > > + dev_info(&pdev->dev, "registered led \"%s\"\n", ctx->wled.cdev.name); > > + > > + return 0; > > +}; > > + [..] > > +MODULE_DESCRIPTION("pm8941 wled driver"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_ALIAS("platform: pm8941-wled"); > No space after ":" > Oops. Regards, Bjorn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-18 23:43 ` Bjorn @ 2014-12-19 8:15 ` Ivan T. Ivanov 0 siblings, 0 replies; 11+ messages in thread From: Ivan T. Ivanov @ 2014-12-19 8:15 UTC (permalink / raw) To: Bjorn Cc: Bryan Wu, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Richard Purdie, Grant Likely, Cavin, Courtney, devicetree@vger.kernel.org, lkml, Linux LED Subsystem, linux-arm-msm@vger.kernel.org On Thu, 2014-12-18 at 15:43 -0800, Bjorn wrote: > On Tue 16 Dec 16:54 PST 2014, Bryan Wu wrote: > > > On Mon, Dec 8, 2014 at 4:22 PM, Bjorn Andersson > > [..] > > > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > > [..] > > > > +config LEDS_PM8941_WLED > > > + tristate "LED support for the Qualcomm PM8941 WLED block" > > > + depends on LEDS_CLASS > > > + depends on REGMAP > > Here should be "select REGMAP" > > > > Thanks > > > If not mistaken the correct way was: depends on SPMI select REGMAP_SPMI or depends on MFD_SPMI_PMIC Regards, Ivan https://patchwork.kernel.org/patch/4566591/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson 2014-12-17 0:54 ` Bryan Wu @ 2014-12-17 1:15 ` Stephen Boyd 2014-12-18 23:57 ` Bjorn 1 sibling, 1 reply; 11+ messages in thread From: Stephen Boyd @ 2014-12-17 1:15 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie, Grant Likely Cc: Courtney Cavin, devicetree, linux-kernel, linux-leds, linux-arm-msm On 12/08/2014 04:22 PM, Bjorn Andersson wrote: > diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/leds/leds-pm8941-wled.c > new file mode 100644 > index 0000000..aa06d0b > --- /dev/null > +++ b/drivers/leds/leds-pm8941-wled.c > @@ -0,0 +1,466 @@ > +/* Copyright (c) 2013, Sony Mobile Communications, AB. > + * > + * 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/leds.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> Missing <linux/kernel.h> for container_of() macro. > + > +enum pm8941_wled_reg { > + PM8941_WLED_REG_VAL_BASE = 0x40, > +#define PM8941_WLED_REG_VAL_MAX 0xFFF > + > + PM8941_WLED_REG_MOD_EN = 0x46, > +#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) > +#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) > + > + PM8941_WLED_REG_SYNC = 0x47, > +#define PM8941_WLED_REG_SYNC_MASK 0x07 > +#define PM8941_WLED_REG_SYNC_LED1 BIT(0) > +#define PM8941_WLED_REG_SYNC_LED2 BIT(1) > +#define PM8941_WLED_REG_SYNC_LED3 BIT(2) > +#define PM8941_WLED_REG_SYNC_ALL 0x07 > +#define PM8941_WLED_REG_SYNC_CLEAR 0x00 > + > + PM8941_WLED_REG_FREQ = 0x4c, > +#define PM8941_WLED_REG_FREQ_MASK 0x0f > + > + PM8941_WLED_REG_OVP = 0x4d, > +#define PM8941_WLED_REG_OVP_MASK 0x03 > + > + PM8941_WLED_REG_BOOST = 0x4e, > +#define PM8941_WLED_REG_BOOST_MASK 0x07 > + > + PM8941_WLED_REG_SINK = 0x4f, > +#define PM8941_WLED_REG_SINK_MASK 0xe0 > +#define PM8941_WLED_REG_SINK_SHFT 0x05 > + > +/* Per-'string' registers below */ > +#define PM8941_WLED_REG_STR_OFFSET 0x10 > + > + PM8941_WLED_REG_STR_MOD_EN_BASE = 0x60, > +#define PM8941_WLED_REG_STR_MOD_EN BIT(7) > +#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) > + > + PM8941_WLED_REG_STR_SCALE_BASE = 0x62, > +#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f > + > + PM8941_WLED_REG_STR_MOD_SRC_BASE = 0x63, > +#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 > +#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 > +#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 > + > + PM8941_WLED_REG_STR_CABC_BASE = 0x66, > +#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) > +#define PM8941_WLED_REG_STR_CABC_EN BIT(7) > +}; I don't see any value in this enum. It's never used in a switch statement so why not just have #defines for the register offsets. It would make reading this a lot easier too. > + > +static int pm8941_wled_set(struct led_classdev *cdev, > + enum led_brightness value) > +{ > + struct pm8941_wled_context *ctx; > + struct pm8941_wled *wled; > + u8 ctrl = 0; > + u16 val; > + int rc; > + int i; > + > + wled = container_of(cdev, struct pm8941_wled, cdev); > + ctx = container_of(wled, struct pm8941_wled_context, wled); > + > + if (value != 0) > + ctrl = PM8941_WLED_REG_MOD_EN_BIT; > + > + val = (value * PM8941_WLED_REG_VAL_MAX) / LED_FULL; Unnecessary parens here. > + > + rc = regmap_update_bits(ctx->regmap, > > + > +static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) > +{ [...] > + > + if (dev->of_node == NULL) { > + dev_err(dev, "no OF node!\n"); > + return -EINVAL; > + } Seems like an unnecessary check. Everything's DT so far. > + > + rc = of_property_read_u32(dev->of_node, "reg", &val); > + if (rc || val > 0xffff) { > + dev_err(dev, "invalid IO resources\n"); > + return rc ? rc : -EINVAL; > + } > + wled->addr = val; > + > + rc = of_property_read_string(dev->of_node, "label", &wled->cdev.name); > + if (rc) > + wled->cdev.name = dev->of_node->name; > + > + wled->cdev.default_trigger = of_get_property(dev->of_node, > + "linux,default-trigger", NULL); > + > + *cfg = pm8941_wled_config_defaults; > + for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) { > + u32 sel, c; > + int j, rj; > + > + rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); > + if (rc) { > + if (rc != -EINVAL) { > + dev_err(dev, "error reading '%s'\n", > + u32_opts[i].name); > + return rc; > + } > + continue; > + } > + > + sel = UINT_MAX; > + rj = -1; > + c = pm8941_wled_values(u32_opts[i].cfg, 0); > + for (j = 0; c != UINT_MAX; ++j) { > + if (c <= val && (sel == UINT_MAX || c >= sel)) { > + sel = c; > + rj = j; > + } > + c = pm8941_wled_values(u32_opts[i].cfg, j + 1); > + } > + if (sel == UINT_MAX) { > + dev_err(dev, "invalid value for '%s'\n", > + u32_opts[i].name); > + return rc; > + } > + if (sel != val) { > + dev_warn(dev, "rounding '%s' down to %u\n", > + u32_opts[i].name, sel); > + } Do we really want to do all this complicated rounding and checking? Can't we just assume the DT is correct? > + dev_dbg(dev, "'%s' = %u\n", u32_opts[i].name, sel); > + *u32_opts[i].val_ptr = rj; > + }; > + > + for (i = 0; i < ARRAY_SIZE(bool_opts); ++i) { > + if (of_property_read_bool(dev->of_node, bool_opts[i].name)) > + *bool_opts[i].val_ptr = true; > + } > + > + cfg->num_strings = cfg->num_strings + 1; > + > + return 0; > +} > + > +static int pm8941_wled_probe(struct platform_device *pdev) > +{ > + struct pm8941_wled_context *ctx; > + struct regmap *regmap; > + int rc; > + > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!regmap) { > + dev_err(&pdev->dev, "Unable to get regmap\n"); > + return -EINVAL; > + } > + > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > + if (ctx == NULL) Nitpick only because it doesn't match the !regmap style above. Please use the !ctx style here as well. > + return -ENOMEM; > + > + ctx->regmap = regmap; > + > + rc = pm8941_wled_configure(&ctx->wled, &pdev->dev); > + if (rc) > + return rc; > + > + rc = pm8941_wled_setup(&ctx->wled); > + if (rc) > + return rc; > + > + ctx->wled.cdev.brightness_set = pm8941_wled_set_brightness; > + > + rc = led_classdev_register(&pdev->dev, &ctx->wled.cdev); > + if (rc) > + return rc; > + > + platform_set_drvdata(pdev, ctx); > + dev_info(&pdev->dev, "registered led \"%s\"\n", ctx->wled.cdev.name); Please don't spam the log with "I'm alive!" messages. > + > + return 0; > +}; > + > +static int pm8941_wled_remove(struct platform_device *pdev) > +{ > + struct pm8941_wled_context *ctx; > + > + ctx = platform_get_drvdata(pdev); > + led_classdev_unregister(&ctx->wled.cdev); > + > + return 0; > +} > + > +static const struct of_device_id pm8941_wled_match_table[] = { > + { .compatible = "qcom,pm8941-wled" }, > + {} > +}; MODULE_DEVICE_TABLE? > + > +static struct platform_driver pm8941_wled_driver = { > + .probe = pm8941_wled_probe, > + .remove = pm8941_wled_remove, > + .driver = { > + .name = "pm8941-wled", > + .owner = THIS_MODULE, > + .of_match_table = pm8941_wled_match_table, > + }, > +}; > + > +module_platform_driver(pm8941_wled_driver); > + > +MODULE_DESCRIPTION("pm8941 wled driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform: pm8941-wled"); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver 2014-12-17 1:15 ` Stephen Boyd @ 2014-12-18 23:57 ` Bjorn 0 siblings, 0 replies; 11+ messages in thread From: Bjorn @ 2014-12-18 23:57 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie, Grant Likely, Cavin, Courtney, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-arm-msm@vger.kernel.org On Tue 16 Dec 17:15 PST 2014, Stephen Boyd wrote: > On 12/08/2014 04:22 PM, Bjorn Andersson wrote: > > diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/leds/leds-pm8941-wled.c [..] > > +#include <linux/leds.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/regmap.h> > > Missing <linux/kernel.h> for container_of() macro. > Thanks > > + > > +enum pm8941_wled_reg { > > + PM8941_WLED_REG_VAL_BASE = 0x40, > > +#define PM8941_WLED_REG_VAL_MAX 0xFFF > > + > > + PM8941_WLED_REG_MOD_EN = 0x46, > > +#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) > > +#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) > > + > > + PM8941_WLED_REG_SYNC = 0x47, > > +#define PM8941_WLED_REG_SYNC_MASK 0x07 > > +#define PM8941_WLED_REG_SYNC_LED1 BIT(0) > > +#define PM8941_WLED_REG_SYNC_LED2 BIT(1) > > +#define PM8941_WLED_REG_SYNC_LED3 BIT(2) > > +#define PM8941_WLED_REG_SYNC_ALL 0x07 > > +#define PM8941_WLED_REG_SYNC_CLEAR 0x00 > > + > > + PM8941_WLED_REG_FREQ = 0x4c, > > +#define PM8941_WLED_REG_FREQ_MASK 0x0f > > + > > + PM8941_WLED_REG_OVP = 0x4d, > > +#define PM8941_WLED_REG_OVP_MASK 0x03 > > + > > + PM8941_WLED_REG_BOOST = 0x4e, > > +#define PM8941_WLED_REG_BOOST_MASK 0x07 > > + > > + PM8941_WLED_REG_SINK = 0x4f, > > +#define PM8941_WLED_REG_SINK_MASK 0xe0 > > +#define PM8941_WLED_REG_SINK_SHFT 0x05 > > + > > +/* Per-'string' registers below */ > > +#define PM8941_WLED_REG_STR_OFFSET 0x10 > > + > > + PM8941_WLED_REG_STR_MOD_EN_BASE = 0x60, > > +#define PM8941_WLED_REG_STR_MOD_EN BIT(7) > > +#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) > > + > > + PM8941_WLED_REG_STR_SCALE_BASE = 0x62, > > +#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f > > + > > + PM8941_WLED_REG_STR_MOD_SRC_BASE = 0x63, > > +#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 > > +#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 > > +#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 > > + > > + PM8941_WLED_REG_STR_CABC_BASE = 0x66, > > +#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) > > +#define PM8941_WLED_REG_STR_CABC_EN BIT(7) > > +}; > > I don't see any value in this enum. It's never used in a switch > statement so why not just have #defines for the register offsets. It > would make reading this a lot easier too. > Make sense, I'll rework it. > > + > > +static int pm8941_wled_set(struct led_classdev *cdev, > > + enum led_brightness value) > > +{ > > + struct pm8941_wled_context *ctx; > > + struct pm8941_wled *wled; > > + u8 ctrl = 0; > > + u16 val; > > + int rc; > > + int i; > > + > > + wled = container_of(cdev, struct pm8941_wled, cdev); > > + ctx = container_of(wled, struct pm8941_wled_context, wled); > > + > > + if (value != 0) > > + ctrl = PM8941_WLED_REG_MOD_EN_BIT; > > + > > + val = (value * PM8941_WLED_REG_VAL_MAX) / LED_FULL; > > Unnecessary parens here. > Agreed > > + > > + rc = regmap_update_bits(ctx->regmap, > > > > + > > +static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) > > +{ > [...] > > + > > + if (dev->of_node == NULL) { > > + dev_err(dev, "no OF node!\n"); > > + return -EINVAL; > > + } > > Seems like an unnecessary check. Everything's DT so far. > Agreed. > > + > > + rc = of_property_read_u32(dev->of_node, "reg", &val); > > + if (rc || val > 0xffff) { > > + dev_err(dev, "invalid IO resources\n"); > > + return rc ? rc : -EINVAL; > > + } > > + wled->addr = val; > > + > > + rc = of_property_read_string(dev->of_node, "label", &wled->cdev.name); > > + if (rc) > > + wled->cdev.name = dev->of_node->name; > > + > > + wled->cdev.default_trigger = of_get_property(dev->of_node, > > + "linux,default-trigger", NULL); > > + > > + *cfg = pm8941_wled_config_defaults; > > + for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) { > > + u32 sel, c; > > + int j, rj; > > + > > + rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); > > + if (rc) { > > + if (rc != -EINVAL) { > > + dev_err(dev, "error reading '%s'\n", > > + u32_opts[i].name); > > + return rc; > > + } > > + continue; > > + } > > + > > + sel = UINT_MAX; > > + rj = -1; > > + c = pm8941_wled_values(u32_opts[i].cfg, 0); > > + for (j = 0; c != UINT_MAX; ++j) { > > + if (c <= val && (sel == UINT_MAX || c >= sel)) { > > + sel = c; > > + rj = j; > > + } > > + c = pm8941_wled_values(u32_opts[i].cfg, j + 1); > > + } > > + if (sel == UINT_MAX) { > > + dev_err(dev, "invalid value for '%s'\n", > > + u32_opts[i].name); > > + return rc; > > + } > > + if (sel != val) { > > + dev_warn(dev, "rounding '%s' down to %u\n", > > + u32_opts[i].name, sel); > > + } > > Do we really want to do all this complicated rounding and checking? > Can't we just assume the DT is correct? > We could remove the printout about rounding, but the logic turns human-readable values into register values (indices), so we need something for that. > > + dev_dbg(dev, "'%s' = %u\n", u32_opts[i].name, sel); > > + *u32_opts[i].val_ptr = rj; > > + }; > > + > > + for (i = 0; i < ARRAY_SIZE(bool_opts); ++i) { > > + if (of_property_read_bool(dev->of_node, bool_opts[i].name)) > > + *bool_opts[i].val_ptr = true; > > + } > > + > > + cfg->num_strings = cfg->num_strings + 1; > > + > > + return 0; > > +} > > + > > +static int pm8941_wled_probe(struct platform_device *pdev) > > +{ > > + struct pm8941_wled_context *ctx; > > + struct regmap *regmap; > > + int rc; > > + > > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > > + if (!regmap) { > > + dev_err(&pdev->dev, "Unable to get regmap\n"); > > + return -EINVAL; > > + } > > + > > + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > > + if (ctx == NULL) > > Nitpick only because it doesn't match the !regmap style above. Please > use the !ctx style here as well. > Agreed > > + return -ENOMEM; > > + > > + ctx->regmap = regmap; > > + > > + rc = pm8941_wled_configure(&ctx->wled, &pdev->dev); > > + if (rc) > > + return rc; > > + > > + rc = pm8941_wled_setup(&ctx->wled); > > + if (rc) > > + return rc; > > + > > + ctx->wled.cdev.brightness_set = pm8941_wled_set_brightness; > > + > > + rc = led_classdev_register(&pdev->dev, &ctx->wled.cdev); > > + if (rc) > > + return rc; > > + > > + platform_set_drvdata(pdev, ctx); > > + dev_info(&pdev->dev, "registered led \"%s\"\n", ctx->wled.cdev.name); > > Please don't spam the log with "I'm alive!" messages. > Ok > > + > > + return 0; > > +}; > > + [..] > > + > > +static const struct of_device_id pm8941_wled_match_table[] = { > > + { .compatible = "qcom,pm8941-wled" }, > > + {} > > +}; > > MODULE_DEVICE_TABLE? > Indeed Thanks for the review Stephen. Regards, Bjorn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block 2014-12-09 0:22 [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bjorn Andersson 2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson @ 2014-12-10 1:19 ` Bryan Wu 2014-12-17 0:54 ` Bryan Wu 2014-12-17 1:15 ` Stephen Boyd 2 siblings, 1 reply; 11+ messages in thread From: Bryan Wu @ 2014-12-10 1:19 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Richard Purdie, Grant Likely, Courtney Cavin, devicetree@vger.kernel.org, lkml, Linux LED Subsystem, linux-arm-msm On Mon, Dec 8, 2014 at 4:22 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote: > From: Courtney Cavin <courtney.cavin@sonymobile.com> > > This adds device tree binding documentation for the WLED ('White' LED) > block on Qualcomm's PM8941 PMICs. > Looks good to me, I need device tree reviewer's Ack to merge it. Thanks, -Bryan > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > --- > .../devicetree/bindings/leds/leds-pm8941-wled.txt | 43 ++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt > new file mode 100644 > index 0000000..a85a964 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt > @@ -0,0 +1,43 @@ > +Binding for Qualcomm PM8941 WLED driver > + > +Required properties: > +- compatible: should be "qcom,pm8941-wled" > +- reg: slave address > + > +Optional properties: > +- label: The label for this led > + See Documentation/devicetree/bindings/leds/common.txt > +- linux,default-trigger: Default trigger assigned to the LED > + See Documentation/devicetree/bindings/leds/common.txt > +- qcom,cs-out: bool; enable current sink output > +- qcom,cabc: bool; enable content adaptive backlight control > +- qcom,ext-gen: bool; use externally generated modulator signal to dim > +- qcom,current-limit: mA; per-string current limit; value from 0 to 25 > + default: 20mA > +- qcom,current-boost-limit: mA; boost current limit; one of: > + 105, 385, 525, 805, 980, 1260, 1400, 1680 > + default: 805mA > +- qcom,switching-freq: kHz; switching frequency; one of: > + 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, > + 1600, 1920, 2400, 3200, 4800, 9600, > + default: 1600kHz > +- qcom,ovp: V; Over-voltage protection limit; one of: > + 27, 29, 32, 35 > + default: 29V > +- qcom,num-strings: #; number of led strings attached; value from 1 to 3 > + default: 2 > + > +Example: > + > +pm8941-wled@d800 { > + compatible = "qcom,pm8941-wled"; > + reg = <0xd800>; > + label = "backlight"; > + > + qcom,cs-out; > + qcom,current-limit = <20>; > + qcom,current-boost-limit = <805>; > + qcom,switching-freq = <1600>; > + qcom,ovp = <29>; > + qcom,num-strings = <2>; > +}; > -- > 1.8.2.2 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block 2014-12-10 1:19 ` [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bryan Wu @ 2014-12-17 0:54 ` Bryan Wu 0 siblings, 0 replies; 11+ messages in thread From: Bryan Wu @ 2014-12-17 0:54 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Richard Purdie, Grant Likely, Courtney Cavin, devicetree@vger.kernel.org, lkml, Linux LED Subsystem, linux-arm-msm On Tue, Dec 9, 2014 at 5:19 PM, Bryan Wu <cooloney@gmail.com> wrote: > On Mon, Dec 8, 2014 at 4:22 PM, Bjorn Andersson > <bjorn.andersson@sonymobile.com> wrote: >> From: Courtney Cavin <courtney.cavin@sonymobile.com> >> >> This adds device tree binding documentation for the WLED ('White' LED) >> block on Qualcomm's PM8941 PMICs. >> > > Looks good to me, I need device tree reviewer's Ack to merge it. > Hi Rob, can you help to review it? -Bryan > Thanks, > -Bryan > >> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> >> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >> --- >> .../devicetree/bindings/leds/leds-pm8941-wled.txt | 43 ++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt >> new file mode 100644 >> index 0000000..a85a964 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt >> @@ -0,0 +1,43 @@ >> +Binding for Qualcomm PM8941 WLED driver >> + >> +Required properties: >> +- compatible: should be "qcom,pm8941-wled" >> +- reg: slave address >> + >> +Optional properties: >> +- label: The label for this led >> + See Documentation/devicetree/bindings/leds/common.txt >> +- linux,default-trigger: Default trigger assigned to the LED >> + See Documentation/devicetree/bindings/leds/common.txt >> +- qcom,cs-out: bool; enable current sink output >> +- qcom,cabc: bool; enable content adaptive backlight control >> +- qcom,ext-gen: bool; use externally generated modulator signal to dim >> +- qcom,current-limit: mA; per-string current limit; value from 0 to 25 >> + default: 20mA >> +- qcom,current-boost-limit: mA; boost current limit; one of: >> + 105, 385, 525, 805, 980, 1260, 1400, 1680 >> + default: 805mA >> +- qcom,switching-freq: kHz; switching frequency; one of: >> + 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, >> + 1600, 1920, 2400, 3200, 4800, 9600, >> + default: 1600kHz >> +- qcom,ovp: V; Over-voltage protection limit; one of: >> + 27, 29, 32, 35 >> + default: 29V >> +- qcom,num-strings: #; number of led strings attached; value from 1 to 3 >> + default: 2 >> + >> +Example: >> + >> +pm8941-wled@d800 { >> + compatible = "qcom,pm8941-wled"; >> + reg = <0xd800>; >> + label = "backlight"; >> + >> + qcom,cs-out; >> + qcom,current-limit = <20>; >> + qcom,current-boost-limit = <805>; >> + qcom,switching-freq = <1600>; >> + qcom,ovp = <29>; >> + qcom,num-strings = <2>; >> +}; >> -- >> 1.8.2.2 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block 2014-12-09 0:22 [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bjorn Andersson 2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson 2014-12-10 1:19 ` [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bryan Wu @ 2014-12-17 1:15 ` Stephen Boyd 2014-12-18 23:36 ` Bjorn 2 siblings, 1 reply; 11+ messages in thread From: Stephen Boyd @ 2014-12-17 1:15 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie, Grant Likely Cc: Courtney Cavin, devicetree, linux-kernel, linux-leds, linux-arm-msm On 12/08/2014 04:22 PM, Bjorn Andersson wrote: > + > +Optional properties: > +- label: The label for this led > + See Documentation/devicetree/bindings/leds/common.txt > +- linux,default-trigger: Default trigger assigned to the LED > + See Documentation/devicetree/bindings/leds/common.txt > +- qcom,cs-out: bool; enable current sink output > +- qcom,cabc: bool; enable content adaptive backlight control > +- qcom,ext-gen: bool; use externally generated modulator signal to dim > +- qcom,current-limit: mA; per-string current limit; value from 0 to 25 > + default: 20mA > +- qcom,current-boost-limit: mA; boost current limit; one of: > + 105, 385, 525, 805, 980, 1260, 1400, 1680 > + default: 805mA > +- qcom,switching-freq: kHz; switching frequency; one of: > + 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, > + 1600, 1920, 2400, 3200, 4800, 9600, > + default: 1600kHz > +- qcom,ovp: V; Over-voltage protection limit; one of: > + 27, 29, 32, 35 > + default: 29V It would be nice if the units were part of the property name, i.e. qcom,ovp-V, qcom,current-boost-limit-mA, etc. That way the units are known without having to refer to the documentation. > +- qcom,num-strings: #; number of led strings attached; value from 1 to 3 > + default: 2 I wonder if qcom,#strings is more appropriate? > + > +Example: > + > +pm8941-wled@d800 { > + compatible = "qcom,pm8941-wled"; > + reg = <0xd800>; > + label = "backlight"; > + > + qcom,cs-out; > + qcom,current-limit = <20>; > + qcom,current-boost-limit = <805>; > + qcom,switching-freq = <1600>; > + qcom,ovp = <29>; > + qcom,num-strings = <2>; > +}; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block 2014-12-17 1:15 ` Stephen Boyd @ 2014-12-18 23:36 ` Bjorn 0 siblings, 0 replies; 11+ messages in thread From: Bjorn @ 2014-12-18 23:36 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie, Grant Likely, Cavin, Courtney, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-arm-msm@vger.kernel.org On Tue 16 Dec 17:15 PST 2014, Stephen Boyd wrote: > On 12/08/2014 04:22 PM, Bjorn Andersson wrote: > [..] > > +- qcom,current-limit: mA; per-string current limit; value from 0 to 25 > > + default: 20mA > > +- qcom,current-boost-limit: mA; boost current limit; one of: > > + 105, 385, 525, 805, 980, 1260, 1400, 1680 > > + default: 805mA [..] > > +- qcom,ovp: V; Over-voltage protection limit; one of: > > + 27, 29, 32, 35 > > + default: 29V > > It would be nice if the units were part of the property name, i.e. > qcom,ovp-V, qcom,current-boost-limit-mA, etc. That way the units are > known without having to refer to the documentation. > That might be useful, on the other hand one probably need to look inte bindings docs to figure out valid values anyways. > > +- qcom,num-strings: #; number of led strings attached; value from 1 to 3 > > + default: 2 > > I wonder if qcom,#strings is more appropriate? > Checking the other docs indicate that it's far more common to use num- than # here, but I haven't found anything written about it - it's more common to name this just "qcom,strings" than "qcom,#strings" at least... Regards, Bjorn ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-12-19 8:15 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-09 0:22 [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bjorn Andersson 2014-12-09 0:22 ` [PATCH 2/2] leds: add Qualcomm PM8941 WLED driver Bjorn Andersson 2014-12-17 0:54 ` Bryan Wu 2014-12-18 23:43 ` Bjorn 2014-12-19 8:15 ` Ivan T. Ivanov 2014-12-17 1:15 ` Stephen Boyd 2014-12-18 23:57 ` Bjorn 2014-12-10 1:19 ` [PATCH 1/2] leds: add DT binding for Qualcomm PM8941 WLED block Bryan Wu 2014-12-17 0:54 ` Bryan Wu 2014-12-17 1:15 ` Stephen Boyd 2014-12-18 23:36 ` Bjorn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).