mfd: rt4831: Adds support for Richtek RT4831 MFD core

PATCH Superseded cy_huang 5 participants 17 messages Dec 11, 2020 – Dec 16, 2020

Subsystems (4)

Device tree bindings Multifunction devices (MFD) LED Backlight class/subsystem

lore

Share & tools

lore

cy_huang

author

2020-12-11 17:37 UTC #0 lore raw

PATCH v4 1/3 mfd: rt4831: Adds support for Richtek RT4831 MFD core

From: ChiYuan Huang <[email protected]>

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

Signed-off-by ChiYuan Huang <[email protected]>

---
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-core.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831-core.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,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

drivers/mfd/Makefile +1 -0

index 1780019..4108141 100644
@@ -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-core.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o

drivers/mfd/rt4831-core.c new file +124 -0

124 lines not on this page · raw message

Signature · 1 line

2.7.4

cy_huang

author

2020-12-11 17:37 UTC #1 lore raw

PATCH v4 2/3 backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

From: ChiYuan Huang <[email protected]>

Adds DT binding document for Richtek RT4831 backlight.

Signed-off-by ChiYuan Huang <[email protected]>

---
since v3
- Move inlcude/dt-bindings/leds/rt4831-backlight.h from v3 mfd binding patch to here.
- Add dual license tag in header and backlight binding document.
- Left backlight dt-binding example only.

---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +76 -0

new file mode 100644
index 00000000..f24c8d1
@@ -0,0 +1,76 @@
+# 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 <[email protected]>
+
+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
+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;
+    };

include/dt-bindings/leds/rt4831-backlight.h new file +23 -0

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#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 */

Signature · 1 line

2.7.4

Daniel Thompson

2020-12-14 10:00 UTC #2 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Hi CY

Quoted 5 lines · cy_huang

From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Signed-off-by ChiYuan Huang <[email protected]>

This patch got keyword filtered and brought to my attention but the rest of the series did not.

If it was a backlight patch series you need to send it To: the all the backlight maintainers.

Daniel.

Quoted 126 lines

---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +76 -0

new file mode 100644
index 00000000..f24c8d1
@@ -0,0 +1,76 @@
+# 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 <[email protected]>
+
+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
+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;
+    };

include/dt-bindings/leds/rt4831-backlight.h new file +23 -0

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#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 */

Signature · 1 line

2.7.4

ChiYuan Huang

author

2020-12-14 14:41 UTC #3 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Hi,

Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：

Quoted 8 lines

Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.

Quoted 8 lines

Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.

Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.

I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?

Quoted 130 lines

Daniel.

Quoted 126 lines

---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +76 -0

new file mode 100644
index 00000000..f24c8d1
@@ -0,0 +1,76 @@
+# 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 <[email protected]>
+
+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
+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;
+    };

include/dt-bindings/leds/rt4831-backlight.h new file +23 -1

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#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

Daniel Thompson

2020-12-14 15:15 UTC #4 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Quoted 19 lines · ChiYuan Huang

Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Quoted 8 lines
Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.

Quoted 8 lines

Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.
I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?

To some extent it's up to you.

I think I would have shared all the pieces at once (although not it Lee, as mfd maintainer, had suggested otherwise).

Daniel.

Quoted 130 lines

Daniel.

Quoted 126 lines

---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +76 -0

new file mode 100644
index 00000000..f24c8d1
@@ -0,0 +1,76 @@
+# 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 <[email protected]>
+
+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
+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;
+    };

include/dt-bindings/leds/rt4831-backlight.h new file +23 -1

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#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

Lee Jones

2020-12-15 07:53 UTC #5 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Quoted 26 lines · Daniel Thompson

Quoted 25 lines · ChiYuan Huang
Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Quoted 8 lines
Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.
Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.

Quoted 7 lines

Quoted 2 lines
I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?
To some extent it's up to you.
I think I would have shared all the pieces at once (although not it Lee, as mfd maintainer, had suggested otherwise).

