linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC
@ 2017-02-24 18:47 sean.wang
  2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: sean.wang @ 2017-02-24 18:47 UTC (permalink / raw)
  To: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	robh+dt, mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

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 brightness as the input in mt6323_led_hw_on() call
- enhanced the 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.

Sean Wang (4):
  Documentation: devicetree: Add document bindings for leds-mt6323
  Documentation: devicetree: 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       |  64 +++
 Documentation/devicetree/bindings/mfd/mt6397.txt   |   4 +
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-mt6323.c                         | 500 +++++++++++++++++++++
 drivers/mfd/mt6397-core.c                          |   4 +
 6 files changed, 581 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6323.txt
 create mode 100644 drivers/leds/leds-mt6323.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323
  2017-02-24 18:47 [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC sean.wang
@ 2017-02-24 18:47 ` sean.wang
  2017-02-26 17:35   ` Jacek Anaszewski
  2017-02-28  1:17   ` Rob Herring
  2017-02-24 18:47 ` [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC sean.wang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: sean.wang @ 2017-02-24 18:47 UTC (permalink / raw)
  To: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	robh+dt, mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

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

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../devicetree/bindings/leds/leds-mt6323.txt       | 64 ++++++++++++++++++++++
 1 file changed, 64 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..950b8c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
@@ -0,0 +1,64 @@
+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"
+- reg : LED channel number (0..3)
+
+Optional properties:
+- 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:
+
+&pwrap {
+	pmic: mt6323 {
+		compatible = "mediatek,mt6323";
+		interrupt-parent = <&pio>;
+		interrupts = <150 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		mt6323led: mt6323led{
+			compatible = "mediatek,mt6323-led";
+
+			led0: isink0 {
+				reg = <0>;
+				label = "LED0"
+				linux,default-trigger = "timer";
+				default-state = "on";
+			};
+			led1: isink1 {
+				reg = <1>;
+				label = "LED1";
+				default-state = "on";
+			};
+			led2: isink2 {
+				reg = <2>;
+				label = "LED2";
+				linux,default-trigger = "timer";
+				default-state = "off";
+			};
+		};
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC
  2017-02-24 18:47 [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC sean.wang
  2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
@ 2017-02-24 18:47 ` sean.wang
  2017-02-28  1:15   ` Rob Herring
  2017-02-24 18:47 ` [PATCH v4 3/4] leds: Add LED support " sean.wang
  2017-02-24 18:47 ` [PATCH v4 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver sean.wang
  3 siblings, 1 reply; 10+ messages in thread
From: sean.wang @ 2017-02-24 18:47 UTC (permalink / raw)
  To: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	robh+dt, mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

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

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 Documentation/devicetree/bindings/mfd/mt6397.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index 949c85f..c568d52 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -34,6 +34,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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/4] leds: Add LED support for MT6323 PMIC
  2017-02-24 18:47 [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC sean.wang
  2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
  2017-02-24 18:47 ` [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC sean.wang
@ 2017-02-24 18:47 ` sean.wang
  2017-02-26 17:35   ` Jacek Anaszewski
  2017-02-24 18:47 ` [PATCH v4 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver sean.wang
  3 siblings, 1 reply; 10+ messages in thread
From: sean.wang @ 2017-02-24 18:47 UTC (permalink / raw)
  To: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	robh+dt, mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

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 <sean.wang@mediatek.com>
---
 drivers/leds/Kconfig       |   8 +
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-mt6323.c | 500 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 509 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..7274581
--- /dev/null
+++ b/drivers/leds/leds-mt6323.c
@@ -0,0 +1,500 @@
+/*
+ * LED driver for Mediatek MT6323 PMIC
+ *
+ * Copyright (C) 2017 Sean Wang <sean.wang@mediatek.com>
+ *
+ * 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
+
+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;
+	u16 period;
+	u8 duty_cycle, duty_hw;
+	int ret;
+
+	/*
+	 * Units are in ms, if over the hardware able
+	 * to support, fallback into software blink
+	 */
+	if (*delay_on + *delay_off > 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;
+	}
+
+	period = *delay_on + *delay_off;
+	/*
+	 * duty_cycle is the percentage of period during
+	 * which the led is ON.
+	 */
+	duty_cycle = 100 * (*delay_on) / period;
+
+	/* hardware doesn't support zero duty cycle. */
+	if (!duty_cycle)
+		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;
+	}
+
+	duty_hw = DIV_ROUND_CLOSEST(duty_cycle * 1000, MT6323_UNIT_DUTY);
+	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");
+			return ret;
+		}
+
+		ret = devm_led_classdev_register(dev, &leds->led[reg]->cdev);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to register LED: %d\n",
+				ret);
+			return ret;
+		}
+		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 <sean.wang@mediatek.com>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver
  2017-02-24 18:47 [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC sean.wang
                   ` (2 preceding siblings ...)
  2017-02-24 18:47 ` [PATCH v4 3/4] leds: Add LED support " sean.wang
@ 2017-02-24 18:47 ` sean.wang
  3 siblings, 0 replies; 10+ messages in thread
From: sean.wang @ 2017-02-24 18:47 UTC (permalink / raw)
  To: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	robh+dt, mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

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 <sean.wang@mediatek.com>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323
  2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
@ 2017-02-26 17:35   ` Jacek Anaszewski
  2017-02-28  1:14     ` Rob Herring
  2017-02-28  1:17   ` Rob Herring
  1 sibling, 1 reply; 10+ messages in thread
From: Jacek Anaszewski @ 2017-02-26 17:35 UTC (permalink / raw)
  To: sean.wang, rpurdie, lee.jones, matthias.bgg, pavel, robh+dt,
	mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede

Hi Sean,

Thanks for the update. I have one comment below.

On 02/24/2017 07:47 PM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> This patch adds documentation for devicetree bindings
> for LED support on MT6323 PMIC
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../devicetree/bindings/leds/leds-mt6323.txt       | 64 ++++++++++++++++++++++
>  1 file changed, 64 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..950b8c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
> @@ -0,0 +1,64 @@
> +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"
> +- reg : LED channel number (0..3)
> +
> +Optional properties:
> +- 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:
> +
> +&pwrap {
> +	pmic: mt6323 {
> +		compatible = "mediatek,mt6323";
> +		interrupt-parent = <&pio>;
> +		interrupts = <150 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +
> +		mt6323led: mt6323led{
> +			compatible = "mediatek,mt6323-led";

You require here also:

#address-cells = <1>;
#size-cells = <0>;

Without these entries you should notice a warning from dtc compiler.


> +
> +			led0: isink0 {
> +				reg = <0>;
> +				label = "LED0"
> +				linux,default-trigger = "timer";
> +				default-state = "on";
> +			};
> +			led1: isink1 {
> +				reg = <1>;
> +				label = "LED1";
> +				default-state = "on";
> +			};
> +			led2: isink2 {
> +				reg = <2>;
> +				label = "LED2";
> +				linux,default-trigger = "timer";
> +				default-state = "off";
> +			};
> +		};
> +	};
> +};
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/4] leds: Add LED support for MT6323 PMIC
  2017-02-24 18:47 ` [PATCH v4 3/4] leds: Add LED support " sean.wang
@ 2017-02-26 17:35   ` Jacek Anaszewski
  0 siblings, 0 replies; 10+ messages in thread
From: Jacek Anaszewski @ 2017-02-26 17:35 UTC (permalink / raw)
  To: sean.wang, rpurdie, lee.jones, matthias.bgg, pavel, robh+dt,
	mark.rutland
  Cc: devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede

Hi Sean,

Thanks for the update. I have one comment to your changes below.

On 02/24/2017 07:47 PM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> 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 <sean.wang@mediatek.com>
> ---
>  drivers/leds/Kconfig       |   8 +
>  drivers/leds/Makefile      |   1 +
>  drivers/leds/leds-mt6323.c | 500 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 509 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..7274581
> --- /dev/null
> +++ b/drivers/leds/leds-mt6323.c
> @@ -0,0 +1,500 @@
> +/*
> + * LED driver for Mediatek MT6323 PMIC
> + *
> + * Copyright (C) 2017 Sean Wang <sean.wang@mediatek.com>
> + *
> + * 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
> +
> +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;
> +	u16 period;
> +	u8 duty_cycle, duty_hw;
> +	int ret;
> +
> +	/*
> +	 * Units are in ms, if over the hardware able
> +	 * to support, fallback into software blink
> +	 */
> +	if (*delay_on + *delay_off > 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;
> +	}
> +
> +	period = *delay_on + *delay_off;
> +	/*
> +	 * duty_cycle is the percentage of period during
> +	 * which the led is ON.
> +	 */
> +	duty_cycle = 100 * (*delay_on) / period;
> +
> +	/* hardware doesn't support zero duty cycle. */
> +	if (!duty_cycle)
> +		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;
> +	}
> +
> +	duty_hw = DIV_ROUND_CLOSEST(duty_cycle * 1000, MT6323_UNIT_DUTY);
> +	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");
> +			return ret;

Here you also need "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);
> +			return ret;

Ditto.

> +		}
> +		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 <sean.wang@mediatek.com>");
> +MODULE_LICENSE("GPL");
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323
  2017-02-26 17:35   ` Jacek Anaszewski
@ 2017-02-28  1:14     ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2017-02-28  1:14 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: sean.wang, rpurdie, lee.jones, matthias.bgg, pavel, mark.rutland,
	devicetree, linux-leds, linux-mediatek, linux-arm-kernel,
	linux-kernel, keyhaede

On Sun, Feb 26, 2017 at 06:35:11PM +0100, Jacek Anaszewski wrote:
> Hi Sean,
> 
> Thanks for the update. I have one comment below.
> 
> On 02/24/2017 07:47 PM, sean.wang@mediatek.com wrote:
> > From: Sean Wang <sean.wang@mediatek.com>
> > 
> > This patch adds documentation for devicetree bindings
> > for LED support on MT6323 PMIC
> > 
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> >  .../devicetree/bindings/leds/leds-mt6323.txt       | 64 ++++++++++++++++++++++
> >  1 file changed, 64 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..950b8c6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-mt6323.txt
> > @@ -0,0 +1,64 @@
> > +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"
> > +- reg : LED channel number (0..3)
> > +
> > +Optional properties:
> > +- 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:
> > +
> > +&pwrap {
> > +	pmic: mt6323 {
> > +		compatible = "mediatek,mt6323";
> > +		interrupt-parent = <&pio>;
> > +		interrupts = <150 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <2>;
> > +
> > +		mt6323led: mt6323led{
> > +			compatible = "mediatek,mt6323-led";
> 
> You require here also:
> 
> #address-cells = <1>;
> #size-cells = <0>;
> 
> Without these entries you should notice a warning from dtc compiler.
> 
> 
> > +
> > +			led0: isink0 {

Also, these should be "led@0"

> > +				reg = <0>;
> > +				label = "LED0"
> > +				linux,default-trigger = "timer";
> > +				default-state = "on";
> > +			};
> > +			led1: isink1 {
> > +				reg = <1>;
> > +				label = "LED1";
> > +				default-state = "on";
> > +			};
> > +			led2: isink2 {
> > +				reg = <2>;
> > +				label = "LED2";
> > +				linux,default-trigger = "timer";
> > +				default-state = "off";
> > +			};
> > +		};
> > +	};
> > +};
> > 
> 
> -- 
> Best regards,
> Jacek Anaszewski

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC
  2017-02-24 18:47 ` [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC sean.wang
@ 2017-02-28  1:15   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2017-02-28  1:15 UTC (permalink / raw)
  To: sean.wang
  Cc: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	mark.rutland, devicetree, linux-leds, linux-mediatek,
	linux-arm-kernel, linux-kernel, keyhaede

On Sat, Feb 25, 2017 at 02:47:19AM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> This patch adds documentation for devicetree bindings
> for LED support as the subnode of MT6323 PMIC
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  Documentation/devicetree/bindings/mfd/mt6397.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
> index 949c85f..c568d52 100644
> --- a/Documentation/devicetree/bindings/mfd/mt6397.txt
> +++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
> @@ -34,6 +34,10 @@ Optional subnodes:
>  - clk
>  	Required properties:
>  		- compatible: "mediatek,mt6397-clk"
> +- led

Your example in patch 1 doesn't match. This one is correct.

> +	Required properties:
> +		- compatible: "mediatek,mt6323-led"
> +	see Documentation/devicetree/bindings/leds/leds-mt6323.txt
>  
>  Example:
>  	pwrap: pwrap@1000f000 {
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323
  2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
  2017-02-26 17:35   ` Jacek Anaszewski
@ 2017-02-28  1:17   ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2017-02-28  1:17 UTC (permalink / raw)
  To: sean.wang
  Cc: rpurdie, jacek.anaszewski, lee.jones, matthias.bgg, pavel,
	mark.rutland, devicetree, linux-leds, linux-mediatek,
	linux-arm-kernel, linux-kernel, keyhaede

On Sat, Feb 25, 2017 at 02:47:18AM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> This patch adds documentation for devicetree bindings
> for LED support on MT6323 PMIC

Also, please use "dt-bindings: leds: ..." for the subject prefix.

> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../devicetree/bindings/leds/leds-mt6323.txt       | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-mt6323.txt

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-28  1:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-24 18:47 [PATCH v4 0/4] leds: add leds-mt6323 support on MT7623 SoC sean.wang
2017-02-24 18:47 ` [PATCH v4 1/4] Documentation: devicetree: Add document bindings for leds-mt6323 sean.wang
2017-02-26 17:35   ` Jacek Anaszewski
2017-02-28  1:14     ` Rob Herring
2017-02-28  1:17   ` Rob Herring
2017-02-24 18:47 ` [PATCH v4 2/4] Documentation: devicetree: Add LED subnode binding for MT6323 PMIC sean.wang
2017-02-28  1:15   ` Rob Herring
2017-02-24 18:47 ` [PATCH v4 3/4] leds: Add LED support " sean.wang
2017-02-26 17:35   ` Jacek Anaszewski
2017-02-24 18:47 ` [PATCH v4 4/4] mfd: mt6397: Add MT6323 LED support into MT6397 driver sean.wang

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).