2017-03-03 13:56:39

by Sean Wang

[permalink] [raw]
Subject: [PATCH v5 0/4] leds: add leds-mt6323 support on MT7623 SoC

From: Sean Wang <[email protected]>

MT7623 SoC uses MT6323 PMIC as the default power supply
which has LED function insides. The patchset introduces
the LED support for MT6323 with on, off and hardware
dimmed and blinked and it should work on other similar
SoCs if also using MT6323.

Changes since v1:
- fixed typo in the comments
- sorted include directives alphabetically
- applied all register definitions with MT6323 prefix
- removed the redundant structure declaration
- fixed coding style defined in kernel doc format consistently
- added error handling into all the occurrences where regmap APIs
are used
- removed loudly debug message
- made magic constant into meaningful macro
- added missing mutex_destroy when module removed called
- updated module license with GPL
- fixed sparse warnings

Changes since v2:
- fixed the typo in devicetree Documentation
- cleaned up the style and explanation in the comments
- applied all macro/struct/function definitions with MT6323 prefix
- put the logic about the setup from device tree into function for simplicity

Changes since v3:
- changed the variable current_brightness into the proper type
- removed the redundant member led_num from mt6323_leds structure
- collected the brightness setting into mt6323_led_hw_brightness() call
- added brightness_get() support into the core layer
- fixed zero duty cycle handling due to hardware doesn't support
- fixed duty cycle calculation mapping to the hardware register
- added the brightness as the input in mt6323_led_hw_on() call
- enhanced error handling in mt6323_led_set_dt_default() call
- fixed potential issue which caused accessing unallocated memory if excessive
the led controller nodes are defined
- fixed redundant memory allocation in mt6323_leds structure
- added reg property for defining the led controller for which the child
node is predestined

Changes since v4:
- enhanced the example in leds-mt6323.txt for patch 1/4 with adding the missing
#address-cell and #size-cell, correcting sub-node naming with led@0 style,
removing the unrelated setup for LED and moving the description for the node
properties into the right place
- enhanced the documentation in mt6397.txt for patch 2/4
- changed the subject prefixes into "dt-bindings: ..." for the device tree commits
- fixed that missing of_node_put() in the error path called from mt6323_led_probe()
- fixed that incorrect duty_cycle calculation mapping into the hardware register in
case duty ratio is lower than 1/64 rather than 0

Sean Wang (4):
dt-bindings: leds: Add document bindings for leds-mt6323
dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC
leds: Add LED support for MT6323 PMIC
mfd: mt6397: Add MT6323 LED support into MT6397 driver

.../devicetree/bindings/leds/leds-mt6323.txt | 63 +++
Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-mt6323.c | 502 +++++++++++++++++++++
drivers/mfd/mt6397-core.c | 4 +
6 files changed, 583 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6323.txt
create mode 100644 drivers/leds/leds-mt6323.c

--
1.9.1


2017-03-03 13:56:49

by Sean Wang

[permalink] [raw]
Subject: [PATCH v5 1/4] dt-bindings: leds: Add document bindings for leds-mt6323

From: Sean Wang <[email protected]>

This patch adds documentation for devicetree bindings
for LED support on MT6323 PMIC

