linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831
@ 2021-04-26  7:18 cy_huang
  2021-04-26  7:18 ` [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight cy_huang
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: cy_huang @ 2021-04-26  7:18 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, daniel.thompson, jingoohan1,
	b.zolnierkie, pavel, robh+dt
  Cc: dri-devel, linux-fbdev, linux-kernel, linux-leds, devicetree, cy_huang

From: ChiYuan Huang <cy_huang@richtek.com>

This adds support Richtek RT4831 core. It includes four channel WLED driver
and Display Bias Voltage outputs.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
Resend this v6 patch series to loop devicetree reviewers.

The RT4831 regulator patches are already on main stream, and can be referred to
9351ab8b0cb6 regulator: rt4831: Adds support for Richtek RT4831 DSV regulator
934b05e81862 regulator: rt4831: Adds DT binding document for Richtek RT4831 DSV regulator

since v6
- Respin the date from 2020 to 2021.
- Rmove the shutdown routine.
- Change the macro OF_MFD_CELL to MFD_CELL_OF.


since v5
- Rename file name from rt4831-core.c to rt4831.c
- Change RICHTEK_VID to RICHTEK_VENDOR_ID.
- Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe.
- Change variable 'val' to the meaningful name 'chip_id'.
- Refine the error log when vendor id is not matched.
- Remove of_match_ptr.

since v2
- Refine Kconfig descriptions.
- Add copyright.
- Refine error logs in probe.
- Refine comment lines in remove and shutdown.
---
 drivers/mfd/Kconfig  |  10 +++++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/rt4831.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)
 create mode 100644 drivers/mfd/rt4831.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b74efa4..3f43834 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1065,6 +1065,16 @@ config MFD_RDC321X
 	  southbridge which provides access to GPIOs and Watchdog using the
 	  southbridge PCI device configuration space.
 
+config MFD_RT4831
+	tristate "Richtek RT4831 four channel WLED and Display Bias Voltage"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  This enables support for the Richtek RT4831 that includes 4 channel
+	  WLED driving and Display Bias Voltage. It's commonly used to provide
+	  power to the LCD display and LCD backlight.
+
 config MFD_RT5033
 	tristate "Richtek RT5033 Power Management IC"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 834f546..5986914 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
 obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
+obj-$(CONFIG_MFD_RT4831)	+= rt4831.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
 
diff --git a/drivers/mfd/rt4831.c b/drivers/mfd/rt4831.c
new file mode 100644
index 00000000..b169781
--- /dev/null
+++ b/drivers/mfd/rt4831.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Richtek Technology Corp.
+ *
+ * Author: ChiYuan Huang <cy_huang@richtek.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_REVISION	0x01
+#define RT4831_REG_ENABLE	0x08
+#define RT4831_REG_I2CPROT	0x15
+
+#define RICHTEK_VENDOR_ID	0x03
+#define RT4831_VID_MASK		GENMASK(1, 0)
+#define RT4831_RESET_MASK	BIT(7)
+#define RT4831_I2CSAFETMR_MASK	BIT(0)
+
+static const struct mfd_cell rt4831_subdevs[] = {
+	MFD_CELL_OF("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+	MFD_CELL_NAME("rt4831-regulator")
+};
+
+static bool rt4831_is_accessible_reg(struct device *dev, unsigned int reg)
+{
+	if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT)
+		return true;
+	return false;
+}
+
+static const struct regmap_config rt4831_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RT4831_REG_I2CPROT,
+
+	.readable_reg = rt4831_is_accessible_reg,
+	.writeable_reg = rt4831_is_accessible_reg,
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+	struct gpio_desc *enable_gpio;
+	struct regmap *regmap;
+	unsigned int chip_id;
+	int ret;
+
+	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(enable_gpio)) {
+		dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+		return PTR_ERR(enable_gpio);
+	}
+
+	regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Failed to initialize regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	ret = regmap_read(regmap, RT4831_REG_REVISION, &chip_id);
+	if (ret) {
+		dev_err(&client->dev, "Failed to get H/W revision\n");
+		return ret;
+	}
+
+	if ((chip_id & RT4831_VID_MASK) != RICHTEK_VENDOR_ID) {
+		dev_err(&client->dev, "Chip vendor ID 0x%02x not matched\n", chip_id);
+		return -ENODEV;
+	}
+
+	/*
+	 * Used to prevent the abnormal shutdown.
+	 * If SCL/SDA both keep low for one second to reset HW.
+	 */
+	ret = regmap_update_bits(regmap, RT4831_REG_I2CPROT, RT4831_I2CSAFETMR_MASK,
+				 RT4831_I2CSAFETMR_MASK);
+	if (ret) {
+		dev_err(&client->dev, "Failed to enable I2C safety timer\n");
+		return ret;
+	}
+
+	return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, rt4831_subdevs,
+				    ARRAY_SIZE(rt4831_subdevs), NULL, 0, NULL);
+}
+
+static int rt4831_remove(struct i2c_client *client)
+{
+	struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+	/* Disable WLED and DSV outputs */
+	return regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}
+
+static const struct of_device_id __maybe_unused rt4831_of_match[] = {
+	{ .compatible = "richtek,rt4831", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rt4831_of_match);
+
+static struct i2c_driver rt4831_driver = {
+	.driver = {
+		.name = "rt4831",
+		.of_match_table = rt4831_of_match,
+	},
+	.probe_new = rt4831_probe,
+	.remove = rt4831_remove,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


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

* [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight
  2021-04-26  7:18 [RESEND PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831 cy_huang
@ 2021-04-26  7:18 ` cy_huang
  2021-04-30 20:10   ` Rob Herring
  2021-04-26  7:18 ` [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831 cy_huang
  2021-04-26  7:18 ` [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight cy_huang
  2 siblings, 1 reply; 11+ messages in thread
From: cy_huang @ 2021-04-26  7:18 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, daniel.thompson, jingoohan1,
	b.zolnierkie, pavel, robh+dt
  Cc: dri-devel, linux-fbdev, linux-kernel, linux-leds, devicetree, cy_huang

From: ChiYuan Huang <cy_huang@richtek.com>

Adds DT binding document for Richtek RT4831 backlight.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Resend this v6 patch series to loop devicetree reviewers.

For next, need to add the below line
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 65 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 ++++++++
 2 files changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
new file mode 100644
index 00000000..4da6a66
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/richtek,rt4831-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT4831 Backlight
+
+maintainers:
+  - ChiYuan Huang <cy_huang@richtek.com>
+
+description: |
+  RT4831 is a mutifunctional device that can provide power to the LCD display
+  and LCD backlight.
+
+  For the LCD backlight, it can provide four channel WLED driving capability.
+  Each channel driving current is up to 30mA
+
+  Datasheet is available at
+  https://www.richtek.com/assets/product_file/RT4831A/DS4831A-05.pdf
+
+properties:
+  compatible:
+    const: richtek,rt4831-backlight
+
+  default-brightness:
+    description: |
+      The default brightness that applied to the system on start-up.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 2048
+
+  max-brightness:
+    description: |
+      The max brightness for the H/W limit
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 2048
+
+  richtek,pwm-enable:
+    description: |
+      Specify the backlight dimming following by PWM duty or by SW control.
+    type: boolean
+
+  richtek,bled-ovp-sel:
+    description: |
+      Backlight OVP level selection, currently support 17V/21V/25V/29V.
+    $ref: /schemas/types.yaml#/definitions/uint8
+    default: 1
+    minimum: 0
+    maximum: 3
+
+  richtek,channel-use:
+    description: |
+      Backlight LED channel to be used.
+      BIT 0/1/2/3 is used to indicate led channel 1/2/3/4 enable or disable.
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 1
+    maximum: 15
+
+required:
+  - compatible
+  - richtek,channel-use
+
+additionalProperties: false
diff --git a/include/dt-bindings/leds/rt4831-backlight.h b/include/dt-bindings/leds/rt4831-backlight.h
new file mode 100644
index 00000000..125c635
--- /dev/null
+++ b/include/dt-bindings/leds/rt4831-backlight.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * This header provides constants for rt4831 backlight bindings.
+ *
+ * Copyright (C) 2020, Richtek Technology Corp.
+ * Author: ChiYuan Huang <cy_huang@richtek.com>
+ */
+
+#ifndef _DT_BINDINGS_RT4831_BACKLIGHT_H
+#define _DT_BINDINGS_RT4831_BACKLIGHT_H
+
+#define RT4831_BLOVPLVL_17V	0
+#define RT4831_BLOVPLVL_21V	1
+#define RT4831_BLOVPLVL_25V	2
+#define RT4831_BLOVPLVL_29V	3
+
+#define RT4831_BLED_CH1EN	(1 << 0)
+#define RT4831_BLED_CH2EN	(1 << 1)
+#define RT4831_BLED_CH3EN	(1 << 2)
+#define RT4831_BLED_CH4EN	(1 << 3)
+#define RT4831_BLED_ALLCHEN	((1 << 4) - 1)
+
+#endif /* _DT_BINDINGS_RT4831_BACKLIGHT_H */
-- 
2.7.4


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

* [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831
  2021-04-26  7:18 [RESEND PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831 cy_huang
  2021-04-26  7:18 ` [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight cy_huang
@ 2021-04-26  7:18 ` cy_huang
  2021-04-30 20:10   ` Rob Herring
  2021-04-26  7:18 ` [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight cy_huang
  2 siblings, 1 reply; 11+ messages in thread
From: cy_huang @ 2021-04-26  7:18 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, daniel.thompson, jingoohan1,
	b.zolnierkie, pavel, robh+dt
  Cc: dri-devel, linux-fbdev, linux-kernel, linux-leds, devicetree, cy_huang

From: ChiYuan Huang <cy_huang@richtek.com>

Adds DT binding document for Richtek RT4831.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
Resend this v6 patch series to loop devicetree reviewers.
---
 .../devicetree/bindings/mfd/richtek,rt4831.yaml    | 90 ++++++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml

diff --git a/Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml b/Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml
new file mode 100644
index 00000000..4762eb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/richtek,rt4831.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT4831 DSV and Backlight Integrated IC
+
+maintainers:
+  - ChiYuan Huang <cy_huang@richtek.com>
+
+description: |
+  RT4831 is a multifunctional device that can provide power to the LCD display
+  and LCD backlight.
+
+  For Display Bias Voltage DSVP and DSVN, the output range is about 4V to 6.5V.
+  It's sufficient to meet the current LCD power requirement.
+
+  For the LCD backlight, it can provide four channel WLED driving capability.
+  Each channel driving current is up to 30mA
+
+  Datasheet is available at
+  https://www.richtek.com/assets/product_file/RT4831A/DS4831A-05.pdf
+
+properties:
+  compatible:
+    const: richtek,rt4831
+
+  reg:
+    description: I2C device address.
+    maxItems: 1
+
+  enable-gpios:
+    description: |
+      GPIO to enable/disable the chip. It is optional.
+      Some usage directly tied this pin to follow VIO 1.8V power on sequence.
+    maxItems: 1
+
+  regulators:
+    $ref: ../regulator/richtek,rt4831-regulator.yaml
+
+  backlight:
+    $ref: ../leds/backlight/richtek,rt4831-backlight.yaml
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      rt4831@11 {
+        compatible = "richtek,rt4831";
+        reg = <0x11>;
+
+        regulators {
+          DSVLCM {
+            regulator-min-microvolt = <4000000>;
+            regulator-max-microvolt = <7150000>;
+            regulator-allow-bypass;
+          };
+          DSVP {
+            regulator-name = "rt4831-dsvp";
+            regulator-min-microvolt = <4000000>;
+            regulator-max-microvolt = <6500000>;
+            regulator-boot-on;
+          };
+          DSVN {
+            regulator-name = "rt4831-dsvn";
+            regulator-min-microvolt = <4000000>;
+            regulator-max-microvolt = <6500000>;
+            regulator-boot-on;
+          };
+        };
+
+        backlight {
+          compatible = "richtek,rt4831-backlight";
+          default-brightness = <1024>;
+          max-brightness = <2048>;
+          richtek,bled-ovp-sel = /bits/ 8 <RT4831_BLOVPLVL_21V>;
+          richtek,channel-use = /bits/ 8 <RT4831_BLED_ALLCHEN>;
+        };
+      };
+    };
-- 
2.7.4


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

* [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight
  2021-04-26  7:18 [RESEND PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831 cy_huang
  2021-04-26  7:18 ` [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight cy_huang
  2021-04-26  7:18 ` [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831 cy_huang
@ 2021-04-26  7:18 ` cy_huang
  2021-04-26 10:19   ` Daniel Thompson
  2 siblings, 1 reply; 11+ messages in thread
From: cy_huang @ 2021-04-26  7:18 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, daniel.thompson, jingoohan1,
	b.zolnierkie, pavel, robh+dt
  Cc: dri-devel, linux-fbdev, linux-kernel, linux-leds, devicetree, cy_huang

From: ChiYuan Huang <cy_huang@richtek.com>

Adds support for Richtek RT4831 backlight.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Resend this v6 patch series to loop devicetree reviewers.

For next, if the typo in Kconfig 'common' to 'commonly' can be added the below line
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/video/backlight/Kconfig            |   8 ++
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/rt4831-backlight.c | 203 +++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 drivers/video/backlight/rt4831-backlight.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d83c87b..de96441 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -289,6 +289,14 @@ config BACKLIGHT_QCOM_WLED
 	  If you have the Qualcomm PMIC, say Y to enable a driver for the
 	  WLED block. Currently it supports PM8941 and PMI8998.
 
+config BACKLIGHT_RT4831
+	tristate "Richtek RT4831 Backlight Driver"
+	depends on MFD_RT4831
+	help
+	  This enables support for Richtek RT4831 Backlight driver.
+	  It's common used to drive the display WLED. There're four channels
+	  inisde, and each channel can provide up to 30mA current.
+
 config BACKLIGHT_SAHARA
 	tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
 	depends on X86
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 685f3f1..cae2c83 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
 obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
 obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
 obj-$(CONFIG_BACKLIGHT_QCOM_WLED)	+= qcom-wled.o
+obj-$(CONFIG_BACKLIGHT_RT4831)		+= rt4831-backlight.o
 obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
 obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
 obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
diff --git a/drivers/video/backlight/rt4831-backlight.c b/drivers/video/backlight/rt4831-backlight.c
new file mode 100644
index 00000000..42155c7
--- /dev/null
+++ b/drivers/video/backlight/rt4831-backlight.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <dt-bindings/leds/rt4831-backlight.h>
+#include <linux/backlight.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_BLCFG	0x02
+#define RT4831_REG_BLDIML	0x04
+#define RT4831_REG_ENABLE	0x08
+
+#define RT4831_BLMAX_BRIGHTNESS	2048
+
+#define RT4831_BLOVP_MASK	GENMASK(7, 5)
+#define RT4831_BLOVP_SHIFT	5
+#define RT4831_BLPWMEN_MASK	BIT(0)
+#define RT4831_BLEN_MASK	BIT(4)
+#define RT4831_BLCH_MASK	GENMASK(3, 0)
+#define RT4831_BLDIML_MASK	GENMASK(2, 0)
+#define RT4831_BLDIMH_MASK	GENMASK(10, 3)
+#define RT4831_BLDIMH_SHIFT	3
+
+struct rt4831_priv {
+	struct device *dev;
+	struct regmap *regmap;
+	struct backlight_device *bl;
+};
+
+static int rt4831_bl_update_status(struct backlight_device *bl_dev)
+{
+	struct rt4831_priv *priv = bl_get_data(bl_dev);
+	int brightness = backlight_get_brightness(bl_dev);
+	unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
+	u8 v[2];
+	int ret;
+
+	if (brightness) {
+		v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
+		v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
+
+		ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+		if (ret)
+			return ret;
+	}
+
+	return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
+
+}
+
+static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
+{
+	struct rt4831_priv *priv = bl_get_data(bl_dev);
+	unsigned int val;
+	u8 v[2];
+	int ret;
+
+	ret = regmap_read(priv->regmap, RT4831_REG_ENABLE, &val);
+	if (ret)
+		return ret;
+
+	if (!(val & RT4831_BLEN_MASK))
+		return 0;
+
+	ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+	if (ret)
+		return ret;
+
+	ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
+
+	return ret;
+}
+
+static const struct backlight_ops rt4831_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = rt4831_bl_update_status,
+	.get_brightness = rt4831_bl_get_brightness,
+};
+
+static int rt4831_parse_backlight_properties(struct rt4831_priv *priv,
+					     struct backlight_properties *bl_props)
+{
+	struct device *dev = priv->dev;
+	u8 propval;
+	u32 brightness;
+	unsigned int val = 0;
+	int ret;
+
+	/* common properties */
+	ret = device_property_read_u32(dev, "max-brightness", &brightness);
+	if (ret)
+		brightness = RT4831_BLMAX_BRIGHTNESS;
+
+	bl_props->max_brightness = min_t(u32, brightness, RT4831_BLMAX_BRIGHTNESS);
+
+	ret = device_property_read_u32(dev, "default-brightness", &brightness);
+	if (ret)
+		brightness = bl_props->max_brightness;
+
+	bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
+
+	/* vendor properties */
+	if (device_property_read_bool(dev, "richtek,pwm-enable"))
+		val = RT4831_BLPWMEN_MASK;
+
+	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
+	if (ret)
+		propval = RT4831_BLOVPLVL_21V;
+
+	propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
+	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
+				 propval << RT4831_BLOVP_SHIFT);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
+	if (ret) {
+		dev_err(dev, "richtek,channel-use DT property missing\n");
+		return ret;
+	}
+
+	if (!(propval & RT4831_BLCH_MASK)) {
+		dev_err(dev, "No channel specified\n");
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLCH_MASK, propval);
+}
+
+static int rt4831_bl_probe(struct platform_device *pdev)
+{
+	struct rt4831_priv *priv;
+	struct backlight_properties bl_props = { .type = BACKLIGHT_RAW,
+						 .scale = BACKLIGHT_SCALE_LINEAR };
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!priv->regmap) {
+		dev_err(&pdev->dev, "Failed to init regmap\n");
+		return -ENODEV;
+	}
+
+	ret = rt4831_parse_backlight_properties(priv, &bl_props);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to parse backlight properties\n");
+		return ret;
+	}
+
+	priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, priv,
+						  &rt4831_bl_ops, &bl_props);
+	if (IS_ERR(priv->bl)) {
+		dev_err(&pdev->dev, "Failed to register backlight\n");
+		return PTR_ERR(priv->bl);
+	}
+
+	backlight_update_status(priv->bl);
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int rt4831_bl_remove(struct platform_device *pdev)
+{
+	struct rt4831_priv *priv = platform_get_drvdata(pdev);
+	struct backlight_device *bl_dev = priv->bl;
+
+	bl_dev->props.brightness = 0;
+	backlight_update_status(priv->bl);
+
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused rt4831_bl_of_match[] = {
+	{ .compatible = "richtek,rt4831-backlight", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rt4831_bl_of_match);
+
+static struct platform_driver rt4831_bl_driver = {
+	.driver = {
+		.name = "rt4831-backlight",
+		.of_match_table = rt4831_bl_of_match,
+	},
+	.probe = rt4831_bl_probe,
+	.remove = rt4831_bl_remove,
+};
+module_platform_driver(rt4831_bl_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


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

* Re: [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight
  2021-04-26  7:18 ` [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight cy_huang
@ 2021-04-26 10:19   ` Daniel Thompson
  2021-04-26 10:50     ` ChiYuan Huang
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2021-04-26 10:19 UTC (permalink / raw)
  To: cy_huang
  Cc: lee.jones, lgirdwood, broonie, jingoohan1, b.zolnierkie, pavel,
	robh+dt, dri-devel, linux-fbdev, linux-kernel, linux-leds,
	devicetree, cy_huang

On Mon, Apr 26, 2021 at 03:18:11PM +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Adds support for Richtek RT4831 backlight.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> Resend this v6 patch series to loop devicetree reviewers.
> 
> For next, if the typo in Kconfig 'common' to 'commonly' can be added the below line
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---

This isn't the best way to handle feedback from multiple maintainers.

It is great to see you are keeping track of feedback. However it doesn't
make sense to RESEND an old patchset and acknowledge that you haven't
fixed a typo yet.

It would be better to fix the typo and to resend a v7.


Daniel.


>  drivers/video/backlight/Kconfig            |   8 ++
>  drivers/video/backlight/Makefile           |   1 +
>  drivers/video/backlight/rt4831-backlight.c | 203 +++++++++++++++++++++++++++++
>  3 files changed, 212 insertions(+)
>  create mode 100644 drivers/video/backlight/rt4831-backlight.c
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index d83c87b..de96441 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -289,6 +289,14 @@ config BACKLIGHT_QCOM_WLED
>  	  If you have the Qualcomm PMIC, say Y to enable a driver for the
>  	  WLED block. Currently it supports PM8941 and PMI8998.
>  
> +config BACKLIGHT_RT4831
> +	tristate "Richtek RT4831 Backlight Driver"
> +	depends on MFD_RT4831
> +	help
> +	  This enables support for Richtek RT4831 Backlight driver.
> +	  It's common used to drive the display WLED. There're four channels
> +	  inisde, and each channel can provide up to 30mA current.
> +
>  config BACKLIGHT_SAHARA
>  	tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
>  	depends on X86
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 685f3f1..cae2c83 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
>  obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
>  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
>  obj-$(CONFIG_BACKLIGHT_QCOM_WLED)	+= qcom-wled.o
> +obj-$(CONFIG_BACKLIGHT_RT4831)		+= rt4831-backlight.o
>  obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
>  obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
>  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
> diff --git a/drivers/video/backlight/rt4831-backlight.c b/drivers/video/backlight/rt4831-backlight.c
> new file mode 100644
> index 00000000..42155c7
> --- /dev/null
> +++ b/drivers/video/backlight/rt4831-backlight.c
> @@ -0,0 +1,203 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <dt-bindings/leds/rt4831-backlight.h>
> +#include <linux/backlight.h>
> +#include <linux/bitops.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define RT4831_REG_BLCFG	0x02
> +#define RT4831_REG_BLDIML	0x04
> +#define RT4831_REG_ENABLE	0x08
> +
> +#define RT4831_BLMAX_BRIGHTNESS	2048
> +
> +#define RT4831_BLOVP_MASK	GENMASK(7, 5)
> +#define RT4831_BLOVP_SHIFT	5
> +#define RT4831_BLPWMEN_MASK	BIT(0)
> +#define RT4831_BLEN_MASK	BIT(4)
> +#define RT4831_BLCH_MASK	GENMASK(3, 0)
> +#define RT4831_BLDIML_MASK	GENMASK(2, 0)
> +#define RT4831_BLDIMH_MASK	GENMASK(10, 3)
> +#define RT4831_BLDIMH_SHIFT	3
> +
> +struct rt4831_priv {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct backlight_device *bl;
> +};
> +
> +static int rt4831_bl_update_status(struct backlight_device *bl_dev)
> +{
> +	struct rt4831_priv *priv = bl_get_data(bl_dev);
> +	int brightness = backlight_get_brightness(bl_dev);
> +	unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
> +	u8 v[2];
> +	int ret;
> +
> +	if (brightness) {
> +		v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
> +		v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
> +
> +		ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
> +
> +}
> +
> +static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
> +{
> +	struct rt4831_priv *priv = bl_get_data(bl_dev);
> +	unsigned int val;
> +	u8 v[2];
> +	int ret;
> +
> +	ret = regmap_read(priv->regmap, RT4831_REG_ENABLE, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (!(val & RT4831_BLEN_MASK))
> +		return 0;
> +
> +	ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> +	if (ret)
> +		return ret;
> +
> +	ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
> +
> +	return ret;
> +}
> +
> +static const struct backlight_ops rt4831_bl_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = rt4831_bl_update_status,
> +	.get_brightness = rt4831_bl_get_brightness,
> +};
> +
> +static int rt4831_parse_backlight_properties(struct rt4831_priv *priv,
> +					     struct backlight_properties *bl_props)
> +{
> +	struct device *dev = priv->dev;
> +	u8 propval;
> +	u32 brightness;
> +	unsigned int val = 0;
> +	int ret;
> +
> +	/* common properties */
> +	ret = device_property_read_u32(dev, "max-brightness", &brightness);
> +	if (ret)
> +		brightness = RT4831_BLMAX_BRIGHTNESS;
> +
> +	bl_props->max_brightness = min_t(u32, brightness, RT4831_BLMAX_BRIGHTNESS);
> +
> +	ret = device_property_read_u32(dev, "default-brightness", &brightness);
> +	if (ret)
> +		brightness = bl_props->max_brightness;
> +
> +	bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
> +
> +	/* vendor properties */
> +	if (device_property_read_bool(dev, "richtek,pwm-enable"))
> +		val = RT4831_BLPWMEN_MASK;
> +
> +	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
> +	if (ret)
> +		propval = RT4831_BLOVPLVL_21V;
> +
> +	propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
> +	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
> +				 propval << RT4831_BLOVP_SHIFT);
> +	if (ret)
> +		return ret;
> +
> +	ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
> +	if (ret) {
> +		dev_err(dev, "richtek,channel-use DT property missing\n");
> +		return ret;
> +	}
> +
> +	if (!(propval & RT4831_BLCH_MASK)) {
> +		dev_err(dev, "No channel specified\n");
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLCH_MASK, propval);
> +}
> +
> +static int rt4831_bl_probe(struct platform_device *pdev)
> +{
> +	struct rt4831_priv *priv;
> +	struct backlight_properties bl_props = { .type = BACKLIGHT_RAW,
> +						 .scale = BACKLIGHT_SCALE_LINEAR };
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = &pdev->dev;
> +
> +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!priv->regmap) {
> +		dev_err(&pdev->dev, "Failed to init regmap\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = rt4831_parse_backlight_properties(priv, &bl_props);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to parse backlight properties\n");
> +		return ret;
> +	}
> +
> +	priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, priv,
> +						  &rt4831_bl_ops, &bl_props);
> +	if (IS_ERR(priv->bl)) {
> +		dev_err(&pdev->dev, "Failed to register backlight\n");
> +		return PTR_ERR(priv->bl);
> +	}
> +
> +	backlight_update_status(priv->bl);
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static int rt4831_bl_remove(struct platform_device *pdev)
> +{
> +	struct rt4831_priv *priv = platform_get_drvdata(pdev);
> +	struct backlight_device *bl_dev = priv->bl;
> +
> +	bl_dev->props.brightness = 0;
> +	backlight_update_status(priv->bl);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused rt4831_bl_of_match[] = {
> +	{ .compatible = "richtek,rt4831-backlight", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rt4831_bl_of_match);
> +
> +static struct platform_driver rt4831_bl_driver = {
> +	.driver = {
> +		.name = "rt4831-backlight",
> +		.of_match_table = rt4831_bl_of_match,
> +	},
> +	.probe = rt4831_bl_probe,
> +	.remove = rt4831_bl_remove,
> +};
> +module_platform_driver(rt4831_bl_driver);
> +
> +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 

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

* Re: [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight
  2021-04-26 10:19   ` Daniel Thompson
@ 2021-04-26 10:50     ` ChiYuan Huang
  2021-04-26 12:24       ` Daniel Thompson
  0 siblings, 1 reply; 11+ messages in thread
From: ChiYuan Huang @ 2021-04-26 10:50 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Lee Jones, lgirdwood, Mark Brown, jingoohan1, b.zolnierkie,
	Pavel Machek, Rob Herring, dri-devel, linux-fbdev, lkml,
	Linux LED Subsystem,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	cy_huang

Hi,

Daniel Thompson <daniel.thompson@linaro.org> 於 2021年4月26日 週一 下午6:19寫道:
>
> On Mon, Apr 26, 2021 at 03:18:11PM +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > Adds support for Richtek RT4831 backlight.
> >
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> > Resend this v6 patch series to loop devicetree reviewers.
> >
> > For next, if the typo in Kconfig 'common' to 'commonly' can be added the below line
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
>
> This isn't the best way to handle feedback from multiple maintainers.
>
> It is great to see you are keeping track of feedback. However it doesn't
> make sense to RESEND an old patchset and acknowledge that you haven't
> fixed a typo yet.
>
> It would be better to fix the typo and to resend a v7.
>
You can refer to the below reply
https://lkml.org/lkml/2021/4/23/229

>
> Daniel.
>
>
> >  drivers/video/backlight/Kconfig            |   8 ++
> >  drivers/video/backlight/Makefile           |   1 +
> >  drivers/video/backlight/rt4831-backlight.c | 203 +++++++++++++++++++++++++++++
> >  3 files changed, 212 insertions(+)
> >  create mode 100644 drivers/video/backlight/rt4831-backlight.c
> >
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index d83c87b..de96441 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -289,6 +289,14 @@ config BACKLIGHT_QCOM_WLED
> >         If you have the Qualcomm PMIC, say Y to enable a driver for the
> >         WLED block. Currently it supports PM8941 and PMI8998.
> >
> > +config BACKLIGHT_RT4831
> > +     tristate "Richtek RT4831 Backlight Driver"
> > +     depends on MFD_RT4831
> > +     help
> > +       This enables support for Richtek RT4831 Backlight driver.
> > +       It's common used to drive the display WLED. There're four channels
> > +       inisde, and each channel can provide up to 30mA current.
> > +
> >  config BACKLIGHT_SAHARA
> >       tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
> >       depends on X86
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index 685f3f1..cae2c83 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -49,6 +49,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA)             += pandora_bl.o
> >  obj-$(CONFIG_BACKLIGHT_PCF50633)     += pcf50633-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_PWM)          += pwm_bl.o
> >  obj-$(CONFIG_BACKLIGHT_QCOM_WLED)    += qcom-wled.o
> > +obj-$(CONFIG_BACKLIGHT_RT4831)               += rt4831-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_SAHARA)               += kb3886_bl.o
> >  obj-$(CONFIG_BACKLIGHT_SKY81452)     += sky81452-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_TOSA)         += tosa_bl.o
> > diff --git a/drivers/video/backlight/rt4831-backlight.c b/drivers/video/backlight/rt4831-backlight.c
> > new file mode 100644
> > index 00000000..42155c7
> > --- /dev/null
> > +++ b/drivers/video/backlight/rt4831-backlight.c
> > @@ -0,0 +1,203 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <dt-bindings/leds/rt4831-backlight.h>
> > +#include <linux/backlight.h>
> > +#include <linux/bitops.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +
> > +#define RT4831_REG_BLCFG     0x02
> > +#define RT4831_REG_BLDIML    0x04
> > +#define RT4831_REG_ENABLE    0x08
> > +
> > +#define RT4831_BLMAX_BRIGHTNESS      2048
> > +
> > +#define RT4831_BLOVP_MASK    GENMASK(7, 5)
> > +#define RT4831_BLOVP_SHIFT   5
> > +#define RT4831_BLPWMEN_MASK  BIT(0)
> > +#define RT4831_BLEN_MASK     BIT(4)
> > +#define RT4831_BLCH_MASK     GENMASK(3, 0)
> > +#define RT4831_BLDIML_MASK   GENMASK(2, 0)
> > +#define RT4831_BLDIMH_MASK   GENMASK(10, 3)
> > +#define RT4831_BLDIMH_SHIFT  3
> > +
> > +struct rt4831_priv {
> > +     struct device *dev;
> > +     struct regmap *regmap;
> > +     struct backlight_device *bl;
> > +};
> > +
> > +static int rt4831_bl_update_status(struct backlight_device *bl_dev)
> > +{
> > +     struct rt4831_priv *priv = bl_get_data(bl_dev);
> > +     int brightness = backlight_get_brightness(bl_dev);
> > +     unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
> > +     u8 v[2];
> > +     int ret;
> > +
> > +     if (brightness) {
> > +             v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
> > +             v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
> > +
> > +             ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
> > +
> > +}
> > +
> > +static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
> > +{
> > +     struct rt4831_priv *priv = bl_get_data(bl_dev);
> > +     unsigned int val;
> > +     u8 v[2];
> > +     int ret;
> > +
> > +     ret = regmap_read(priv->regmap, RT4831_REG_ENABLE, &val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (!(val & RT4831_BLEN_MASK))
> > +             return 0;
> > +
> > +     ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct backlight_ops rt4831_bl_ops = {
> > +     .options = BL_CORE_SUSPENDRESUME,
> > +     .update_status = rt4831_bl_update_status,
> > +     .get_brightness = rt4831_bl_get_brightness,
> > +};
> > +
> > +static int rt4831_parse_backlight_properties(struct rt4831_priv *priv,
> > +                                          struct backlight_properties *bl_props)
> > +{
> > +     struct device *dev = priv->dev;
> > +     u8 propval;
> > +     u32 brightness;
> > +     unsigned int val = 0;
> > +     int ret;
> > +
> > +     /* common properties */
> > +     ret = device_property_read_u32(dev, "max-brightness", &brightness);
> > +     if (ret)
> > +             brightness = RT4831_BLMAX_BRIGHTNESS;
> > +
> > +     bl_props->max_brightness = min_t(u32, brightness, RT4831_BLMAX_BRIGHTNESS);
> > +
> > +     ret = device_property_read_u32(dev, "default-brightness", &brightness);
> > +     if (ret)
> > +             brightness = bl_props->max_brightness;
> > +
> > +     bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
> > +
> > +     /* vendor properties */
> > +     if (device_property_read_bool(dev, "richtek,pwm-enable"))
> > +             val = RT4831_BLPWMEN_MASK;
> > +
> > +     ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
> > +     if (ret)
> > +             propval = RT4831_BLOVPLVL_21V;
> > +
> > +     propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
> > +     ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
> > +                              propval << RT4831_BLOVP_SHIFT);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
> > +     if (ret) {
> > +             dev_err(dev, "richtek,channel-use DT property missing\n");
> > +             return ret;
> > +     }
> > +
> > +     if (!(propval & RT4831_BLCH_MASK)) {
> > +             dev_err(dev, "No channel specified\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLCH_MASK, propval);
> > +}
> > +
> > +static int rt4831_bl_probe(struct platform_device *pdev)
> > +{
> > +     struct rt4831_priv *priv;
> > +     struct backlight_properties bl_props = { .type = BACKLIGHT_RAW,
> > +                                              .scale = BACKLIGHT_SCALE_LINEAR };
> > +     int ret;
> > +
> > +     priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     priv->dev = &pdev->dev;
> > +
> > +     priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > +     if (!priv->regmap) {
> > +             dev_err(&pdev->dev, "Failed to init regmap\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     ret = rt4831_parse_backlight_properties(priv, &bl_props);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "Failed to parse backlight properties\n");
> > +             return ret;
> > +     }
> > +
> > +     priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, priv,
> > +                                               &rt4831_bl_ops, &bl_props);
> > +     if (IS_ERR(priv->bl)) {
> > +             dev_err(&pdev->dev, "Failed to register backlight\n");
> > +             return PTR_ERR(priv->bl);
> > +     }
> > +
> > +     backlight_update_status(priv->bl);
> > +     platform_set_drvdata(pdev, priv);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rt4831_bl_remove(struct platform_device *pdev)
> > +{
> > +     struct rt4831_priv *priv = platform_get_drvdata(pdev);
> > +     struct backlight_device *bl_dev = priv->bl;
> > +
> > +     bl_dev->props.brightness = 0;
> > +     backlight_update_status(priv->bl);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id __maybe_unused rt4831_bl_of_match[] = {
> > +     { .compatible = "richtek,rt4831-backlight", },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, rt4831_bl_of_match);
> > +
> > +static struct platform_driver rt4831_bl_driver = {
> > +     .driver = {
> > +             .name = "rt4831-backlight",
> > +             .of_match_table = rt4831_bl_of_match,
> > +     },
> > +     .probe = rt4831_bl_probe,
> > +     .remove = rt4831_bl_remove,
> > +};
> > +module_platform_driver(rt4831_bl_driver);
> > +
> > +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> >

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

* Re: [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight
  2021-04-26 10:50     ` ChiYuan Huang
@ 2021-04-26 12:24       ` Daniel Thompson
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Thompson @ 2021-04-26 12:24 UTC (permalink / raw)
  To: ChiYuan Huang
  Cc: Lee Jones, lgirdwood, Mark Brown, jingoohan1, b.zolnierkie,
	Pavel Machek, Rob Herring, dri-devel, linux-fbdev, lkml,
	Linux LED Subsystem,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	cy_huang

On Mon, Apr 26, 2021 at 06:50:15PM +0800, ChiYuan Huang wrote:
> Hi,
> 
> Daniel Thompson <daniel.thompson@linaro.org> 於 2021年4月26日 週一 下午6:19寫道:
> >
> > On Mon, Apr 26, 2021 at 03:18:11PM +0800, cy_huang wrote:
> > > From: ChiYuan Huang <cy_huang@richtek.com>
> > >
> > > Adds support for Richtek RT4831 backlight.
> > >
> > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > ---
> > > Resend this v6 patch series to loop devicetree reviewers.
> > >
> > > For next, if the typo in Kconfig 'common' to 'commonly' can be added the below line
> > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > ---
> >
> > This isn't the best way to handle feedback from multiple maintainers.
> >
> > It is great to see you are keeping track of feedback. However it doesn't
> > make sense to RESEND an old patchset and acknowledge that you haven't
> > fixed a typo yet.
> >
> > It would be better to fix the typo and to resend a v7.
>
> You can refer to the below reply
> https://lkml.org/lkml/2021/4/23/229

Indeed.

However that thread is ambiguous about resending with changes (PATCH v7)
and resending without changes (PATCH RESEND v6). Given you said in March
there was a v7 series coming, I think it would have been better to send a
new version. 

Don't worry about this too much. This is turning into quite a lot of
fuss for a two letter typo. However it's very unusual to send out a
patch (except perhaps an RFC) together with a comment listing what you
need to fix next time around. It is pretty much always better to fix
it first and then send it.


Daniel.


> > >  drivers/video/backlight/Kconfig            |   8 ++
> > >  drivers/video/backlight/Makefile           |   1 +
> > >  drivers/video/backlight/rt4831-backlight.c | 203 +++++++++++++++++++++++++++++
> > >  3 files changed, 212 insertions(+)
> > >  create mode 100644 drivers/video/backlight/rt4831-backlight.c
> > >
> > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > > index d83c87b..de96441 100644
> > > --- a/drivers/video/backlight/Kconfig
> > > +++ b/drivers/video/backlight/Kconfig
> > > @@ -289,6 +289,14 @@ config BACKLIGHT_QCOM_WLED
> > >         If you have the Qualcomm PMIC, say Y to enable a driver for the
> > >         WLED block. Currently it supports PM8941 and PMI8998.
> > >
> > > +config BACKLIGHT_RT4831
> > > +     tristate "Richtek RT4831 Backlight Driver"
> > > +     depends on MFD_RT4831
> > > +     help
> > > +       This enables support for Richtek RT4831 Backlight driver.
> > > +       It's common used to drive the display WLED. There're four channels
> > > +       inisde, and each channel can provide up to 30mA current.
> > > +
> > >  config BACKLIGHT_SAHARA
> > >       tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
> > >       depends on X86
> > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > > index 685f3f1..cae2c83 100644
> > > --- a/drivers/video/backlight/Makefile
> > > +++ b/drivers/video/backlight/Makefile
> > > @@ -49,6 +49,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA)             += pandora_bl.o
> > >  obj-$(CONFIG_BACKLIGHT_PCF50633)     += pcf50633-backlight.o
> > >  obj-$(CONFIG_BACKLIGHT_PWM)          += pwm_bl.o
> > >  obj-$(CONFIG_BACKLIGHT_QCOM_WLED)    += qcom-wled.o
> > > +obj-$(CONFIG_BACKLIGHT_RT4831)               += rt4831-backlight.o
> > >  obj-$(CONFIG_BACKLIGHT_SAHARA)               += kb3886_bl.o
> > >  obj-$(CONFIG_BACKLIGHT_SKY81452)     += sky81452-backlight.o
> > >  obj-$(CONFIG_BACKLIGHT_TOSA)         += tosa_bl.o
> > > diff --git a/drivers/video/backlight/rt4831-backlight.c b/drivers/video/backlight/rt4831-backlight.c
> > > new file mode 100644
> > > index 00000000..42155c7
> > > --- /dev/null
> > > +++ b/drivers/video/backlight/rt4831-backlight.c
> > > @@ -0,0 +1,203 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#include <dt-bindings/leds/rt4831-backlight.h>
> > > +#include <linux/backlight.h>
> > > +#include <linux/bitops.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/property.h>
> > > +#include <linux/regmap.h>
> > > +
> > > +#define RT4831_REG_BLCFG     0x02
> > > +#define RT4831_REG_BLDIML    0x04
> > > +#define RT4831_REG_ENABLE    0x08
> > > +
> > > +#define RT4831_BLMAX_BRIGHTNESS      2048
> > > +
> > > +#define RT4831_BLOVP_MASK    GENMASK(7, 5)
> > > +#define RT4831_BLOVP_SHIFT   5
> > > +#define RT4831_BLPWMEN_MASK  BIT(0)
> > > +#define RT4831_BLEN_MASK     BIT(4)
> > > +#define RT4831_BLCH_MASK     GENMASK(3, 0)
> > > +#define RT4831_BLDIML_MASK   GENMASK(2, 0)
> > > +#define RT4831_BLDIMH_MASK   GENMASK(10, 3)
> > > +#define RT4831_BLDIMH_SHIFT  3
> > > +
> > > +struct rt4831_priv {
> > > +     struct device *dev;
> > > +     struct regmap *regmap;
> > > +     struct backlight_device *bl;
> > > +};
> > > +
> > > +static int rt4831_bl_update_status(struct backlight_device *bl_dev)
> > > +{
> > > +     struct rt4831_priv *priv = bl_get_data(bl_dev);
> > > +     int brightness = backlight_get_brightness(bl_dev);
> > > +     unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
> > > +     u8 v[2];
> > > +     int ret;
> > > +
> > > +     if (brightness) {
> > > +             v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
> > > +             v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
> > > +
> > > +             ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> > > +             if (ret)
> > > +                     return ret;
> > > +     }
> > > +
> > > +     return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
> > > +
> > > +}
> > > +
> > > +static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
> > > +{
> > > +     struct rt4831_priv *priv = bl_get_data(bl_dev);
> > > +     unsigned int val;
> > > +     u8 v[2];
> > > +     int ret;
> > > +
> > > +     ret = regmap_read(priv->regmap, RT4831_REG_ENABLE, &val);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     if (!(val & RT4831_BLEN_MASK))
> > > +             return 0;
> > > +
> > > +     ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static const struct backlight_ops rt4831_bl_ops = {
> > > +     .options = BL_CORE_SUSPENDRESUME,
> > > +     .update_status = rt4831_bl_update_status,
> > > +     .get_brightness = rt4831_bl_get_brightness,
> > > +};
> > > +
> > > +static int rt4831_parse_backlight_properties(struct rt4831_priv *priv,
> > > +                                          struct backlight_properties *bl_props)
> > > +{
> > > +     struct device *dev = priv->dev;
> > > +     u8 propval;
> > > +     u32 brightness;
> > > +     unsigned int val = 0;
> > > +     int ret;
> > > +
> > > +     /* common properties */
> > > +     ret = device_property_read_u32(dev, "max-brightness", &brightness);
> > > +     if (ret)
> > > +             brightness = RT4831_BLMAX_BRIGHTNESS;
> > > +
> > > +     bl_props->max_brightness = min_t(u32, brightness, RT4831_BLMAX_BRIGHTNESS);
> > > +
> > > +     ret = device_property_read_u32(dev, "default-brightness", &brightness);
> > > +     if (ret)
> > > +             brightness = bl_props->max_brightness;
> > > +
> > > +     bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
> > > +
> > > +     /* vendor properties */
> > > +     if (device_property_read_bool(dev, "richtek,pwm-enable"))
> > > +             val = RT4831_BLPWMEN_MASK;
> > > +
> > > +     ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
> > > +     if (ret)
> > > +             propval = RT4831_BLOVPLVL_21V;
> > > +
> > > +     propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
> > > +     ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
> > > +                              propval << RT4831_BLOVP_SHIFT);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
> > > +     if (ret) {
> > > +             dev_err(dev, "richtek,channel-use DT property missing\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     if (!(propval & RT4831_BLCH_MASK)) {
> > > +             dev_err(dev, "No channel specified\n");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLCH_MASK, propval);
> > > +}
> > > +
> > > +static int rt4831_bl_probe(struct platform_device *pdev)
> > > +{
> > > +     struct rt4831_priv *priv;
> > > +     struct backlight_properties bl_props = { .type = BACKLIGHT_RAW,
> > > +                                              .scale = BACKLIGHT_SCALE_LINEAR };
> > > +     int ret;
> > > +
> > > +     priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > > +     if (!priv)
> > > +             return -ENOMEM;
> > > +
> > > +     priv->dev = &pdev->dev;
> > > +
> > > +     priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > +     if (!priv->regmap) {
> > > +             dev_err(&pdev->dev, "Failed to init regmap\n");
> > > +             return -ENODEV;
> > > +     }
> > > +
> > > +     ret = rt4831_parse_backlight_properties(priv, &bl_props);
> > > +     if (ret) {
> > > +             dev_err(&pdev->dev, "Failed to parse backlight properties\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, priv,
> > > +                                               &rt4831_bl_ops, &bl_props);
> > > +     if (IS_ERR(priv->bl)) {
> > > +             dev_err(&pdev->dev, "Failed to register backlight\n");
> > > +             return PTR_ERR(priv->bl);
> > > +     }
> > > +
> > > +     backlight_update_status(priv->bl);
> > > +     platform_set_drvdata(pdev, priv);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rt4831_bl_remove(struct platform_device *pdev)
> > > +{
> > > +     struct rt4831_priv *priv = platform_get_drvdata(pdev);
> > > +     struct backlight_device *bl_dev = priv->bl;
> > > +
> > > +     bl_dev->props.brightness = 0;
> > > +     backlight_update_status(priv->bl);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static const struct of_device_id __maybe_unused rt4831_bl_of_match[] = {
> > > +     { .compatible = "richtek,rt4831-backlight", },
> > > +     {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, rt4831_bl_of_match);
> > > +
> > > +static struct platform_driver rt4831_bl_driver = {
> > > +     .driver = {
> > > +             .name = "rt4831-backlight",
> > > +             .of_match_table = rt4831_bl_of_match,
> > > +     },
> > > +     .probe = rt4831_bl_probe,
> > > +     .remove = rt4831_bl_remove,
> > > +};
> > > +module_platform_driver(rt4831_bl_driver);
> > > +
> > > +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> > > +MODULE_LICENSE("GPL v2");
> > > --
> > > 2.7.4
> > >

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

* Re: [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight
  2021-04-26  7:18 ` [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight cy_huang
@ 2021-04-30 20:10   ` Rob Herring
  2021-05-04  1:10     ` ChiYuan Huang
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-04-30 20:10 UTC (permalink / raw)
  To: cy_huang
  Cc: lee.jones, lgirdwood, broonie, daniel.thompson, jingoohan1,
	b.zolnierkie, pavel, dri-devel, linux-fbdev, linux-kernel,
	linux-leds, devicetree, cy_huang

On Mon, Apr 26, 2021 at 03:18:09PM +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Adds DT binding document for Richtek RT4831 backlight.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> Resend this v6 patch series to loop devicetree reviewers.
> 
> For next, need to add the below line
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  .../leds/backlight/richtek,rt4831-backlight.yaml   | 65 ++++++++++++++++++++++
>  include/dt-bindings/leds/rt4831-backlight.h        | 23 ++++++++
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
>  create mode 100644 include/dt-bindings/leds/rt4831-backlight.h
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
> new file mode 100644
> index 00000000..4da6a66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/richtek,rt4831-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT4831 Backlight
> +
> +maintainers:
> +  - ChiYuan Huang <cy_huang@richtek.com>
> +
> +description: |
> +  RT4831 is a mutifunctional device that can provide power to the LCD display
> +  and LCD backlight.
> +
> +  For the LCD backlight, it can provide four channel WLED driving capability.
> +  Each channel driving current is up to 30mA
> +
> +  Datasheet is available at
> +  https://www.richtek.com/assets/product_file/RT4831A/DS4831A-05.pdf
> +

Need to reference common backlight schema:

allOf:
  - $ref: common.yaml#

> +properties:
> +  compatible:
> +    const: richtek,rt4831-backlight
> +
> +  default-brightness:
> +    description: |
> +      The default brightness that applied to the system on start-up.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Drop description and $ref. Covered in common.yaml.

> +    minimum: 0
> +    maximum: 2048
> +
> +  max-brightness:
> +    description: |
> +      The max brightness for the H/W limit
> +    $ref: /schemas/types.yaml#/definitions/uint32

And here.

> +    minimum: 0
> +    maximum: 2048
> +
> +  richtek,pwm-enable:
> +    description: |
> +      Specify the backlight dimming following by PWM duty or by SW control.
> +    type: boolean
> +
> +  richtek,bled-ovp-sel:
> +    description: |
> +      Backlight OVP level selection, currently support 17V/21V/25V/29V.
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    default: 1
> +    minimum: 0
> +    maximum: 3
> +
> +  richtek,channel-use:
> +    description: |
> +      Backlight LED channel to be used.
> +      BIT 0/1/2/3 is used to indicate led channel 1/2/3/4 enable or disable.
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 1
> +    maximum: 15
> +
> +required:
> +  - compatible
> +  - richtek,channel-use
> +
> +additionalProperties: false
> diff --git a/include/dt-bindings/leds/rt4831-backlight.h b/include/dt-bindings/leds/rt4831-backlight.h
> new file mode 100644
> index 00000000..125c635
> --- /dev/null
> +++ b/include/dt-bindings/leds/rt4831-backlight.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * This header provides constants for rt4831 backlight bindings.
> + *
> + * Copyright (C) 2020, Richtek Technology Corp.
> + * Author: ChiYuan Huang <cy_huang@richtek.com>
> + */
> +
> +#ifndef _DT_BINDINGS_RT4831_BACKLIGHT_H
> +#define _DT_BINDINGS_RT4831_BACKLIGHT_H
> +
> +#define RT4831_BLOVPLVL_17V	0
> +#define RT4831_BLOVPLVL_21V	1
> +#define RT4831_BLOVPLVL_25V	2
> +#define RT4831_BLOVPLVL_29V	3
> +
> +#define RT4831_BLED_CH1EN	(1 << 0)
> +#define RT4831_BLED_CH2EN	(1 << 1)
> +#define RT4831_BLED_CH3EN	(1 << 2)
> +#define RT4831_BLED_CH4EN	(1 << 3)
> +#define RT4831_BLED_ALLCHEN	((1 << 4) - 1)
> +
> +#endif /* _DT_BINDINGS_RT4831_BACKLIGHT_H */
> -- 
> 2.7.4
> 

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

* Re: [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831
  2021-04-26  7:18 ` [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831 cy_huang
@ 2021-04-30 20:10   ` Rob Herring
  2021-05-04  1:04     ` ChiYuan Huang
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-04-30 20:10 UTC (permalink / raw)
  To: cy_huang
  Cc: b.zolnierkie, linux-kernel, daniel.thompson, lgirdwood,
	linux-leds, pavel, lee.jones, linux-fbdev, cy_huang, jingoohan1,
	devicetree, broonie, dri-devel, robh+dt

On Mon, 26 Apr 2021 15:18:10 +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Adds DT binding document for Richtek RT4831.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
> Resend this v6 patch series to loop devicetree reviewers.
> ---
>  .../devicetree/bindings/mfd/richtek,rt4831.yaml    | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831
  2021-04-30 20:10   ` Rob Herring
@ 2021-05-04  1:04     ` ChiYuan Huang
  0 siblings, 0 replies; 11+ messages in thread
From: ChiYuan Huang @ 2021-05-04  1:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: b.zolnierkie, lkml, Daniel Thompson, lgirdwood,
	Linux LED Subsystem, Pavel Machek, Lee Jones, linux-fbdev,
	cy_huang, jingoohan1,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Mark Brown, dri-devel, Rob Herring

HI,

Rob Herring <robh@kernel.org> 於 2021年5月1日 週六 上午4:10寫道:
>
> On Mon, 26 Apr 2021 15:18:10 +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > Adds DT binding document for Richtek RT4831.
> >
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> > Resend this v6 patch series to loop devicetree reviewers.
> > ---
> >  .../devicetree/bindings/mfd/richtek,rt4831.yaml    | 90 ++++++++++++++++++++++
> >  1 file changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml
> >
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Will merge in next v7. Thanks.

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

* Re: [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight
  2021-04-30 20:10   ` Rob Herring
@ 2021-05-04  1:10     ` ChiYuan Huang
  0 siblings, 0 replies; 11+ messages in thread
From: ChiYuan Huang @ 2021-05-04  1:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, lgirdwood, Mark Brown, Daniel Thompson, jingoohan1,
	b.zolnierkie, Pavel Machek, dri-devel, linux-fbdev, lkml,
	Linux LED Subsystem,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	cy_huang

Hi,

Rob Herring <robh@kernel.org> 於 2021年5月1日 週六 上午4:10寫道:
>
> On Mon, Apr 26, 2021 at 03:18:09PM +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > Adds DT binding document for Richtek RT4831 backlight.
> >
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> > Resend this v6 patch series to loop devicetree reviewers.
> >
> > For next, need to add the below line
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> >  .../leds/backlight/richtek,rt4831-backlight.yaml   | 65 ++++++++++++++++++++++
> >  include/dt-bindings/leds/rt4831-backlight.h        | 23 ++++++++
> >  2 files changed, 88 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
> >  create mode 100644 include/dt-bindings/leds/rt4831-backlight.h
> >
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
> > new file mode 100644
> > index 00000000..4da6a66
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/richtek,rt4831-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Richtek RT4831 Backlight
> > +
> > +maintainers:
> > +  - ChiYuan Huang <cy_huang@richtek.com>
> > +
> > +description: |
> > +  RT4831 is a mutifunctional device that can provide power to the LCD display
> > +  and LCD backlight.
> > +
> > +  For the LCD backlight, it can provide four channel WLED driving capability.
> > +  Each channel driving current is up to 30mA
> > +
> > +  Datasheet is available at
> > +  https://www.richtek.com/assets/product_file/RT4831A/DS4831A-05.pdf
> > +
>
> Need to reference common backlight schema:
>
> allOf:
>   - $ref: common.yaml#
>
> > +properties:
> > +  compatible:
> > +    const: richtek,rt4831-backlight
> > +
> > +  default-brightness:
> > +    description: |
> > +      The default brightness that applied to the system on start-up.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> Drop description and $ref. Covered in common.yaml.
>
OK
> > +    minimum: 0
> > +    maximum: 2048
> > +
> > +  max-brightness:
> > +    description: |
> > +      The max brightness for the H/W limit
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> And here.
>
OK

The above changes will be merged in next, thanks.
After mfd code patch reviewed, I'll send v7 for all of review comments.
> > +    minimum: 0
> > +    maximum: 2048
> > +
> > +  richtek,pwm-enable:
> > +    description: |
> > +      Specify the backlight dimming following by PWM duty or by SW control.
> > +    type: boolean
> > +
> > +  richtek,bled-ovp-sel:
> > +    description: |
> > +      Backlight OVP level selection, currently support 17V/21V/25V/29V.
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    default: 1
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +  richtek,channel-use:
> > +    description: |
> > +      Backlight LED channel to be used.
> > +      BIT 0/1/2/3 is used to indicate led channel 1/2/3/4 enable or disable.
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    minimum: 1
> > +    maximum: 15
> > +
> > +required:
> > +  - compatible
> > +  - richtek,channel-use
> > +
> > +additionalProperties: false
> > diff --git a/include/dt-bindings/leds/rt4831-backlight.h b/include/dt-bindings/leds/rt4831-backlight.h
> > new file mode 100644
> > index 00000000..125c635
> > --- /dev/null
> > +++ b/include/dt-bindings/leds/rt4831-backlight.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * This header provides constants for rt4831 backlight bindings.
> > + *
> > + * Copyright (C) 2020, Richtek Technology Corp.
> > + * Author: ChiYuan Huang <cy_huang@richtek.com>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_RT4831_BACKLIGHT_H
> > +#define _DT_BINDINGS_RT4831_BACKLIGHT_H
> > +
> > +#define RT4831_BLOVPLVL_17V  0
> > +#define RT4831_BLOVPLVL_21V  1
> > +#define RT4831_BLOVPLVL_25V  2
> > +#define RT4831_BLOVPLVL_29V  3
> > +
> > +#define RT4831_BLED_CH1EN    (1 << 0)
> > +#define RT4831_BLED_CH2EN    (1 << 1)
> > +#define RT4831_BLED_CH3EN    (1 << 2)
> > +#define RT4831_BLED_CH4EN    (1 << 3)
> > +#define RT4831_BLED_ALLCHEN  ((1 << 4) - 1)
> > +
> > +#endif /* _DT_BINDINGS_RT4831_BACKLIGHT_H */
> > --
> > 2.7.4
> >

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

end of thread, other threads:[~2021-05-04  1:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26  7:18 [RESEND PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831 cy_huang
2021-04-26  7:18 ` [RESEND PATCH v6 2/4] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight cy_huang
2021-04-30 20:10   ` Rob Herring
2021-05-04  1:10     ` ChiYuan Huang
2021-04-26  7:18 ` [RESEND PATCH v6 3/4] mfd: rt4831: Adds DT binding document for Richtek RT4831 cy_huang
2021-04-30 20:10   ` Rob Herring
2021-05-04  1:04     ` ChiYuan Huang
2021-04-26  7:18 ` [RESEND PATCH v6 4/4] backlight: rt4831: Adds support for Richtek RT4831 backlight cy_huang
2021-04-26 10:19   ` Daniel Thompson
2021-04-26 10:50     ` ChiYuan Huang
2021-04-26 12:24       ` Daniel Thompson

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