You should not need to concern yourself with patch ordering outside of the realms of the set i.e. [PATCH 1/x], [PATCH 2/x], etc.

If you just send the whole patch set and you do not specify otherwise, it will be applied, in order, as a set.

Sending subsystem patches without the correct maintainers as recipients is bad form. Many of us have filters on, so this tactic will seldom work in any case.

Signature · 4 lines

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

ChiYuan Huang

author

2020-12-15 09:09 UTC #6 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Hi, Lee:

Lee Jones <[email protected]> 於 2020年12月15日週二下午3:53寫道：

Quoted 40 lines · Daniel Thompson

Quoted 27 lines · ChiYuan Huang
Quoted 25 lines
Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Quoted 8 lines
Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.
Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.
Quoted 7 lines
Quoted 2 lines
I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?
To some extent it's up to you.
I think I would have shared all the pieces at once (although not it Lee, as mfd maintainer, had suggested otherwise).
You should not need to concern yourself with patch ordering outside of the realms of the set i.e. [PATCH 1/x], [PATCH 2/x], etc.

Quoted 7 lines

If you just send the whole patch set and you do not specify otherwise, it will be applied, in order, as a set.
Sending subsystem patches without the correct maintainers as recipients is bad form. Many of us have filters on, so this tactic will seldom work in any case.

In my case, there're mfd/backlight/regulator for RT4831.
You mean I can just send the whole patch set directly to whole mfd/backlight/regulator maintainers.
And you can filter like as the keyword to review the related contents, right?

From my original thought, the order is mfd -> backlight-> regulator, one by one due to different maintainers.
Maybe I think too much about the patch ordering

If so, after getting the comment from Rob, I'll send the whole patch to you. Thanks for the notice.

Quoted 5 lines

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

Lee Jones

2020-12-15 09:24 UTC #7 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Quoted 56 lines · ChiYuan Huang

Hi, Lee:
Lee Jones <[email protected]> 於 2020年12月15日週二下午3:53寫道：
Quoted 40 lines · Daniel Thompson
Quoted 27 lines · ChiYuan Huang
Quoted 25 lines
Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Quoted 8 lines
Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.
Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.
Quoted 7 lines
Quoted 2 lines
I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?
To some extent it's up to you.
I think I would have shared all the pieces at once (although not it Lee, as mfd maintainer, had suggested otherwise).
You should not need to concern yourself with patch ordering outside of the realms of the set i.e. [PATCH 1/x], [PATCH 2/x], etc.
Quoted 7 lines
If you just send the whole patch set and you do not specify otherwise, it will be applied, in order, as a set.
Sending subsystem patches without the correct maintainers as recipients is bad form. Many of us have filters on, so this tactic will seldom work in any case.
In my case, there're mfd/backlight/regulator for RT4831.
You mean I can just send the whole patch set directly to whole mfd/backlight/regulator maintainers.
And you can filter like as the keyword to review the related contents, right?

Quoted 6 lines

From my original thought, the order is mfd -> backlight-> regulator, one by one due to different maintainers.
Maybe I think too much about the patch ordering
If so, after getting the comment from Rob, I'll send the whole patch to you. Thanks for the notice.

Simply send them all as a single patch-set. It's a good idea to add all maintainers to all patches. We will then coordinate amongst ourselves and come up with the best merge strategy.

Signature · 4 lines

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

Rob Herring

maintainer

2020-12-14 23:18 UTC #8 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

In reply to ChiYuan Huang

Quoted 19 lines · ChiYuan Huang

Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 4 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Quoted 8 lines
Quoted 1 line
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.

Quoted 8 lines

Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.
I still have backlight/regulator source code patch after mfd reviewing. Do you want me to send all the patches without waiting for mfd reviewing?

What happened to the regulator part of the binding? I said you could merge it into the mfd schema, not drop it. Bindings should be complete so we get a full picture of a device.

Rob

cy_huang(黃啟原)

2020-12-15 05:22 UTC #9 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Quoted 28 lines · ChiYuan Huang

Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 3 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Quoted 9 lines
Quoted 3 lines
Adds DT binding document for Richtek RT4831 backlight.
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.
Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.

Quoted 4 lines

I still have backlight/regulator source code patch after mfd reviewing.
Do you want me to send all the patches without waiting for mfd reviewing?

What happened to the regulator part of the binding? I said you could merge it into the mfd schema, not drop it. Bindings should be complete so we get a full picture of a device.

Yes, I remove the regulator dt-binding and directly merge into mfd schema. Could you check the v4 3/3 patch?
Or you just want me to remove regulator dt-binding example in regulator dt-binding patch?

Sorry, please also loop my gmail account.
Due to my company mailbox, I'm not sure whether the reply format is correct or not.

Rob
************* Email Confidentiality Notice ********************

The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!

ChiYuan Huang

author

2020-12-15 05:18 UTC #10 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

cy_huang(黃啟原) <[email protected]> 於 2020年12月15日週二下午1:08寫道：

Quoted 30 lines · ChiYuan Huang

Quoted 28 lines
Hi,
Daniel Thompson <[email protected]> 於 2020年12月14日週一下午5:59寫道：
Quoted 8 lines
Hi CY
Quoted 3 lines · cy_huang
From: ChiYuan Huang <[email protected]>
Quoted 9 lines
Quoted 3 lines
Adds DT binding document for Richtek RT4831 backlight.
Signed-off-by ChiYuan Huang <[email protected]>
This patch got keyword filtered and brought to my attention but the rest of the series did not.
If it was a backlight patch series you need to send it To: the all the backlight maintainers.
Yes, I'm waiting for mfd reviewing.
Due to mfd patch, I need to add backlight dt-binding patch prior to backlight source code.
Or autobuild robot will said mfd dt-binding build fail from Rob. That's why I send the backlight dt-binding prior to the source code.

Quoted 8 lines

Quoted 4 lines
I still have backlight/regulator source code patch after mfd reviewing.
Do you want me to send all the patches without waiting for mfd reviewing?
What happened to the regulator part of the binding? I said you could merge it into the mfd schema, not drop it. Bindings should be complete so we get a full picture of a device.

Sorry I found the gmail account already loop in. Just my gmail problem. I cannot see the email in it.....
The reply is below.
Yes, I remove the regulator dt-binding and directly merge into mfd schema. Could you check the v4 3/3 patch?
Or you just want me to remove regulator dt-binding example, not whole dt-binding file?

Quoted 5 lines

Rob
************* Email Confidentiality Notice ********************
The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!

Rob Herring

maintainer

2020-12-14 23:19 UTC #11 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

In reply to cy_huang

Quoted 88 lines · cy_huang

From: ChiYuan Huang <[email protected]>

Adds DT binding document for Richtek RT4831 backlight.

Signed-off-by ChiYuan Huang <[email protected]>

---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76 ++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml
 create mode 100644 include/dt-bindings/leds/rt4831-backlight.h

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +65 -0

new file mode 100644
index 00000000..f24c8d1
@@ -0,0 +1,76 @@
+# 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 <[email protected]>
+
+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

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml line 75

Quoted 10 lines

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +10 -0

+
+examples:
+  - |
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;

This is in the MFD schema already, so drop the example.

Quoted 33 lines

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +1 -0

+    };

include/dt-bindings/leds/rt4831-backlight.h new file +23 -0

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#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 */

Signature · 1 line

2.7.4

cy_huang(黃啟原)

2020-12-15 05:22 UTC #12 lore raw

Re: [PATCH v4 2/3] backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight

Quoted 112 lines · cy_huang

Quoted 110 lines
From: ChiYuan Huang <[email protected]>
Adds DT binding document for Richtek RT4831 backlight.
Signed-off-by ChiYuan Huang <[email protected]>
---
since v3
- Move inlcude/dt-bindings/leds/rt4831-backlight.h from v3 mfd binding patch to here.
- Add dual license tag in header and backlight binding document.
- Left backlight dt-binding example only.
---
 .../leds/backlight/richtek,rt4831-backlight.yaml   | 76
++++++++++++++++++++++
 include/dt-bindings/leds/rt4831-backlight.h        | 23 +++++++
 2 files changed, 99 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..f24c8d1
--- /dev/null
+++
b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831- backlight.yaml
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +4 -0
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/richtek,rt4831-b
acklight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +8 -0
+
+title: Richtek RT4831 Backlight
+
+maintainers:
+  - ChiYuan Huang <[email protected]>
+
+description: |
+  RT4831 is a mutifunctional device that can provide power to the
LCD display
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +3 -0
+  and LCD backlight.
+
+  For the LCD backlight, it can provide four channel WLED driving
capability.
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +4 -0
+  Each channel driving current is up to 30mA
+
+  Datasheet is available at
+  https://www.richtek.com/assets/product_file/RT4831A/DS4831A-05.p
df
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +8 -0
+
+properties:
+  compatible:
+    const: richtek,rt4831-backlight
+
+  default-brightness:
+    description: |
+      The default brightness that applied to the system on start-
up.
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +14 -0
+    $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.
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +5 -0
+    type: boolean
+
+  richtek,bled-ovp-sel:
+    description: |
+      Backlight OVP level selection, currently support
17V/21V/25V/29V.
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +9 -0
+    $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.
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +12 -0
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 1
+    maximum: 15
+
+required:
+  - compatible
+  - richtek,channel-use
+
+additionalProperties: false
+
+examples:
+  - |

Quoted 9 lines

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml line 75
Quoted 7 lines
Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +7 -0
+    #include <dt-bindings/leds/rt4831-backlight.h>
+    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>;
This is in the MFD schema already, so drop the example.

Will ack in next series patch.

Quoted 24 lines

Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml new file +1 -0

+    };

include/dt-bindings/leds/rt4831-backlight.h new file +15 -0

new file mode 100644
index 00000000..125c635
@@ -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 <[email protected]>
+ */
+
+#ifndef _DT_BINDINGS_RT4831_BACKLIGHT_H
+#define _DT_BINDINGS_RT4831_BACKLIGHT_H
+
+#define RT4831_BLOVPLVL_17V0
+#define RT4831_BLOVPLVL_21V1
+#define RT4831_BLOVPLVL_25V2
+#define RT4831_BLOVPLVL_29V3

Quoted 8 lines

include/dt-bindings/leds/rt4831-backlight.h line 23
Quoted 8 lines
include/dt-bindings/leds/rt4831-backlight.h new file +8 -0
+
+#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 */

************* Email Confidentiality Notice ********************

cy_huang

author

2020-12-11 17:37 UTC #13 lore raw

PATCH v4 3/3 mfd: rt4831: Adds DT binding document for Richtek RT4831 MFD core

In reply to cy_huang

From: ChiYuan Huang <[email protected]>

Adds DT binding document for Richtek RT4831 MFD core.

Signed-off-by ChiYuan Huang <[email protected]>

---
This patch depends on
"backlight: rt4831: Adds DT binding document for Richtek RT4831 backlight".

since v4
- remove v3 regulator binding patch, directly merge it into mfd binding.

since v3
- Move include/dt-bindings/leds/rt4831-backlight.h to patch 0002.
- Add dual license tag in mfd binding document.

since v2
- Add regulator-allow-bypass flag in DSVLCM.

---
 .../devicetree/bindings/mfd/richtek,rt4831.yaml    | 108 +++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml

Documentation/devicetree/bindings/mfd/richtek,rt4831.yaml new file +108 -0

new file mode 100644
index 00000000..9de7c4a
@@ -0,0 +1,108 @@
+# 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 <[email protected]>
+
+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.
+
+  DSVLCM is a boost regulator in IC internal as DSVP and DSVN input power.
+  Its voltage should be configured above 0.15V to 0.2V gap larger than the
+  voltage needed for DSVP and DSVN. Too much voltage gap could improve the
+  voltage drop from the heavy loading scenario. But it also make the power
+  efficiency worse. It's a trade-off.
+
+  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:
+    type: object
+
+    patternProperties:
+      "^DSV(LCM|P|N)$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+
+    required:
+      - DSVLCM
+      - DSVP
+      - DSVN
+
+    additionalProperties: false
+
+  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>;
+        };
+      };
+    };

Signature · 1 line

2.7.4

Lee Jones

2020-12-16 14:13 UTC #14 lore raw

In reply to cy_huang

Quoted 3 lines · cy_huang

From: ChiYuan Huang <[email protected]>
This adds support Richtek RT4831 MFD core. It includes four channel WLED driver

Drop mentions of MFD. Just core driver will do.

Quoted 42 lines

and Display Bias Voltage outputs.

Signed-off-by ChiYuan Huang <[email protected]>

---
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-core.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831-core.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,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

drivers/mfd/Makefile +0 -0

index 1780019..4108141 100644

drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 238

Quoted 4 lines

drivers/mfd/Makefile +1 -0

@@ -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-core.o

Why is this called -core ...

drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 239

Quoted 1 line

drivers/mfd/Makefile +0 -0
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o

... and this isn't?

Quoted 25 lines

drivers/mfd/Makefile +0 -0

 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o

drivers/mfd/rt4831-core.c new file +17 -0

new file mode 100644
index 00000000..f837c06
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.
+ *
+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#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

drivers/mfd/rt4831-core.c line 27

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+
+#define RICHTEK_VID		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[] = {
+	OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+	MFD_CELL_NAME("rt4831-regulator")
+};

Just a little note about these helpers. I'm planning on unifying the names pretty soon. So if you have to rebase, please watch out for the rename.

Essentially OF_MFD_CELL() will soon become MFD_CELL_OF().

Quoted 9 lines

drivers/mfd/rt4831-core.c new file +9 -0

+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,

drivers/mfd/rt4831-core.c line 47

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+	.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;

My preference would be "enable_gpio" to ensure it's easily identifiable further on.

drivers/mfd/rt4831-core.c line 49

Quoted 2 lines

drivers/mfd/rt4831-core.c new file +2 -0
+	struct regmap *regmap;
+	unsigned int val;

'val' is not a great name for a variable that is used for a specific purpose. How about 'revision'?

Quoted 11 lines

drivers/mfd/rt4831-core.c new file +11 -0

+	int ret;
+
+	enable = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(enable)) {
+		dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+		return PTR_ERR(enable);
+	}
+
+	regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Failed to initialize regmap\n");

drivers/mfd/rt4831-core.c line 70

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+		return PTR_ERR(regmap);
+	}
+ 
+	ret = regmap_read(regmap, RT4831_REG_REVISION, &val);
+	if (ret) {
+		dev_err(&client->dev, "Failed to get H/W revision\n");
+		return ret;
+	}
+
+	if ((val & RT4831_VID_MASK) != RICHTEK_VID) {

What does VID stand for here?

The fact that I have to ask probably means it should be changed.

Variant ID perhaps? If so, call the define 'VARIANT_ID'.

drivers/mfd/rt4831-core.c line 71

Quoted 1 line

drivers/mfd/rt4831-core.c new file +1 -0
+		dev_err(&client->dev, "VID not matched, val = 0x%02x\n", val);

Using variable names means nothing to the user.

"revision: 0x%02x" ?

Quoted 34 lines

drivers/mfd/rt4831-core.c new file +34 -0

+		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 void rt4831_shutdown(struct i2c_client *client)
+{
+	struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+	/* Disable WLED and DSV outputs */
+	regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}
+

drivers/mfd/rt4831-core.c line 115

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+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 = of_match_ptr(rt4831_of_match),

The trend is to not use of_match_ptr() anymore, as these devices can not be instantiated using ACPI.

Quoted 9 lines

drivers/mfd/rt4831-core.c new file +9 -0

+	},
+	.probe_new = rt4831_probe,
+	.remove = rt4831_remove,
+	.shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");

Signature · 4 lines

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

ChiYuan Huang

author

2020-12-16 14:50 UTC #15 lore raw

Lee Jones <[email protected]> 於 2020年12月16日週三下午10:12寫道：

Quoted 58 lines · cy_huang

Quoted 4 lines
From: ChiYuan Huang <[email protected]>
This adds support Richtek RT4831 MFD core. It includes four channel WLED driver
Drop mentions of MFD. Just core driver will do.
Quoted 42 lines
and Display Bias Voltage outputs.
Signed-off-by ChiYuan Huang <[email protected]>
---
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-core.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831-core.c
drivers/mfd/Kconfig +10 -0
index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,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
drivers/mfd/Makefile +0 -0
index 1780019..4108141 100644
drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 238
Quoted 4 lines
drivers/mfd/Makefile +1 -0
@@ -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-core.o
Why is this called -core ...

Quoted 4 lines

drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 239
Quoted 1 line
drivers/mfd/Makefile +0 -0
 obj-$(CONFIG_MFD_RT5033)     += rt5033.o
... and this isn't?

Ok, I'm rename the mfd file to rt4831 only.
Due to this mfd is the parent of all sub device, to use 'core' is trying to distinguish from rt4831-regulator or rt4831-backlight. My original thought is not to let the user confused.
If to add the postfix '-core' in the file name is bad, I think it can be removed.

Quoted 40 lines

Quoted 25 lines

drivers/mfd/Makefile +0 -0

 obj-$(CONFIG_MFD_SKY81452)   += sky81452.o

drivers/mfd/rt4831-core.c new file +17 -0

new file mode 100644
index 00000000..f837c06
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.
+ *
+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#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

drivers/mfd/rt4831-core.c line 27

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+
+#define RICHTEK_VID          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[] = {
+     OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+     MFD_CELL_NAME("rt4831-regulator")
+};

Just a little note about these helpers. I'm planning on unifying the names pretty soon. So if you have to rebase, please watch out for the rename.

Quoted 2 lines

Essentially OF_MFD_CELL() will soon become MFD_CELL_OF().

Yes, I'll check this change. BTW, is it possible to get this patch that I can integrate it into my codebase in advance.

Quoted 14 lines

drivers/mfd/rt4831-core.c new file +14 -0

+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,

Quoted 9 lines

drivers/mfd/rt4831-core.c line 47
Quoted 5 lines
drivers/mfd/rt4831-core.c new file +5 -0
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+     struct gpio_desc *enable;
My preference would be "enable_gpio" to ensure it's easily identifiable further on.

Yes, it make sense.

Quoted 6 lines

drivers/mfd/rt4831-core.c line 49
Quoted 2 lines
drivers/mfd/rt4831-core.c new file +2 -0
+     struct regmap *regmap;
+     unsigned int val;
'val' is not a great name for a variable that is used for a specific purpose. How about 'revision'?

Actually, this is just for the vendor id check.
Maybe to named 'vendor_id' is more suitable.
This register can be divided into two parts, bit[7:4] is the fixed vendor identifier, only bit[3:0] is for the chip revision.

Quoted 24 lines

Quoted 11 lines

drivers/mfd/rt4831-core.c new file +11 -0

+     int ret;
+
+     enable = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+     if (IS_ERR(enable)) {
+             dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+             return PTR_ERR(enable);
+     }
+
+     regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+     if (IS_ERR(regmap)) {
+             dev_err(&client->dev, "Failed to initialize regmap\n");

drivers/mfd/rt4831-core.c line 70

Quoted 10 lines

drivers/mfd/rt4831-core.c new file +10 -0

+             return PTR_ERR(regmap);
+     }
+
+     ret = regmap_read(regmap, RT4831_REG_REVISION, &val);
+     if (ret) {
+             dev_err(&client->dev, "Failed to get H/W revision\n");
+             return ret;
+     }
+
+     if ((val & RT4831_VID_MASK) != RICHTEK_VID) {

What does VID stand for here?

Quoted 4 lines

The fact that I have to ask probably means it should be changed.
Variant ID perhaps? If so, call the define 'VARIANT_ID'.

Like as the the above explaination.
'RICHTEK_VID' may not be good. So it make you confused. I'll rename it to 'RICHTEK_VENDOR_ID'. How about this naming?

Quoted 6 lines

drivers/mfd/rt4831-core.c line 71
Quoted 1 line
drivers/mfd/rt4831-core.c new file +1 -0
+             dev_err(&client->dev, "VID not matched, val = 0x%02x\n", val);
Using variable names means nothing to the user.
"revision: 0x%02x" ?

How about this one "CHIP ID: 0x%02x"?
May I use 'chip_id to replace this variable 'val' naming?

Quoted 39 lines

drivers/mfd/rt4831-core.c new file +39 -0

+             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 void rt4831_shutdown(struct i2c_client *client)
+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     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);

Quoted 9 lines

drivers/mfd/rt4831-core.c line 115
Quoted 5 lines
drivers/mfd/rt4831-core.c new file +5 -0
+
+static struct i2c_driver rt4831_driver = {
+     .driver = {
+             .name = "rt4831",
+             .of_match_table = of_match_ptr(rt4831_of_match),
The trend is to not use of_match_ptr() anymore, as these devices can not be instantiated using ACPI.

OK, I'll remove 'of_match_ptr'.

Quoted 15 lines

drivers/mfd/rt4831-core.c line 124
Quoted 9 lines
drivers/mfd/rt4831-core.c new file +9 -0
+     },
+     .probe_new = rt4831_probe,
+     .remove = rt4831_remove,
+     .shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Lee Jones

2020-12-16 14:59 UTC #16 lore raw

Quoted 111 lines · ChiYuan Huang

Lee Jones <[email protected]> 於 2020年12月16日週三下午10:12寫道：
Quoted 58 lines · cy_huang
Quoted 4 lines
From: ChiYuan Huang <[email protected]>
This adds support Richtek RT4831 MFD core. It includes four channel WLED driver
Drop mentions of MFD. Just core driver will do.
Quoted 42 lines
and Display Bias Voltage outputs.
Signed-off-by ChiYuan Huang <[email protected]>
---
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-core.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831-core.c
drivers/mfd/Kconfig +10 -0
index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,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
drivers/mfd/Makefile +0 -0
index 1780019..4108141 100644
drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 238
Quoted 4 lines
drivers/mfd/Makefile +1 -0
@@ -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-core.o
Why is this called -core ...
Quoted 4 lines
drivers/mfd/Makefile obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o · line 239
Quoted 1 line
drivers/mfd/Makefile +0 -0
 obj-$(CONFIG_MFD_RT5033)     += rt5033.o
... and this isn't?
Ok, I'm rename the mfd file to rt4831 only.
Due to this mfd is the parent of all sub device, to use 'core' is trying to distinguish from rt4831-regulator or rt4831-backlight. My original thought is not to let the user confused.
If to add the postfix '-core' in the file name is bad, I think it can be removed.
Quoted 40 lines
Quoted 25 lines
drivers/mfd/Makefile +0 -0
 obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
 
drivers/mfd/rt4831-core.c new file +17 -0
new file mode 100644
index 00000000..f837c06
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.
+ *
+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#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
drivers/mfd/rt4831-core.c line 27
Quoted 10 lines
drivers/mfd/rt4831-core.c new file +10 -0
+
+#define RICHTEK_VID          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[] = {
+     OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+     MFD_CELL_NAME("rt4831-regulator")
+};
Just a little note about these helpers. I'm planning on unifying the names pretty soon. So if you have to rebase, please watch out for the rename.

Quoted 4 lines

Quoted 2 lines
Essentially OF_MFD_CELL() will soon become MFD_CELL_OF().
Yes, I'll check this change. BTW, is it possible to get this patch that I can integrate it into my codebase in advance.

Once it's applied, it will be in Linux -next.

Until then, don't worry about it.

Quoted 107 lines

Quoted 7 lines
drivers/mfd/rt4831-core.c line 47
Quoted 3 lines
drivers/mfd/rt4831-core.c new file +3 -0
+static int rt4831_probe(struct i2c_client *client)
+{
+     struct gpio_desc *enable;
My preference would be "enable_gpio" to ensure it's easily identifiable further on.
Yes, it make sense.
Quoted 6 lines
drivers/mfd/rt4831-core.c line 49
Quoted 2 lines
drivers/mfd/rt4831-core.c new file +2 -0
+     struct regmap *regmap;
+     unsigned int val;
'val' is not a great name for a variable that is used for a specific purpose. How about 'revision'?
Actually, this is just for the vendor id check.
Maybe to named 'vendor_id' is more suitable.
This register can be divided into two parts, bit[7:4] is the fixed vendor identifier, only bit[3:0] is for the chip revision.
Quoted 24 lines
Quoted 11 lines
drivers/mfd/rt4831-core.c new file +11 -0
+     int ret;
+
+     enable = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+     if (IS_ERR(enable)) {
+             dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+             return PTR_ERR(enable);
+     }
+
+     regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+     if (IS_ERR(regmap)) {
+             dev_err(&client->dev, "Failed to initialize regmap\n");
drivers/mfd/rt4831-core.c line 70
Quoted 10 lines
drivers/mfd/rt4831-core.c new file +10 -0
+             return PTR_ERR(regmap);
+     }
+
+     ret = regmap_read(regmap, RT4831_REG_REVISION, &val);
+     if (ret) {
+             dev_err(&client->dev, "Failed to get H/W revision\n");
+             return ret;
+     }
+
+     if ((val & RT4831_VID_MASK) != RICHTEK_VID) {
What does VID stand for here?
Quoted 4 lines
The fact that I have to ask probably means it should be changed.
Variant ID perhaps? If so, call the define 'VARIANT_ID'.
Like as the the above explaination.
'RICHTEK_VID' may not be good. So it make you confused. I'll rename it to 'RICHTEK_VENDOR_ID'. How about this naming?
Quoted 6 lines
drivers/mfd/rt4831-core.c line 71
Quoted 1 line
drivers/mfd/rt4831-core.c new file +1 -0
+             dev_err(&client->dev, "VID not matched, val = 0x%02x\n", val);
Using variable names means nothing to the user.
"revision: 0x%02x" ?
How about this one "CHIP ID: 0x%02x"?
May I use 'chip_id to replace this variable 'val' naming?
Quoted 39 lines
Quoted 39 lines
drivers/mfd/rt4831-core.c new file +39 -0
+             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 void rt4831_shutdown(struct i2c_client *client)
+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     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);
Quoted 9 lines
drivers/mfd/rt4831-core.c line 115
Quoted 5 lines
drivers/mfd/rt4831-core.c new file +5 -0
+
+static struct i2c_driver rt4831_driver = {
+     .driver = {
+             .name = "rt4831",
+             .of_match_table = of_match_ptr(rt4831_of_match),
The trend is to not use of_match_ptr() anymore, as these devices can not be instantiated using ACPI.
OK, I'll remove 'of_match_ptr'.

Quoted 9 lines

drivers/mfd/rt4831-core.c line 124

Quoted 9 lines

drivers/mfd/rt4831-core.c new file +9 -0

+     },
+     .probe_new = rt4831_probe,
+     .remove = rt4831_remove,
+     .shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");

Yes, all fine. Thanks.

Signature · 4 lines

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