Signed-off-by: Sean Wang <[email protected]>
---
.../devicetree/bindings/leds/leds-mt6323.txt | 63 ++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6323.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-mt6323.txt b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
new file mode 100644
index 0000000..135c441
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
@@ -0,0 +1,63 @@
+Device Tree Bindings for LED support on MT6323 PMIC
+
+MT6323 LED controller is subfunction provided by
+MT6323 PMIC, so the LED controllers are defined as
+the subnode of the function node provided by MT6323
+PMIC controller that is being defined as one kind of
+Muti-Function Device (MFD) using shared bus called
+PMIC wrapper for each subfunction to access remote
+MT6323 PMIC hardware.
+
+For MT6323 MFD bindings see:
+Documentation/devicetree/bindings/mfd/mt6397.txt
+For MediaTek PMIC wrapper bindings see:
+Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
+
+There's sub-node for the LED controller that describes
+the initial behavior for each LED physically and currently
+only four LED sub-nodes can be supported.
+
+Required properties:
+- compatible : Must be "mediatek,mt6323-led"
+- address-cells : Must be 1
+- size-cells : Must be 0
+
+LED sub-node properties:
+- reg : LED channel number (0..3)
+- label : (optional)
+ see Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger : (optional)
+ see Documentation/devicetree/bindings/leds/common.txt
+- default-state: (optional) The initial state of the LED
+ see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+ pmic: mt6323 {
+ compatible = "mediatek,mt6323";
+
+ ...
+
+ mt6323led: leds {
+ compatible = "mediatek,mt6323-led";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ label = "LED0";
+ linux,default-trigger = "timer";
+ default-state = "on";
+ };
+ led@1 {
+ reg = <1>;
+ label = "LED1";
+ default-state = "off";
+ };
+ led@2 {
+ reg = <2>;
+ label = "LED2";
+ default-state = "on";
+ };
+ };
+ };
--
1.9.1

2017-03-03 13:56:56

by Sean Wang

[permalink] [raw]
Subject: [PATCH v5 3/4] leds: Add LED support for MT6323 PMIC

From: Sean Wang <[email protected]>

MT6323 PMIC is a multi-function device that includes
LED function. It allows attaching up to 4 LEDs which
can either be on, off or dimmed and/or blinked with
the controller.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-mt6323.c | 502 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 511 insertions(+)
create mode 100644 drivers/leds/leds-mt6323.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index c621cbb..30095fc 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -117,6 +117,14 @@ config LEDS_MIKROTIK_RB532
This option enables support for the so called "User LED" of
Mikrotik's Routerboard 532.

+config LEDS_MT6323
+ tristate "LED Support for Mediatek MT6323 PMIC"
+ depends on LEDS_CLASS
+ depends on MFD_MT6397
+ help
+ This option enables support for on-chip LED drivers found on
+ Mediatek MT6323 PMIC.
+
config LEDS_S3C24XX
tristate "LED Support for Samsung S3C24XX GPIO LEDs"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6b82737..4feb332 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o
obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o
obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
+obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o

# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-mt6323.c b/drivers/leds/leds-mt6323.c
new file mode 100644
index 0000000..48081bc
--- /dev/null
+++ b/drivers/leds/leds-mt6323.c
@@ -0,0 +1,502 @@
+/*
+ * LED driver for Mediatek MT6323 PMIC
+ *
+ * Copyright (C) 2017 Sean Wang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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/kernel.h>
+#include <linux/leds.h>
+#include <linux/mfd/mt6323/registers.h>
+#include <linux/mfd/mt6397/core.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * Register field for MT6323_TOP_CKPDN0 to enable
+ * 32K clock common for LED device.
+ */
+#define MT6323_RG_DRV_32K_CK_PDN BIT(11)
+#define MT6323_RG_DRV_32K_CK_PDN_MASK BIT(11)
+
+/*
+ * Register field for MT6323_TOP_CKPDN2 to enable
+ * individual clock for LED device.
+ */
+#define MT6323_RG_ISINK_CK_PDN(i) BIT(i)
+#define MT6323_RG_ISINK_CK_PDN_MASK(i) BIT(i)
+
+/*
+ * Register field for MT6323_TOP_CKCON1 to select
+ * clock source.
+ */
+#define MT6323_RG_ISINK_CK_SEL_MASK(i) (BIT(10) << (i))
+
+/*
+ * Register for MT6323_ISINK_CON0 to setup the
+ * duty cycle of the blink.
+ */
+#define MT6323_ISINK_CON0(i) (MT6323_ISINK0_CON0 + 0x8 * (i))
+#define MT6323_ISINK_DIM_DUTY_MASK (0x1f << 8)
+#define MT6323_ISINK_DIM_DUTY(i) (((i) << 8) & \
+ MT6323_ISINK_DIM_DUTY_MASK)
+
+/* Register to setup the period of the blink. */
+#define MT6323_ISINK_CON1(i) (MT6323_ISINK0_CON1 + 0x8 * (i))
+#define MT6323_ISINK_DIM_FSEL_MASK (0xffff)
+#define MT6323_ISINK_DIM_FSEL(i) ((i) & MT6323_ISINK_DIM_FSEL_MASK)
+
+/* Register to control the brightness. */
+#define MT6323_ISINK_CON2(i) (MT6323_ISINK0_CON2 + 0x8 * (i))
+#define MT6323_ISINK_CH_STEP_SHIFT 12
+#define MT6323_ISINK_CH_STEP_MASK (0x7 << 12)
+#define MT6323_ISINK_CH_STEP(i) (((i) << 12) & \
+ MT6323_ISINK_CH_STEP_MASK)
+#define MT6323_ISINK_SFSTR0_TC_MASK (0x3 << 1)
+#define MT6323_ISINK_SFSTR0_TC(i) (((i) << 1) & \
+ MT6323_ISINK_SFSTR0_TC_MASK)
+#define MT6323_ISINK_SFSTR0_EN_MASK BIT(0)
+#define MT6323_ISINK_SFSTR0_EN BIT(0)
+
+/* Register to LED channel enablement. */
+#define MT6323_ISINK_CH_EN_MASK(i) BIT(i)
+#define MT6323_ISINK_CH_EN(i) BIT(i)
+
+#define MT6323_MAX_PERIOD 10000
+#define MT6323_MAX_LEDS 4
+#define MT6323_MAX_BRIGHTNESS 6
+#define MT6323_UNIT_DUTY 3125
+#define MT6323_CAL_HW_DUTY(o, p) DIV_ROUND_CLOSEST((o) * 100000ul,\
+ (p) * MT6323_UNIT_DUTY)
+
+struct mt6323_leds;
+
+/**
+ * struct mt6323_led - state container for the LED device
+ * @id: the identifier in MT6323 LED device
+ * @parent: the pointer to MT6323 LED controller
+ * @cdev: LED class device for this LED device
+ * @current_brightness: current state of the LED device
+ */
+struct mt6323_led {
+ int id;
+ struct mt6323_leds *parent;
+ struct led_classdev cdev;
+ enum led_brightness current_brightness;
+};
+
+/**
+ * struct mt6323_leds - state container for holding LED controller
+ * of the driver
+ * @dev: the device pointer
+ * @hw: the underlying hardware providing shared
+ * bus for the register operations
+ * @lock: the lock among process context
+ * @led: the array that contains the state of individual
+ * LED device
+ */
+struct mt6323_leds {
+ struct device *dev;
+ struct mt6397_chip *hw;
+ /* protect among process context */
+ struct mutex lock;
+ struct mt6323_led *led[MT6323_MAX_LEDS];
+};
+
+static int mt6323_led_hw_brightness(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ struct regmap *regmap = leds->hw->regmap;
+ u32 con2_mask = 0, con2_val = 0;
+ int ret;
+
+ /*
+ * Setup current output for the corresponding
+ * brightness level.
+ */
+ con2_mask |= MT6323_ISINK_CH_STEP_MASK |
+ MT6323_ISINK_SFSTR0_TC_MASK |
+ MT6323_ISINK_SFSTR0_EN_MASK;
+ con2_val |= MT6323_ISINK_CH_STEP(brightness - 1) |
+ MT6323_ISINK_SFSTR0_TC(2) |
+ MT6323_ISINK_SFSTR0_EN;
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_CON2(led->id),
+ con2_mask, con2_val);
+ return ret;
+}
+
+static int mt6323_led_hw_off(struct led_classdev *cdev)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ struct regmap *regmap = leds->hw->regmap;
+ unsigned int status;
+ int ret;
+
+ status = MT6323_ISINK_CH_EN(led->id);
+ ret = regmap_update_bits(regmap, MT6323_ISINK_EN_CTRL,
+ MT6323_ISINK_CH_EN_MASK(led->id), ~status);
+ if (ret < 0)
+ return ret;
+
+ usleep_range(100, 300);
+ ret = regmap_update_bits(regmap, MT6323_TOP_CKPDN2,
+ MT6323_RG_ISINK_CK_PDN_MASK(led->id),
+ MT6323_RG_ISINK_CK_PDN(led->id));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static enum led_brightness
+mt6323_get_led_hw_brightness(struct led_classdev *cdev)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ struct regmap *regmap = leds->hw->regmap;
+ unsigned int status;
+ int ret;
+
+ ret = regmap_read(regmap, MT6323_TOP_CKPDN2, &status);
+ if (ret < 0)
+ return ret;
+
+ if (status & MT6323_RG_ISINK_CK_PDN_MASK(led->id))
+ return 0;
+
+ ret = regmap_read(regmap, MT6323_ISINK_EN_CTRL, &status);
+ if (ret < 0)
+ return ret;
+
+ if (!(status & MT6323_ISINK_CH_EN(led->id)))
+ return 0;
+
+ ret = regmap_read(regmap, MT6323_ISINK_CON2(led->id), &status);
+ if (ret < 0)
+ return ret;
+
+ return ((status & MT6323_ISINK_CH_STEP_MASK)
+ >> MT6323_ISINK_CH_STEP_SHIFT) + 1;
+}
+
+static int mt6323_led_hw_on(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ struct regmap *regmap = leds->hw->regmap;
+ unsigned int status;
+ int ret;
+
+ /*
+ * Setup required clock source, enable the corresponding
+ * clock and channel and let work with continuous blink as
+ * the default.
+ */
+ ret = regmap_update_bits(regmap, MT6323_TOP_CKCON1,
+ MT6323_RG_ISINK_CK_SEL_MASK(led->id), 0);
+ if (ret < 0)
+ return ret;
+
+ status = MT6323_RG_ISINK_CK_PDN(led->id);
+ ret = regmap_update_bits(regmap, MT6323_TOP_CKPDN2,
+ MT6323_RG_ISINK_CK_PDN_MASK(led->id),
+ ~status);
+ if (ret < 0)
+ return ret;
+
+ usleep_range(100, 300);
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_EN_CTRL,
+ MT6323_ISINK_CH_EN_MASK(led->id),
+ MT6323_ISINK_CH_EN(led->id));
+ if (ret < 0)
+ return ret;
+
+ ret = mt6323_led_hw_brightness(cdev, brightness);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_CON0(led->id),
+ MT6323_ISINK_DIM_DUTY_MASK,
+ MT6323_ISINK_DIM_DUTY(31));
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_CON1(led->id),
+ MT6323_ISINK_DIM_FSEL_MASK,
+ MT6323_ISINK_DIM_FSEL(1000));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int mt6323_led_set_blink(struct led_classdev *cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ struct regmap *regmap = leds->hw->regmap;
+ unsigned long period;
+ u8 duty_hw;
+ int ret;
+
+ /*
+ * Units are in ms, if over the hardware able
+ * to support, fallback into software blink
+ */
+ period = *delay_on + *delay_off;
+
+ if (period > MT6323_MAX_PERIOD)
+ return -EINVAL;
+
+ /*
+ * LED subsystem requires a default user
+ * friendly blink pattern for the LED so using
+ * 1Hz duty cycle 50% here if without specific
+ * value delay_on and delay off being assigned.
+ */
+ if (!*delay_on && !*delay_off) {
+ *delay_on = 500;
+ *delay_off = 500;
+ }
+
+ /*
+ * Calculate duty_hw based on the percentage of period during
+ * which the led is ON.
+ */
+ duty_hw = MT6323_CAL_HW_DUTY(*delay_on, period);
+
+ /* hardware doesn't support zero duty cycle. */
+ if (!duty_hw)
+ return -EINVAL;
+
+ mutex_lock(&leds->lock);
+ /*
+ * Set max_brightness as the software blink behavior
+ * when no blink brightness.
+ */
+ if (!led->current_brightness) {
+ ret = mt6323_led_hw_on(cdev, cdev->max_brightness);
+ if (ret < 0)
+ goto out;
+ led->current_brightness = cdev->max_brightness;
+ }
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_CON0(led->id),
+ MT6323_ISINK_DIM_DUTY_MASK,
+ MT6323_ISINK_DIM_DUTY(duty_hw - 1));
+ if (ret < 0)
+ goto out;
+
+ ret = regmap_update_bits(regmap, MT6323_ISINK_CON1(led->id),
+ MT6323_ISINK_DIM_FSEL_MASK,
+ MT6323_ISINK_DIM_FSEL(period - 1));
+out:
+ mutex_unlock(&leds->lock);
+
+ return ret;
+}
+
+static int mt6323_led_set_brightness(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ struct mt6323_leds *leds = led->parent;
+ int ret;
+
+ mutex_lock(&leds->lock);
+
+ if (!led->current_brightness && brightness) {
+ ret = mt6323_led_hw_on(cdev, brightness);
+ if (ret < 0)
+ goto out;
+ } else if (brightness) {
+ ret = mt6323_led_hw_brightness(cdev, brightness);
+ if (ret < 0)
+ goto out;
+ } else {
+ ret = mt6323_led_hw_off(cdev);
+ if (ret < 0)
+ goto out;
+ }
+
+ led->current_brightness = brightness;
+out:
+ mutex_unlock(&leds->lock);
+
+ return ret;
+}
+
+static int mt6323_led_set_dt_default(struct led_classdev *cdev,
+ struct device_node *np)
+{
+ struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev);
+ const char *state;
+ int ret = 0;
+
+ led->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
+ led->cdev.default_trigger = of_get_property(np,
+ "linux,default-trigger",
+ NULL);
+
+ state = of_get_property(np, "default-state", NULL);
+ if (state) {
+ if (!strcmp(state, "keep")) {
+ ret = mt6323_get_led_hw_brightness(cdev);
+ if (ret < 0)
+ return ret;
+ led->current_brightness = ret;
+ ret = 0;
+ } else if (!strcmp(state, "on")) {
+ ret =
+ mt6323_led_set_brightness(cdev, cdev->max_brightness);
+ } else {
+ ret = mt6323_led_set_brightness(cdev, LED_OFF);
+ }
+ }
+
+ return ret;
+}
+
+static int mt6323_led_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *child;
+ struct mt6397_chip *hw = dev_get_drvdata(pdev->dev.parent);
+ struct mt6323_leds *leds;
+ struct mt6323_led *led;
+ int ret;
+ unsigned int status;
+ u32 reg;
+
+ leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL);
+ if (!leds)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, leds);
+ leds->dev = dev;
+
+ /*
+ * leds->hw points to the underlying bus for the register
+ * controlled.
+ */
+ leds->hw = hw;
+ mutex_init(&leds->lock);
+
+ status = MT6323_RG_DRV_32K_CK_PDN;
+ ret = regmap_update_bits(leds->hw->regmap, MT6323_TOP_CKPDN0,
+ MT6323_RG_DRV_32K_CK_PDN_MASK, ~status);
+ if (ret < 0) {
+ dev_err(leds->dev,
+ "Failed to update MT6323_TOP_CKPDN0 Register\n");
+ return ret;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ ret = of_property_read_u32(child, "reg", &reg);
+ if (ret) {
+ dev_err(dev, "Failed to read led 'reg' property\n");
+ goto put_child_node;
+ }
+
+ if (reg < 0 || reg > MT6323_MAX_LEDS || leds->led[reg]) {
+ dev_err(dev, "Invalid led reg %u\n", reg);
+ ret = -EINVAL;
+ goto put_child_node;
+ }
+
+ led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+ if (!led) {
+ ret = -ENOMEM;
+ goto put_child_node;
+ }
+
+ leds->led[reg] = led;
+ leds->led[reg]->id = reg;
+ leds->led[reg]->cdev.max_brightness = MT6323_MAX_BRIGHTNESS;
+ leds->led[reg]->cdev.brightness_set_blocking =
+ mt6323_led_set_brightness;
+ leds->led[reg]->cdev.blink_set = mt6323_led_set_blink;
+ leds->led[reg]->cdev.brightness_get =
+ mt6323_get_led_hw_brightness;
+ leds->led[reg]->parent = leds;
+
+ ret = mt6323_led_set_dt_default(&leds->led[reg]->cdev, child);
+ if (ret < 0) {
+ dev_err(leds->dev,
+ "Failed to LED set default from devicetree\n");
+ goto put_child_node;
+ }
+
+ ret = devm_led_classdev_register(dev, &leds->led[reg]->cdev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register LED: %d\n",
+ ret);
+ goto put_child_node;
+ }
+ leds->led[reg]->cdev.dev->of_node = child;
+ }
+
+ return 0;
+
+put_child_node:
+ of_node_put(child);
+ return ret;
+}
+
+static int mt6323_led_remove(struct platform_device *pdev)
+{
+ struct mt6323_leds *leds = platform_get_drvdata(pdev);
+ int i;
+
+ /* Turn the LEDs off on driver removal. */
+ for (i = 0 ; leds->led[i] ; i++)
+ mt6323_led_hw_off(&leds->led[i]->cdev);
+
+ regmap_update_bits(leds->hw->regmap, MT6323_TOP_CKPDN0,
+ MT6323_RG_DRV_32K_CK_PDN_MASK,
+ MT6323_RG_DRV_32K_CK_PDN);
+
+ mutex_destroy(&leds->lock);
+
+ return 0;
+}
+
+static const struct of_device_id mt6323_led_dt_match[] = {
+ { .compatible = "mediatek,mt6323-led" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6323_led_dt_match);
+
+static struct platform_driver mt6323_led_driver = {
+ .probe = mt6323_led_probe,
+ .remove = mt6323_led_remove,
+ .driver = {
+ .name = "mt6323-led",
+ .of_match_table = mt6323_led_dt_match,
+ },
+};
+
+module_platform_driver(mt6323_led_driver);
+
+MODULE_DESCRIPTION("LED driver for Mediatek MT6323 PMIC");
+MODULE_AUTHOR("Sean Wang <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.9.1

2017-03-03 13:56:50

by Sean Wang

[permalink] [raw]
Subject: [PATCH v5 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver

From: Sean Wang <[email protected]>

Add compatible string as "mt6323-led" that will make
the OF core spawn child devices for the LED subnode
of that MT6323 MFD device.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/mfd/mt6397-core.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index e14d8b0..8e601c8 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -48,6 +48,10 @@
.name = "mt6323-regulator",
.of_compatible = "mediatek,mt6323-regulator"
},
+ {
+ .name = "mt6323-led",
+ .of_compatible = "mediatek,mt6323-led"
+ },
};

static const struct mfd_cell mt6397_devs[] = {
--
1.9.1

2017-03-03 13:57:23

by Sean Wang

[permalink] [raw]
Subject: [PATCH v5 2/4] dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC

From: Sean Wang <[email protected]>

This patch adds documentation for devicetree bindings
for LED support as the subnode of MT6323 PMIC

Signed-off-by: Sean Wang <[email protected]>
---
Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index 949c85f..522a3bb 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -6,6 +6,7 @@ MT6397/MT6323 is a multifunction device with the following sub modules:
- Audio codec
- GPIO
- Clock
+- LED

It is interfaced to host controller using SPI interface by a proprietary hardware
called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap.
@@ -34,6 +35,10 @@ Optional subnodes:
- clk
Required properties:
- compatible: "mediatek,mt6397-clk"
+- led
+ Required properties:
+ - compatible: "mediatek,mt6323-led"
+ see Documentation/devicetree/bindings/leds/leds-mt6323.txt

Example:
pwrap: pwrap@1000f000 {
--
1.9.1

2017-03-12 14:04:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 1/4] dt-bindings: leds: Add document bindings for leds-mt6323

On Fri, Mar 03, 2017 at 09:56:22PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> This patch adds documentation for devicetree bindings
> for LED support on MT6323 PMIC
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../devicetree/bindings/leds/leds-mt6323.txt | 63 ++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6323.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-mt6323.txt b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
> new file mode 100644
> index 0000000..135c441
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
> @@ -0,0 +1,63 @@
> +Device Tree Bindings for LED support on MT6323 PMIC
> +
> +MT6323 LED controller is subfunction provided by
> +MT6323 PMIC, so the LED controllers are defined as
> +the subnode of the function node provided by MT6323
> +PMIC controller that is being defined as one kind of
> +Muti-Function Device (MFD) using shared bus called
> +PMIC wrapper for each subfunction to access remote
> +MT6323 PMIC hardware.

Your line wrap is kind of short. Fix you line wrap to 72-80 char.

> +
> +For MT6323 MFD bindings see:
> +Documentation/devicetree/bindings/mfd/mt6397.txt
> +For MediaTek PMIC wrapper bindings see:
> +Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> +
> +There's sub-node for the LED controller that describes
> +the initial behavior for each LED physically and currently
> +only four LED sub-nodes can be supported.
> +
> +Required properties:
> +- compatible : Must be "mediatek,mt6323-led"
> +- address-cells : Must be 1
> +- size-cells : Must be 0
> +
> +LED sub-node properties:
> +- reg : LED channel number (0..3)
> +- label : (optional)
> + see Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger : (optional)
> + see Documentation/devicetree/bindings/leds/common.txt
> +- default-state: (optional) The initial state of the LED
> + see Documentation/devicetree/bindings/leds/common.txt

Group these by required and optional. You can drop "The initial state of
the LED".

> +
> +Example:
> +
> + pmic: mt6323 {

mt6323: pmic {

> + compatible = "mediatek,mt6323";
> +
> + ...
> +
> + mt6323led: leds {
> + compatible = "mediatek,mt6323-led";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + reg = <0>;
> + label = "LED0";
> + linux,default-trigger = "timer";
> + default-state = "on";
> + };
> + led@1 {
> + reg = <1>;
> + label = "LED1";
> + default-state = "off";
> + };
> + led@2 {
> + reg = <2>;
> + label = "LED2";
> + default-state = "on";
> + };
> + };
> + };
> --
> 1.9.1
>

2017-03-12 14:05:56

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC

On Fri, Mar 03, 2017 at 09:56:23PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> This patch adds documentation for devicetree bindings
> for LED support as the subnode of MT6323 PMIC
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
> 1 file changed, 5 insertions(+)

Acked-by: Rob Herring <[email protected]>

2017-03-13 12:00:48

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC



On 03/03/17 14:56, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> This patch adds documentation for devicetree bindings
> for LED support as the subnode of MT6323 PMIC
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
> index 949c85f..522a3bb 100644
> --- a/Documentation/devicetree/bindings/mfd/mt6397.txt
> +++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
> @@ -6,6 +6,7 @@ MT6397/MT6323 is a multifunction device with the following sub modules:
> - Audio codec
> - GPIO
> - Clock
> +- LED
>
> It is interfaced to host controller using SPI interface by a proprietary hardware
> called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap.
> @@ -34,6 +35,10 @@ Optional subnodes:
> - clk
> Required properties:
> - compatible: "mediatek,mt6397-clk"
> +- led
> + Required properties:
> + - compatible: "mediatek,mt6323-led"
> + see Documentation/devicetree/bindings/leds/leds-mt6323.txt

Part of this patch is already part of linux-next:
05daab3 ("Documentation: devicetree: Add LED subnode binding for MT6323
PMIC")

Regards,
Matthias

2017-03-13 18:36:40

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC

Hi Matthias,

On 03/13/2017 01:00 PM, Matthias Brugger wrote:
>
>
> On 03/03/17 14:56, [email protected] wrote:
>> From: Sean Wang <[email protected]>
>>
>> This patch adds documentation for devicetree bindings
>> for LED support as the subnode of MT6323 PMIC
>>
>> Signed-off-by: Sean Wang <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt
>> b/Documentation/devicetree/bindings/mfd/mt6397.txt
>> index 949c85f..522a3bb 100644
>> --- a/Documentation/devicetree/bindings/mfd/mt6397.txt
>> +++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
>> @@ -6,6 +6,7 @@ MT6397/MT6323 is a multifunction device with the
>> following sub modules:
>> - Audio codec
>> - GPIO
>> - Clock
>> +- LED
>>
>> It is interfaced to host controller using SPI interface by a
>> proprietary hardware
>> called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of
>> pwrap.
>> @@ -34,6 +35,10 @@ Optional subnodes:
>> - clk
>> Required properties:
>> - compatible: "mediatek,mt6397-clk"
>> +- led
>> + Required properties:
>> + - compatible: "mediatek,mt6323-led"
>> + see Documentation/devicetree/bindings/leds/leds-mt6323.txt
>
> Part of this patch is already part of linux-next:
> 05daab3 ("Documentation: devicetree: Add LED subnode binding for MT6323
> PMIC")

Yes, it's been merged via mfd tree. It doesn't break anything besides
that it points to non-existent file. Hopefully we will manage to merge
the LED part before merge window, otherwise the mfd part will have to
be dropped.

--
Best regards,
Jacek Anaszewski

2017-03-14 05:40:20

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] dt-bindings: mfd: Add LED subnode binding for MT6323 PMIC

On Mon, 2017-03-13 at 13:00 +0100, Matthias Brugger wrote:
>
> On 03/03/17 14:56, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > This patch adds documentation for devicetree bindings
> > for LED support as the subnode of MT6323 PMIC
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > Documentation/devicetree/bindings/mfd/mt6397.txt | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
> > index 949c85f..522a3bb 100644
> > --- a/Documentation/devicetree/bindings/mfd/mt6397.txt
> > +++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
> > @@ -6,6 +6,7 @@ MT6397/MT6323 is a multifunction device with the following sub modules:
> > - Audio codec
> > - GPIO
> > - Clock
> > +- LED
> >
> > It is interfaced to host controller using SPI interface by a proprietary hardware
> > called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap.
> > @@ -34,6 +35,10 @@ Optional subnodes:
> > - clk
> > Required properties:
> > - compatible: "mediatek,mt6397-clk"
> > +- led
> > + Required properties:
> > + - compatible: "mediatek,mt6323-led"
> > + see Documentation/devicetree/bindings/leds/leds-mt6323.txt
>
> Part of this patch is already part of linux-next:
> 05daab3 ("Documentation: devicetree: Add LED subnode binding for MT6323
> PMIC")
>
Hi Matthias,

There is still small differences from the merged patch.

I will submit the series for the next version based on newer kernel and

split the already merged ones out. Thanks for your kindly remainder.

Sean
> Regards,
> Matthias


2017-03-14 14:42:56

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver

On Fri, 03 Mar 2017, [email protected] wrote:

> From: Sean Wang <[email protected]>
>
> Add compatible string as "mt6323-led" that will make
> the OF core spawn child devices for the LED subnode
> of that MT6323 MFD device.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/mfd/mt6397-core.c | 4 ++++
> 1 file changed, 4 insertions(+)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index e14d8b0..8e601c8 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -48,6 +48,10 @@
> .name = "mt6323-regulator",
> .of_compatible = "mediatek,mt6323-regulator"
> },
> + {

Put this on the line above.

> + .name = "mt6323-led",
> + .of_compatible = "mediatek,mt6323-led"
> + },
> };
>
> static const struct mfd_cell mt6397_devs[] = {

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog