2019-06-05 12:59:09

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v6 0/5] LM36274 Introduction

Hello

The v5 patchset missed adding in the new validation code.
Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
So this will reduce the patchset by 1.

Sorry for the extra noise on the patchsets. The change was lost when I converted
the patches from the mainline branch to the LED branch.

This change was made on top of the branch

repo: https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
branch: ti-lmu-led-drivers


Dan Murphy (5):
dt-bindings: mfd: Add lm36274 bindings to ti-lmu
mfd: ti-lmu: Add LM36274 support to the ti-lmu
regulator: lm363x: Add support for LM36274
dt-bindings: leds: Add LED bindings for the LM36274
leds: lm36274: Introduce the TI LM36274 LED driver

.../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++
.../devicetree/bindings/mfd/ti-lmu.txt | 54 ++++++
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++
drivers/mfd/Kconfig | 5 +-
drivers/mfd/ti-lmu.c | 14 ++
drivers/regulator/Kconfig | 2 +-
drivers/regulator/lm363x-regulator.c | 78 +++++++-
include/linux/mfd/ti-lmu-register.h | 23 +++
include/linux/mfd/ti-lmu.h | 4 +
11 files changed, 437 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
create mode 100644 drivers/leds/leds-lm36274.c

--
2.21.0.5.gaeb582a983


2019-06-05 12:59:22

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v6 4/5] dt-bindings: leds: Add LED bindings for the LM36274

Add the LM36274 LED specific bindings.

Signed-off-by: Dan Murphy <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
.../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm36274.txt b/Documentation/devicetree/bindings/leds/leds-lm36274.txt
new file mode 100644
index 000000000000..329393700191
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm36274.txt
@@ -0,0 +1,82 @@
+* Texas Instruments LM36274 4-Channel LCD Backlight Driver w/Integrated Bias
+
+The LM36274 is an integrated four-channel WLED driver and LCD bias supply.
+The backlight boost provides the power to bias four parallel LED strings with
+up to 29V total output voltage. The 11-bit LED current is programmable via
+the I2C bus and/or controlled via a logic level PWM input from 60 μA to 30 mA.
+
+Parent device properties are documented in ../mfd/ti_lmu.txt
+Regulator properties are documented in ../regulator/lm363x-regulator.txt
+
+Required backlight properties:
+ - compatible:
+ "ti,lm36274-backlight"
+ - reg : 0
+ - #address-cells : 1
+ - #size-cells : 0
+ - led-sources : Indicates which LED strings will be enabled.
+ Values from 0-3, sources is 0 based so strings will be
+ source value + 1.
+
+Optional backlight properties:
+ - label : see Documentation/devicetree/bindings/leds/common.txt
+ - linux,default-trigger :
+ see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+HVLED string 1 and 3 are controlled by control bank A and HVLED 2 string is
+controlled by control bank B.
+
+lm36274@11 {
+ compatible = "ti,lm36274";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x11>;
+
+ enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+
+ regulators {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lm363x-regulator";
+
+ enable-gpios = <&pioC 0 GPIO_ACTIVE_HIGH>,
+ <&pioC 1 GPIO_ACTIVE_HIGH>;
+
+ vboost {
+ regulator-name = "lcd_boost";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <7150000>;
+ regulator-always-on;
+ };
+
+ vpos {
+ regulator-name = "lcd_vpos";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6500000>;
+ };
+
+ vneg {
+ regulator-name = "lcd_vneg";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6500000>;
+ };
+ };
+
+ backlight {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lm36274-backlight";
+
+ led@0 {
+ reg = <0>;
+ led-sources = <0 2>;
+ label = "white:backlight_cluster";
+ linux,default-trigger = "backlight";
+ };
+ };
+};
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lm36274.pdf
--
2.21.0.5.gaeb582a983

2019-06-05 12:59:47

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v6 5/5] leds: lm36274: Introduce the TI LM36274 LED driver

Introduce the LM36274 LED driver. This driver uses the ti-lmu
MFD driver to probe this LED driver. The driver configures only the
LED registers and enables the outputs according to the config file.

The driver utilizes the TI LMU (Lighting Management Unit) LED common
framework to set the brightness bits.

Signed-off-by: Dan Murphy <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/Kconfig | 8 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++++++++++++++++++++
3 files changed, 183 insertions(+)
create mode 100644 drivers/leds/leds-lm36274.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 91cb047059a0..61c585049b2d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -798,6 +798,14 @@ config LEDS_LM3697
Say Y to enable the LM3697 LED driver for TI LMU devices.
This supports the LED device LM3697.

+config LEDS_LM36274
+ tristate "LED driver for LM36274"
+ depends on LEDS_TI_LMU_COMMON
+ depends on MFD_TI_LMU
+ help
+ Say Y to enable the LM36274 LED driver for TI LMU devices.
+ This supports the LED device LM36274.
+
comment "LED Triggers"
source "drivers/leds/trigger/Kconfig"

diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6c3350404ede..c52934732c1a 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
obj-$(CONFIG_LEDS_TI_LMU_COMMON) += leds-ti-lmu-common.o
obj-$(CONFIG_LEDS_LM3697) += leds-lm3697.o
+obj-$(CONFIG_LEDS_LM36274) += leds-lm36274.o

# LED SPI Drivers
obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
new file mode 100644
index 000000000000..b47786d36d21
--- /dev/null
+++ b/drivers/leds/leds-lm36274.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LM36274 LED chip family driver
+// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/leds-ti-lmu-common.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+
+#include <uapi/linux/uleds.h>
+
+#define LM36274_MAX_STRINGS 4
+#define LM36274_BL_EN BIT(4)
+
+/**
+ * struct lm36274
+ * @pdev: platform device
+ * @led_dev: led class device
+ * @lmu_data: Register and setting values for common code
+ * @regmap: Devices register map
+ * @dev: Pointer to the devices device struct
+ * @led_sources - The LED strings supported in this array
+ * @num_leds - Number of LED strings are supported in this array
+ */
+struct lm36274 {
+ struct platform_device *pdev;
+ struct led_classdev led_dev;
+ struct ti_lmu_bank lmu_data;
+ struct regmap *regmap;
+ struct device *dev;
+
+ u32 led_sources[LM36274_MAX_STRINGS];
+ int num_leds;
+};
+
+static int lm36274_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brt_val)
+{
+ struct lm36274 *led = container_of(led_cdev, struct lm36274, led_dev);
+
+ return ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+}
+
+static int lm36274_init(struct lm36274 *lm36274_data)
+{
+ int enable_val = 0;
+ int i;
+
+ for (i = 0; i < lm36274_data->num_leds; i++)
+ enable_val |= (1 << lm36274_data->led_sources[i]);
+
+ if (!enable_val) {
+ dev_err(lm36274_data->dev, "No LEDs were enabled\n");
+ return -EINVAL;
+ }
+
+ enable_val |= LM36274_BL_EN;
+
+ return regmap_write(lm36274_data->regmap, LM36274_REG_BL_EN,
+ enable_val);
+}
+
+static int lm36274_parse_dt(struct lm36274 *lm36274_data)
+{
+ struct fwnode_handle *child = NULL;
+ char label[LED_MAX_NAME_SIZE];
+ struct device *dev = &lm36274_data->pdev->dev;
+ const char *name;
+ int child_cnt;
+ int ret = -EINVAL;
+
+ /* There should only be 1 node */
+ child_cnt = device_get_child_node_count(dev);
+ if (child_cnt != 1)
+ return ret;
+
+ device_for_each_child_node(dev, child) {
+ ret = fwnode_property_read_string(child, "label", &name);
+ if (ret)
+ snprintf(label, sizeof(label),
+ "%s::", lm36274_data->pdev->name);
+ else
+ snprintf(label, sizeof(label),
+ "%s:%s", lm36274_data->pdev->name, name);
+
+ lm36274_data->num_leds = fwnode_property_read_u32_array(child,
+ "led-sources",
+ NULL, 0);
+ if (lm36274_data->num_leds <= 0)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u32_array(child, "led-sources",
+ lm36274_data->led_sources,
+ lm36274_data->num_leds);
+ if (ret) {
+ dev_err(dev, "led-sources property missing\n");
+ return -EINVAL;
+ }
+
+ fwnode_property_read_string(child, "linux,default-trigger",
+ &lm36274_data->led_dev.default_trigger);
+
+ }
+
+ lm36274_data->lmu_data.regmap = lm36274_data->regmap;
+ lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+ lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+ lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+
+ lm36274_data->led_dev.name = label;
+ lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+ lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
+
+ return ret;
+}
+
+static int lm36274_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct lm36274 *lm36274_data;
+ int ret;
+
+ lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),
+ GFP_KERNEL);
+ if (!lm36274_data) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ lm36274_data->pdev = pdev;
+ lm36274_data->dev = lmu->dev;
+ lm36274_data->regmap = lmu->regmap;
+ dev_set_drvdata(&pdev->dev, lm36274_data);
+
+ ret = lm36274_parse_dt(lm36274_data);
+ if (ret) {
+ dev_err(lm36274_data->dev, "Failed to parse DT node\n");
+ return ret;
+ }
+
+ ret = lm36274_init(lm36274_data);
+ if (ret) {
+ dev_err(lm36274_data->dev, "Failed to init the device\n");
+ return ret;
+ }
+
+ return devm_led_classdev_register(lm36274_data->dev,
+ &lm36274_data->led_dev);
+}
+
+static const struct of_device_id of_lm36274_leds_match[] = {
+ { .compatible = "ti,lm36274-backlight", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_lm36274_leds_match);
+
+static struct platform_driver lm36274_driver = {
+ .probe = lm36274_probe,
+ .driver = {
+ .name = "lm36274-leds",
+ },
+};
+module_platform_driver(lm36274_driver)
+
+MODULE_DESCRIPTION("Texas Instruments LM36274 LED driver");
+MODULE_AUTHOR("Dan Murphy <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.21.0.5.gaeb582a983

2019-06-05 12:59:54

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v6 2/5] mfd: ti-lmu: Add LM36274 support to the ti-lmu

Add the LM36274 register support to the ti-lmu MFD driver.

Signed-off-by: Dan Murphy <[email protected]>
Acked-by: Lee Jones <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/mfd/Kconfig | 5 ++---
drivers/mfd/ti-lmu.c | 14 ++++++++++++++
include/linux/mfd/ti-lmu-register.h | 23 +++++++++++++++++++++++
include/linux/mfd/ti-lmu.h | 4 ++++
4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8933485b28e7..a69aca3c2dab 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1335,9 +1335,8 @@ config MFD_TI_LMU
select REGMAP_I2C
help
Say yes here to enable support for TI LMU chips.
-
- TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, and LM3695.
- It consists of backlight, LED and regulator driver.
+ TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, LM3695 and
+ LM36274. It consists of backlight, LED and regulator driver.
It provides consistent device controls for lighting functions.

config MFD_OMAP_USB_HOST
diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index 89b1c5b584af..691ab9dd6236 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -111,6 +111,17 @@ static const struct mfd_cell lm3695_devices[] = {
},
};

+static const struct mfd_cell lm36274_devices[] = {
+ LM363X_REGULATOR(LM36274_BOOST),
+ LM363X_REGULATOR(LM36274_LDO_POS),
+ LM363X_REGULATOR(LM36274_LDO_NEG),
+ {
+ .name = "lm36274-leds",
+ .id = LM36274,
+ .of_compatible = "ti,lm36274-backlight",
+ },
+};
+
#define TI_LMU_DATA(chip, max_reg) \
static const struct ti_lmu_data chip##_data = \
{ \
@@ -123,6 +134,7 @@ TI_LMU_DATA(lm3631, LM3631_MAX_REG);
TI_LMU_DATA(lm3632, LM3632_MAX_REG);
TI_LMU_DATA(lm3633, LM3633_MAX_REG);
TI_LMU_DATA(lm3695, LM3695_MAX_REG);
+TI_LMU_DATA(lm36274, LM36274_MAX_REG);

static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
@@ -191,6 +203,7 @@ static const struct of_device_id ti_lmu_of_match[] = {
{ .compatible = "ti,lm3632", .data = &lm3632_data },
{ .compatible = "ti,lm3633", .data = &lm3633_data },
{ .compatible = "ti,lm3695", .data = &lm3695_data },
+ { .compatible = "ti,lm36274", .data = &lm36274_data },
{ }
};
MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
@@ -200,6 +213,7 @@ static const struct i2c_device_id ti_lmu_ids[] = {
{ "lm3632", LM3632 },
{ "lm3633", LM3633 },
{ "lm3695", LM3695 },
+ { "lm36274", LM36274 },
{ }
};
MODULE_DEVICE_TABLE(i2c, ti_lmu_ids);
diff --git a/include/linux/mfd/ti-lmu-register.h b/include/linux/mfd/ti-lmu-register.h
index 76998b01764b..076d8dea38fd 100644
--- a/include/linux/mfd/ti-lmu-register.h
+++ b/include/linux/mfd/ti-lmu-register.h
@@ -189,4 +189,27 @@
#define LM3695_REG_BRT_MSB 0x14

#define LM3695_MAX_REG 0x14
+
+/* LM36274 */
+#define LM36274_REG_REV 0x01
+#define LM36274_REG_BL_CFG_1 0x02
+#define LM36274_REG_BL_CFG_2 0x03
+#define LM36274_REG_BRT_LSB 0x04
+#define LM36274_REG_BRT_MSB 0x05
+#define LM36274_REG_BL_EN 0x08
+
+#define LM36274_REG_BIAS_CONFIG_1 0x09
+#define LM36274_EXT_EN_MASK BIT(0)
+#define LM36274_EN_VNEG_MASK BIT(1)
+#define LM36274_EN_VPOS_MASK BIT(2)
+
+#define LM36274_REG_BIAS_CONFIG_2 0x0a
+#define LM36274_REG_BIAS_CONFIG_3 0x0b
+#define LM36274_REG_VOUT_BOOST 0x0c
+#define LM36274_REG_VOUT_POS 0x0d
+#define LM36274_REG_VOUT_NEG 0x0e
+#define LM36274_VOUT_MASK 0x3F
+
+#define LM36274_MAX_REG 0x13
+
#endif
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 54e9d272e81c..0957598c7d41 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -26,6 +26,7 @@ enum ti_lmu_id {
LM3632,
LM3633,
LM3695,
+ LM36274,
LMU_MAX_ID,
};

@@ -67,6 +68,9 @@ enum lm363x_regulator_id {
LM3632_BOOST, /* Boost output */
LM3632_LDO_POS, /* Positive display bias output */
LM3632_LDO_NEG, /* Negative display bias output */
+ LM36274_BOOST, /* Boost output */
+ LM36274_LDO_POS, /* Positive display bias output */
+ LM36274_LDO_NEG, /* Negative display bias output */
};

/**
--
2.21.0.5.gaeb582a983

2019-06-05 19:34:05

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] LM36274 Introduction

Hi Dan,

Thank you for the v6.

Patches 4/5 and 5/5 don't contain amendments I made to
the respective patches on the ib-leds-mfd-regulator branch
(that address issues raised by Pavel), so I just kept those
unchanged. Besides that I updated the remaining ones.

Please check the ib-leds-mfd-regulator branch. I'll create a pull
request once I get a confirmation from you saying that everything
is as expected.

Best regards,
Jacek Anaszewski

On 6/5/19 2:56 PM, Dan Murphy wrote:
> Hello
>
> The v5 patchset missed adding in the new validation code.
> Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
> So this will reduce the patchset by 1.
>
> Sorry for the extra noise on the patchsets. The change was lost when I converted
> the patches from the mainline branch to the LED branch.
>
> This change was made on top of the branch
>
> repo: https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
> branch: ti-lmu-led-drivers
>
>
> Dan Murphy (5):
> dt-bindings: mfd: Add lm36274 bindings to ti-lmu
> mfd: ti-lmu: Add LM36274 support to the ti-lmu
> regulator: lm363x: Add support for LM36274
> dt-bindings: leds: Add LED bindings for the LM36274
> leds: lm36274: Introduce the TI LM36274 LED driver
>
> .../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++
> .../devicetree/bindings/mfd/ti-lmu.txt | 54 ++++++
> drivers/leds/Kconfig | 8 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++
> drivers/mfd/Kconfig | 5 +-
> drivers/mfd/ti-lmu.c | 14 ++
> drivers/regulator/Kconfig | 2 +-
> drivers/regulator/lm363x-regulator.c | 78 +++++++-
> include/linux/mfd/ti-lmu-register.h | 23 +++
> include/linux/mfd/ti-lmu.h | 4 +
> 11 files changed, 437 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
> create mode 100644 drivers/leds/leds-lm36274.c
>

2019-06-06 09:54:21

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v6 2/5] mfd: ti-lmu: Add LM36274 support to the ti-lmu

On Wed 2019-06-05 07:56:31, Dan Murphy wrote:
> Add the LM36274 register support to the ti-lmu MFD driver.
>
> Signed-off-by: Dan Murphy <[email protected]>
> Acked-by: Lee Jones <[email protected]>
> Signed-off-by: Jacek Anaszewski <[email protected]>

Acked-by: Pavel Machek <[email protected]>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (451.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-06 10:05:08

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v6 4/5] dt-bindings: leds: Add LED bindings for the LM36274

Hi!

> .../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm36274.txt b/Documentation/devicetree/bindings/leds/leds-lm36274.txt
> new file mode 100644
> index 000000000000..329393700191
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm36274.txt
> @@ -0,0 +1,82 @@
> +* Texas Instruments LM36274 4-Channel LCD Backlight Driver w/Integrated Bias
> +
> +The LM36274 is an integrated four-channel WLED driver and LCD bias supply.
> +The backlight boost provides the power to bias four parallel LED strings with
> +up to 29V total output voltage. The 11-bit LED current is programmable via
> +the I2C bus and/or controlled via a logic level PWM input from 60 μA to 30 mA.
> +
> +Parent device properties are documented in ../mfd/ti_lmu.txt
> +Regulator properties are documented in ../regulator/lm363x-regulator.txt

Should these paths follow the same format as below
(Documentation/devicetree/bindings)

?

Otherwise looks good.

Acked-by: Pavel Machek <[email protected]>
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.34 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-06 10:09:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] leds: lm36274: Introduce the TI LM36274 LED driver

On Wed 2019-06-05 07:56:34, Dan Murphy wrote:
> Introduce the LM36274 LED driver. This driver uses the ti-lmu
> MFD driver to probe this LED driver. The driver configures only the
> LED registers and enables the outputs according to the config file.
>
> The driver utilizes the TI LMU (Lighting Management Unit) LED common
> framework to set the brightness bits.
>
> Signed-off-by: Dan Murphy <[email protected]>
> Signed-off-by: Jacek Anaszewski <[email protected]>

Actually, one more thing. Not too important, but.. how did Jacek's
signoff get there? He should add his signoff when he applies it to the
git...

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (778.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-06 12:26:15

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] leds: lm36274: Introduce the TI LM36274 LED driver

Pavel

On 6/6/19 5:07 AM, Pavel Machek wrote:
> On Wed 2019-06-05 07:56:34, Dan Murphy wrote:
>> Introduce the LM36274 LED driver. This driver uses the ti-lmu
>> MFD driver to probe this LED driver. The driver configures only the
>> LED registers and enables the outputs according to the config file.
>>
>> The driver utilizes the TI LMU (Lighting Management Unit) LED common
>> framework to set the brightness bits.
>>
>> Signed-off-by: Dan Murphy <[email protected]>
>> Signed-off-by: Jacek Anaszewski <[email protected]>
> Actually, one more thing. Not too important, but.. how did Jacek's
> signoff get there? He should add his signoff when he applies it to the
> git...
>
This signoff came from the ti-lmu-led-drivers branch that Jacek created
in his linux-leds repo.

Unfortunately that branch does not seem to exist now so Jacek would need
to confirm this.

Dan

2019-06-06 16:25:59

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] leds: lm36274: Introduce the TI LM36274 LED driver

Hi!

> Introduce the LM36274 LED driver. This driver uses the ti-lmu
> MFD driver to probe this LED driver. The driver configures only the
> LED registers and enables the outputs according to the config file.
>
> The driver utilizes the TI LMU (Lighting Management Unit) LED common
> framework to set the brightness bits.

Nothing too bad, but...

> +static int lm36274_parse_dt(struct lm36274 *lm36274_data)
> +{
> + struct fwnode_handle *child = NULL;
> + char label[LED_MAX_NAME_SIZE];
> + struct device *dev = &lm36274_data->pdev->dev;
> + const char *name;
> + int child_cnt;
> + int ret = -EINVAL;
> +
> + /* There should only be 1 node */
> + child_cnt = device_get_child_node_count(dev);
> + if (child_cnt != 1)
> + return ret;

I'd do direct "return -EINVAL" here.

> + device_for_each_child_node(dev, child) {
> + ret = fwnode_property_read_string(child, "label", &name);
> + if (ret)
> + snprintf(label, sizeof(label),
> + "%s::", lm36274_data->pdev->name);
> + else
> + snprintf(label, sizeof(label),
> + "%s:%s", lm36274_data->pdev->name, name);
> +
> + lm36274_data->num_leds = fwnode_property_read_u32_array(child,
> + "led-sources",
> + NULL, 0);
> + if (lm36274_data->num_leds <= 0)
> + return -ENODEV;
> +
> + ret = fwnode_property_read_u32_array(child, "led-sources",
> + lm36274_data->led_sources,
> + lm36274_data->num_leds);
> + if (ret) {
> + dev_err(dev, "led-sources property missing\n");
> + return -EINVAL;

Should it return ret here? If read array failed with -ENOMEM, we may
want to propagate that.

> + }
> +
> + fwnode_property_read_string(child, "linux,default-trigger",
> + &lm36274_data->led_dev.default_trigger);
> +
> + }
> +
> + lm36274_data->lmu_data.regmap = lm36274_data->regmap;
> + lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
> + lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
> + lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
> +
> + lm36274_data->led_dev.name = label;
> + lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
> + lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
> +
> + return ret;

I'd do "return 0" here. It is success. Yes, ret will always have that
value at this moment, but...

> +static int lm36274_probe(struct platform_device *pdev)
> +{
> + struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
> + struct lm36274 *lm36274_data;
> + int ret;
> +
> + lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),
> + GFP_KERNEL);
> + if (!lm36274_data) {
> + ret = -ENOMEM;
> + return ret;
> + }

Just do return -ENOMEM;

Acked-by: Pavel Machek <[email protected]>
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (2.87 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-11 12:27:18

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] LM36274 Introduction

Jacek

Reviewed and tested the updated branch.  Looks good to me.

Dan

On 6/5/19 2:31 PM, Jacek Anaszewski wrote:
> Hi Dan,
>
> Thank you for the v6.
>
> Patches 4/5 and 5/5 don't contain amendments I made to
> the respective patches on the ib-leds-mfd-regulator branch
> (that address issues raised by Pavel), so I just kept those
> unchanged. Besides that I updated the remaining ones.
>
> Please check the ib-leds-mfd-regulator branch. I'll create a pull
> request once I get a confirmation from you saying that everything
> is as expected.
>
> Best regards,
> Jacek Anaszewski
>
> On 6/5/19 2:56 PM, Dan Murphy wrote:
>> Hello
>>
>> The v5 patchset missed adding in the new validation code.
>> Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
>> So this will reduce the patchset by 1.
>>
>> Sorry for the extra noise on the patchsets.  The change was lost when
>> I converted
>> the patches from the mainline branch to the LED branch.
>>
>> This change was made on top of the branch
>>
>> repo:
>> https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
>> branch: ti-lmu-led-drivers
>>
>>
>> Dan Murphy (5):
>>    dt-bindings: mfd: Add lm36274 bindings to ti-lmu
>>    mfd: ti-lmu: Add LM36274 support to the ti-lmu
>>    regulator: lm363x: Add support for LM36274
>>    dt-bindings: leds: Add LED bindings for the LM36274
>>    leds: lm36274: Introduce the TI LM36274 LED driver
>>
>>   .../devicetree/bindings/leds/leds-lm36274.txt |  82 +++++++++
>>   .../devicetree/bindings/mfd/ti-lmu.txt        |  54 ++++++
>>   drivers/leds/Kconfig                          |   8 +
>>   drivers/leds/Makefile                         |   1 +
>>   drivers/leds/leds-lm36274.c                   | 174 ++++++++++++++++++
>>   drivers/mfd/Kconfig                           |   5 +-
>>   drivers/mfd/ti-lmu.c                          |  14 ++
>>   drivers/regulator/Kconfig                     |   2 +-
>>   drivers/regulator/lm363x-regulator.c          |  78 +++++++-
>>   include/linux/mfd/ti-lmu-register.h           |  23 +++
>>   include/linux/mfd/ti-lmu.h                    |   4 +
>>   11 files changed, 437 insertions(+), 8 deletions(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/leds/leds-lm36274.txt
>>   create mode 100644 drivers/leds/leds-lm36274.c
>>
>

2019-06-24 14:43:14

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] LM36274 Introduction

On Wed, 05 Jun 2019, Dan Murphy wrote:

> Hello
>
> The v5 patchset missed adding in the new validation code.
> Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
> So this will reduce the patchset by 1.
>
> Sorry for the extra noise on the patchsets. The change was lost when I converted
> the patches from the mainline branch to the LED branch.
>
> This change was made on top of the branch
>
> repo: https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
> branch: ti-lmu-led-drivers
>
>
> Dan Murphy (5):
> dt-bindings: mfd: Add lm36274 bindings to ti-lmu
> mfd: ti-lmu: Add LM36274 support to the ti-lmu
> regulator: lm363x: Add support for LM36274
> dt-bindings: leds: Add LED bindings for the LM36274
> leds: lm36274: Introduce the TI LM36274 LED driver
>
> .../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++
> .../devicetree/bindings/mfd/ti-lmu.txt | 54 ++++++
> drivers/leds/Kconfig | 8 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++
> drivers/mfd/Kconfig | 5 +-
> drivers/mfd/ti-lmu.c | 14 ++
> drivers/regulator/Kconfig | 2 +-
> drivers/regulator/lm363x-regulator.c | 78 +++++++-
> include/linux/mfd/ti-lmu-register.h | 23 +++
> include/linux/mfd/ti-lmu.h | 4 +
> 11 files changed, 437 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
> create mode 100644 drivers/leds/leds-lm36274.c

Can you finish of satisfying everyone's comments and re-send with all
the Acks you've collected so far? If you turn this around quickly,
you might still get into v5.3.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-06-24 20:38:46

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] LM36274 Introduction

Lee

On 6/24/19 9:42 AM, Lee Jones wrote:
> On Wed, 05 Jun 2019, Dan Murphy wrote:
>
>> Hello
>>
>> The v5 patchset missed adding in the new validation code.
>> Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
>> So this will reduce the patchset by 1.
>>
>> Sorry for the extra noise on the patchsets. The change was lost when I converted
>> the patches from the mainline branch to the LED branch.
>>
>> This change was made on top of the branch
>>
>> repo: https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
>> branch: ti-lmu-led-drivers
>>
>>
>> Dan Murphy (5):
>> dt-bindings: mfd: Add lm36274 bindings to ti-lmu
>> mfd: ti-lmu: Add LM36274 support to the ti-lmu
>> regulator: lm363x: Add support for LM36274
>> dt-bindings: leds: Add LED bindings for the LM36274
>> leds: lm36274: Introduce the TI LM36274 LED driver
>>
>> .../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++
>> .../devicetree/bindings/mfd/ti-lmu.txt | 54 ++++++
>> drivers/leds/Kconfig | 8 +
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++
>> drivers/mfd/Kconfig | 5 +-
>> drivers/mfd/ti-lmu.c | 14 ++
>> drivers/regulator/Kconfig | 2 +-
>> drivers/regulator/lm363x-regulator.c | 78 +++++++-
>> include/linux/mfd/ti-lmu-register.h | 23 +++
>> include/linux/mfd/ti-lmu.h | 4 +
>> 11 files changed, 437 insertions(+), 8 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
>> create mode 100644 drivers/leds/leds-lm36274.c
> Can you finish of satisfying everyone's comments and re-send with all
> the Acks you've collected so far? If you turn this around quickly,
> you might still get into v5.3.
>

The changes were made by Jacek and I reviewed and tested them

https://lkml.org/lkml/2019/6/11/455

2019-06-25 07:40:14

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] LM36274 Introduction

On Mon, 24 Jun 2019, Dan Murphy wrote:

> Lee
>
> On 6/24/19 9:42 AM, Lee Jones wrote:
> > On Wed, 05 Jun 2019, Dan Murphy wrote:
> >
> > > Hello
> > >
> > > The v5 patchset missed adding in the new validation code.
> > > Patch 1 of the v5 series was squashed into patch 4 of the v5 series.
> > > So this will reduce the patchset by 1.
> > >
> > > Sorry for the extra noise on the patchsets. The change was lost when I converted
> > > the patches from the mainline branch to the LED branch.
> > >
> > > This change was made on top of the branch
> > >
> > > repo: https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
> > > branch: ti-lmu-led-drivers
> > >
> > >
> > > Dan Murphy (5):
> > > dt-bindings: mfd: Add lm36274 bindings to ti-lmu
> > > mfd: ti-lmu: Add LM36274 support to the ti-lmu
> > > regulator: lm363x: Add support for LM36274
> > > dt-bindings: leds: Add LED bindings for the LM36274
> > > leds: lm36274: Introduce the TI LM36274 LED driver
> > >
> > > .../devicetree/bindings/leds/leds-lm36274.txt | 82 +++++++++
> > > .../devicetree/bindings/mfd/ti-lmu.txt | 54 ++++++
> > > drivers/leds/Kconfig | 8 +
> > > drivers/leds/Makefile | 1 +
> > > drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++
> > > drivers/mfd/Kconfig | 5 +-
> > > drivers/mfd/ti-lmu.c | 14 ++
> > > drivers/regulator/Kconfig | 2 +-
> > > drivers/regulator/lm363x-regulator.c | 78 +++++++-
> > > include/linux/mfd/ti-lmu-register.h | 23 +++
> > > include/linux/mfd/ti-lmu.h | 4 +
> > > 11 files changed, 437 insertions(+), 8 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm36274.txt
> > > create mode 100644 drivers/leds/leds-lm36274.c
> > Can you finish of satisfying everyone's comments and re-send with all
> > the Acks you've collected so far? If you turn this around quickly,
> > you might still get into v5.3.
> >
>
> The changes were made by Jacek and I reviewed and tested them
>
> https://lkml.org/lkml/2019/6/11/455

Ah, this was related to the recent GIT PULL craziness.

Thanks for letting me know.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog