linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] RTL8231 GPIO expander support
@ 2021-06-12 21:12 Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

The RTL8231 GPIO and LED expander can be configured for use as an MDIO or SMI
bus device. Currently only the MDIO mode is supported, although SMI mode
support should be fairly straightforward, once an SMI bus driver is available.

Provided features by the RTL8231:
  - Up to 37 GPIOs
    - Configurable drive strength: 8mA or 4mA (currently unsupported)
    - Input debouncing on GPIOs 31-36
  - Up to 88 LEDs in multiple scan matrix groups
    - On, off, or one of six toggling intervals
    - "single-color mode": 2×36 single color LEDs + 8 bi-color LEDs
    - "bi-color mode": (12 + 2×6) bi-color LEDs + 24 single color LEDs
  - Up to one PWM output (currently unsupported)
    - Fixed duty cycle, 8 selectable frequencies (1.2kHz - 4.8kHz)

The GPIO controller uses gpio-regmap. To support the aliased data input and
output registers, the regmap interface is extended to supported atomic,
uncached register reads. This is then used with a new quirk for gpio-regmap.

Register access is provided through a new MDIO regmap provider. The required
MDIO regmap support was merged in Mark Brown's regmap repository, and can be
found under the regmap-mdio tag:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git/tag/?h=regmap-mdio

---
Another revision of this patch series, now without (virtual) register paging.
After a few other (failed) attemps, I added a call to the regmap interface to
perform atomic, uncached register reads. Combined with the appropriate caching
of the register values, this can provide a split view of the data registers for
gpio-regmap. See patches 1/7 and 2/7.

These additions allowed the MFD core driver to be a bit less complex. The GPIO
support didn't see significant changes, so I've kept the review tags. The
bindings and LED driver are unchanged.

With this patch series (hopefully) nearing its final form, I was wondering if
this could be merged via the MFD tree, when all the necessary reviews and/or
acks are present. Would that be OK for everyone?

Changes since v4:
  - List myself as maintainer for this chip
  - Add uncached register reads to regmap; replaces virtual registers
Link: https://lore.kernel.org/lkml/cover.1622713678.git.sander@svanheule.net/

Changes since v3:
  - Drop gpio-regmap direction-before-value quirk
  - Use secondary virtual register range to enable proper read-modify-write
    behaviour on GPIO output values
  - Add pin debounce support
  - Switch to generic pinmux functions

Changes since v2:
  - MDIO regmap support was merged, so patch is dropped here
  - Implement feedback for DT bindings
  - Use correct module names in Kconfigs
  - Fix k*alloc return value checks
  - Introduce GPIO regmap quirks to set output direction first
  - pinctrl: Use static pin descriptions for pin controller
  - pinctrl: Fix gpio consumer resource leak
  - mfd: Replace CONFIG_PM-ifdef'ery
  - leds: Rename interval to interval_ms

Changes since v1:
  - Reintroduce MDIO regmap, with fixed Kconfig dependencies
  - Add configurable dir/value order for gpio-regmap direction_out call
  - Drop allocations for regmap fields that are used only on init
  - Move some definitions to MFD header
  - Add PM ops to replace driver remove for MFD
  - Change pinctrl driver to (modified) gpio-regmap
  - Change leds driver to use fwnode

Changes since RFC:
  - Dropped MDIO regmap interface. I was unable to resolve the Kconfig
    dependency issue, so have reverted to using regmap_config.reg_read/write.
  - Added pinctrl support
  - Added LED support
  - Changed root device to MFD, with pinctrl and leds child devices. Root
    device is now an mdio_device driver.

Sander Vanheule (8):
  regmap: Support atomic forced uncached reads
  gpio: regmap: Add quirk for aliased data registers
  dt-bindings: leds: Binding for RTL8231 scan matrix
  dt-bindings: mfd: Binding for RTL8231
  mfd: Add RTL8231 core device
  pinctrl: Add RTL8231 pin control and GPIO support
  leds: Add support for RTL8231 LED scan matrix
  MAINTAINERS: Add RTL8231 MFD driver

 .../bindings/leds/realtek,rtl8231-leds.yaml   | 166 +++++++
 .../bindings/mfd/realtek,rtl8231.yaml         | 190 ++++++++
 MAINTAINERS                                   |  10 +
 drivers/base/regmap/regmap.c                  |  33 ++
 drivers/gpio/gpio-regmap.c                    |   7 +-
 drivers/leds/Kconfig                          |  10 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-rtl8231.c                   | 291 ++++++++++++
 drivers/mfd/Kconfig                           |   9 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rtl8231.c                         | 186 ++++++++
 drivers/pinctrl/Kconfig                       |  11 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/pinctrl-rtl8231.c             | 438 ++++++++++++++++++
 include/linux/gpio/regmap.h                   |  13 +
 include/linux/mfd/rtl8231.h                   |  71 +++
 include/linux/regmap.h                        |   8 +
 17 files changed, 1445 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml
 create mode 100644 drivers/leds/leds-rtl8231.c
 create mode 100644 drivers/mfd/rtl8231.c
 create mode 100644 drivers/pinctrl/pinctrl-rtl8231.c
 create mode 100644 include/linux/mfd/rtl8231.h

-- 
2.31.1


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

* [PATCH v5 1/8] regmap: Support atomic forced uncached reads
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:23   ` Andy Shevchenko
  2021-06-14 10:33   ` Mark Brown
  2021-06-12 21:12 ` [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers Sander Vanheule
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

When a user wants to read a single uncached register, cache bypassing
can be enabled. However, this is not atomic unless an external lock is
used for the regmap. When using regcache_cache_bypass, the original
bypass state also cannot be restored.

Add support to atomically read a single uncached value, bypassing any
regmap cache.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/base/regmap/regmap.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/regmap.h       |  8 ++++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fe3e38dd5324..a828f05535b7 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2745,6 +2745,39 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
 }
 EXPORT_SYMBOL_GPL(regmap_read);
 
+/**
+ * regmap_read_bypassed() - Read a value from a single register, bypassing the cache
+ *
+ * @map: Register map to read from
+ * @reg: Register to be read from
+ * @val: Pointer to store read value
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val)
+{
+	bool bypass;
+	int ret;
+
+	if (!IS_ALIGNED(reg, map->reg_stride))
+		return -EINVAL;
+
+	map->lock(map->lock_arg);
+
+	bypass = map->cache_bypass;
+	map->cache_bypass = true;
+
+	ret = _regmap_read(map, reg, val);
+
+	map->cache_bypass = bypass;
+
+	map->unlock(map->lock_arg);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_read_bypassed);
+
 /**
  * regmap_raw_read() - Read raw data from the device
  *
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index f5f08dd0a116..a54dc00326ba 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1133,6 +1133,7 @@ int regmap_multi_reg_write_bypassed(struct regmap *map,
 int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 			   const void *val, size_t val_len);
 int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
+int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val);
 int regmap_raw_read(struct regmap *map, unsigned int reg,
 		    void *val, size_t val_len);
 int regmap_noinc_read(struct regmap *map, unsigned int reg,
@@ -1607,6 +1608,13 @@ static inline int regmap_read(struct regmap *map, unsigned int reg,
 	return -EINVAL;
 }
 
+static inline int regmap_read_bypassed(struct regmap *map, unsigned int reg,
+				       unsigned int *val)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_raw_read(struct regmap *map, unsigned int reg,
 				  void *val, size_t val_len)
 {
-- 
2.31.1


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

* [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:29   ` Andy Shevchenko
  2021-06-12 21:12 ` [PATCH v5 3/8] dt-bindings: leds: Binding for RTL8231 scan matrix Sander Vanheule
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

Some chips have the read-only input and write-only output data registers
aliased to the same offset. As a result it is not possible to perform
read-modify-writes on the output values, when a line is still configured
as input.

Add a quirk for aliased data registers, and document how the regmap
should be set up for correct operation.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-regmap.c  |  7 ++++++-
 include/linux/gpio/regmap.h | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 134cedf151a7..3a3d0d9a945c 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -15,6 +15,7 @@ struct gpio_regmap {
 	struct device *parent;
 	struct regmap *regmap;
 	struct gpio_chip gpio_chip;
+	unsigned int quirks;
 
 	int reg_stride;
 	int ngpio_per_reg;
@@ -68,7 +69,10 @@ static int gpio_regmap_get(struct gpio_chip *chip, unsigned int offset)
 	if (ret)
 		return ret;
 
-	ret = regmap_read(gpio->regmap, reg, &val);
+	if (gpio->quirks & GPIO_REGMAP_QUIRK_ALIASED_DATA)
+		ret = regmap_read_bypassed(gpio->regmap, reg, &val);
+	else
+		ret = regmap_read(gpio->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -227,6 +231,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 
 	gpio->parent = config->parent;
 	gpio->regmap = config->regmap;
+	gpio->quirks = config->quirks;
 	gpio->ngpio_per_reg = config->ngpio_per_reg;
 	gpio->reg_stride = config->reg_stride;
 	gpio->reg_mask_xlate = config->reg_mask_xlate;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index 334dd928042b..b0751a10fa4a 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -12,6 +12,17 @@ struct regmap;
 #define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
 #define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
 
+enum gpio_regmap_quirk {
+	/*
+	 * For hardware where the (read-only) input and (write-only) output
+	 * registers are aliased to the same offset. In this case the register
+	 * must not be marked as volatile and a regcache must be used, to cache
+	 * the write-only output values. Register reads for the input values
+	 * will be performed by bypassing the cache.
+	 */
+	GPIO_REGMAP_QUIRK_ALIASED_DATA = BIT(0),
+};
+
 /**
  * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
  * @parent:		The parent device
@@ -31,6 +42,7 @@ struct regmap;
  * @reg_stride:		(Optional) May be set if the registers (of the
  *			same type, dat, set, etc) are not consecutive.
  * @ngpio_per_reg:	Number of GPIOs per register
+ * @quirks:		Flags indicating GPIO chip hardware issues
  * @irq_domain:		(Optional) IRQ domain if the controller is
  *			interrupt-capable
  * @reg_mask_xlate:     (Optional) Translates base address and GPIO
@@ -73,6 +85,7 @@ struct gpio_regmap_config {
 	unsigned int reg_dir_out_base;
 	int reg_stride;
 	int ngpio_per_reg;
+	unsigned int quirks;
 	struct irq_domain *irq_domain;
 
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
-- 
2.31.1


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

* [PATCH v5 3/8] dt-bindings: leds: Binding for RTL8231 scan matrix
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 4/8] dt-bindings: mfd: Binding for RTL8231 Sander Vanheule
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule, Rob Herring

Add a binding description for the Realtek RTL8231's LED support, which
consists of up to 88 LEDs arranged in a number of scanning matrices.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
Reviewed-by: Rob Herring <robh@kernel.org>

---
v4:
- Add Rob's review tag
---
 .../bindings/leds/realtek,rtl8231-leds.yaml   | 166 ++++++++++++++++++
 1 file changed, 166 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml

diff --git a/Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml b/Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml
new file mode 100644
index 000000000000..560249cd7e8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml
@@ -0,0 +1,166 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/realtek,rtl8231-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek RTL8231 LED scan matrix.
+
+maintainers:
+  - Sander Vanheule <sander@svanheule.net>
+
+description: |
+  The RTL8231 has support for driving a number of LED matrices, by scanning
+  over the LEDs pins, alternatingly lighting different columns and/or rows.
+
+  This functionality is available on an RTL8231, when it is configured for use
+  as an MDIO device, or SMI device.
+
+  In single color scan mode, 88 LEDs are supported. These are grouped into
+  three output matrices:
+    - Group A of 6×6 single color LEDs. Rows and columns are driven by GPIO
+      pins 0-11.
+               L0[n]    L1[n]    L2[n]    L0[n+6]  L1[n+6]  L2[n+6]
+                |        |        |        |        |        |
+       P0/P6  --<--------<--------<--------<--------<--------< (3)
+                |        |        |        |        |        |
+       P1/P7  --<--------<--------<--------<--------<--------< (4)
+                |        |        |        |        |        |
+       P2/P8  --<--------<--------<--------<--------<--------< (5)
+                |        |        |        |        |        |
+       P3/P9  --<--------<--------<--------<--------<--------< (6)
+                |        |        |        |        |        |
+       P4/P10 --<--------<--------<--------<--------<--------< (7)
+                |        |        |        |        |        |
+       P5/P11 --<--------<--------<--------<--------<--------< (8)
+               (0)      (1)      (2)      (9)     (10)     (11)
+    - Group B of 6×6 single color LEDs. Rows and columns are driven by GPIO
+      pins 12-23.
+               L0[n]    L1[n]    L2[n]    L0[n+6]  L1[n+6]  L2[n+6]
+                |        |        |        |        |        |
+      P12/P18 --<--------<--------<--------<--------<--------< (15)
+                |        |        |        |        |        |
+      P13/P19 --<--------<--------<--------<--------<--------< (16)
+                |        |        |        |        |        |
+      P14/P20 --<--------<--------<--------<--------<--------< (17)
+                |        |        |        |        |        |
+      P15/P21 --<--------<--------<--------<--------<--------< (18)
+                |        |        |        |        |        |
+      P16/P22 --<--------<--------<--------<--------<--------< (19)
+                |        |        |        |        |        |
+      P17/P23 --<--------<--------<--------<--------<--------< (20)
+              (12)     (13)     (14)    (21)      (22)     (23)
+    - Group C of 8 pairs of anti-parallel (or bi-color) LEDs. LED selection is
+      provided by GPIO pins 24-27 and 29-32, polarity selection by GPIO 28.
+               P24     P25  ...  P30     P31
+                |       |         |       |
+      LED POL --X-------X---/\/---X-------X (28)
+              (24)    (25)  ... (31)    (32)
+
+  In bi-color scan mode, 72 LEDs are supported. These are grouped into four
+  output matrices:
+    - Group A of 12 pairs of anti-parallel LEDs. LED selection is provided
+      by GPIO pins 0-11, polarity selection by GPIO 12.
+    - Group B of 6 pairs of anti-parallel LEDs. LED selection is provided
+      by GPIO pins 23-28, polarity selection by GPIO 21.
+    - Group C of 6 pairs of anti-parallel LEDs. LED selection is provided
+      by GPIO pins 29-34, polarity selection by GPIO 22.
+    - Group of 4×6 single color LEDs. Rows are driven by GPIO pins 15-20,
+      columns by GPIO pins 13-14 and 21-22 (shared with groups B and C).
+           L2[n]    L2[n+6]   L2[n+12]  L2[n+18]
+            |        |         |         |
+       +0 --<--------<---------<---------< (15)
+            |        |         |         |
+       +1 --<--------<---------<---------< (16)
+            |        |         |         |
+       +2 --<--------<---------<---------< (17)
+            |        |         |         |
+       +3 --<--------<---------<---------< (18)
+            |        |         |         |
+       +4 --<--------<---------<---------< (19)
+            |        |         |         |
+       +6 --<--------<---------<---------< (20)
+          (13)     (14)      (21)      (22)
+
+  This node must always be a child of a 'realtek,rtl8231' node.
+
+properties:
+  $nodename:
+    const: led-controller
+
+  compatible:
+    const: realtek,rtl8231-leds
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+  realtek,led-scan-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: |
+      Specify the scanning mode the chip should run in. See general description
+      for how the scanning matrices are wired up.
+    enum: ["single-color", "bi-color"]
+
+patternProperties:
+  "^led@":
+    description: |
+      LEDs are addressed by their port index and led index. Ports 0-23 always
+      support three LEDs. Additionally, but only when used in single color scan
+      mode, ports 24-31 support two LEDs.
+    type: object
+
+    properties:
+      reg:
+        items:
+          - description: port index
+            maximum: 31
+          - description: led index
+            maximum: 2
+
+    allOf:
+      - $ref: ../leds/common.yaml#
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - realtek,led-scan-mode
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    led-controller {
+        compatible = "realtek,rtl8231-leds";
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        realtek,led-scan-mode = "single-color";
+
+        led@0,0 {
+            reg = <0 0>;
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <0>;
+        };
+
+        led@0,1 {
+            reg = <0 1>;
+            color = <LED_COLOR_ID_AMBER>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <0>;
+        };
+
+        led@0,2 {
+            reg = <0 2>;
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_STATUS;
+        };
+    };
-- 
2.31.1


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

* [PATCH v5 4/8] dt-bindings: mfd: Binding for RTL8231
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
                   ` (2 preceding siblings ...)
  2021-06-12 21:12 ` [PATCH v5 3/8] dt-bindings: leds: Binding for RTL8231 scan matrix Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 5/8] mfd: Add RTL8231 core device Sander Vanheule
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule, Rob Herring

Add a binding description for the Realtek RTL8231, a GPIO and LED
expander chip commonly used in ethernet switches based on a Realtek
switch SoC. These chips can be addressed via an MDIO or SMI bus, or used
as a plain 36-bit shift register.

This binding only describes the feature set provided by the MDIO/SMI
configuration, and covers the GPIO, PWM, and pin control properties. The
LED properties are defined in a separate binding.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

---
v4:
- Add Rob's and Linus' review tags
---
 .../bindings/mfd/realtek,rtl8231.yaml         | 190 ++++++++++++++++++
 1 file changed, 190 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml

diff --git a/Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml b/Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml
new file mode 100644
index 000000000000..4e2326401560
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml
@@ -0,0 +1,190 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/realtek,rtl8231.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek RTL8231 GPIO and LED expander.
+
+maintainers:
+  - Sander Vanheule <sander@svanheule.net>
+
+description: |
+  The RTL8231 is a GPIO and LED expander chip, providing up to 37 GPIOs, up to
+  88 LEDs, and up to one PWM output. This device is frequently used alongside
+  Realtek switch SoCs, to provide additional I/O capabilities.
+
+  To manage the RTL8231's features, its strapping pins can be used to configure
+  it in one of three modes: shift register, MDIO device, or SMI device. The
+  shift register mode does not need special support. In MDIO or SMI mode, most
+  pins can be configured as a GPIO output, LED matrix scan line/column, or as a
+  PWM output.
+
+  The GPIO, PWM, and pin control are part of the main node. LED support is
+  configured as a sub-node.
+
+properties:
+  compatible:
+    const: realtek,rtl8231
+
+  reg:
+    description: MDIO or SMI device address.
+    maxItems: 1
+
+  # GPIO support
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description: |
+      The first cell is the pin number and the second cell is used to specify
+      the GPIO active state.
+
+  gpio-ranges:
+    description: |
+      Must reference itself, and provide a zero-based mapping for 37 pins.
+    maxItems: 1
+
+  # Pin muxing and configuration
+  drive-strength:
+    description: |
+      Common drive strength used for all GPIO output pins, must be 4mA or 8mA.
+      On reset, this value will default to 8mA.
+    enum: [4, 8]
+
+  # LED scanning matrix
+  led-controller:
+    $ref: ../leds/realtek,rtl8231-leds.yaml#
+
+  # PWM output
+  "#pwm-cells":
+    description: |
+      Twos cells with PWM index (must be 0) and PWM frequency in Hz. To use
+      the PWM output, gpio35 must be muxed to its 'pwm' function. Valid
+      frequency values for consumers are 1200, 1600, 2000, 2400, 2800, 3200,
+      4000, and 4800.
+    const: 2
+
+patternProperties:
+  "-pins$":
+    type: object
+    $ref: ../pinctrl/pinmux-node.yaml#
+
+    properties:
+      pins:
+        items:
+          enum: ["gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5", "gpio6",
+                 "gpio7", "gpio8", "gpio9", "gpio10", "gpio11", "gpio12", "gpio13",
+                 "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19", "gpio20",
+                 "gpio21", "gpio22", "gpio23", "gpio24", "gpio25", "gpio26", "gpio27",
+                 "gpio28", "gpio29", "gpio30", "gpio31", "gpio32", "gpio33", "gpio34",
+                 "gpio35", "gpio36"]
+        minItems: 1
+        maxItems: 37
+      function:
+        description: |
+          Select which function to use. "gpio" is supported for all pins, "led" is supported
+          for pins 0-34, "pwm" is supported for pin 35.
+        enum: ["gpio", "led", "pwm"]
+
+    required:
+      - pins
+      - function
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+  - gpio-ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    // Minimal example
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        expander0: expander@0 {
+            compatible = "realtek,rtl8231";
+            reg = <0>;
+
+            gpio-controller;
+            #gpio-cells = <2>;
+            gpio-ranges = <&expander0 0 0 37>;
+        };
+    };
+  - |
+    // All bells and whistles included
+    #include <dt-bindings/leds/common.h>
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        expander1: expander@1 {
+            compatible = "realtek,rtl8231";
+            reg = <1>;
+
+            gpio-controller;
+            #gpio-cells = <2>;
+            gpio-ranges = <&expander1 0 0 37>;
+
+            #pwm-cells = <2>;
+
+            drive-strength = <4>;
+
+            button-pins {
+                pins = "gpio36";
+                function = "gpio";
+                input-debounce = "100000";
+            };
+
+            pwm-pins {
+                pins = "gpio35";
+                function = "pwm";
+            };
+
+            led-pins {
+                pins = "gpio0", "gpio1", "gpio3", "gpio4";
+                function = "led";
+            };
+
+            led-controller {
+                compatible = "realtek,rtl8231-leds";
+                #address-cells = <2>;
+                #size-cells = <0>;
+
+                realtek,led-scan-mode = "single-color";
+
+                led@0,0 {
+                    reg = <0 0>;
+                    color = <LED_COLOR_ID_GREEN>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <0>;
+                };
+
+                led@0,1 {
+                    reg = <0 1>;
+                    color = <LED_COLOR_ID_AMBER>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <0>;
+                };
+
+                led@1,0 {
+                    reg = <1 0>;
+                    color = <LED_COLOR_ID_GREEN>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <1>;
+                };
+
+                led@1,1 {
+                    reg = <1 1>;
+                    color = <LED_COLOR_ID_AMBER>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <1>;
+                };
+            };
+        };
+    };
-- 
2.31.1


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

* [PATCH v5 5/8] mfd: Add RTL8231 core device
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
                   ` (3 preceding siblings ...)
  2021-06-12 21:12 ` [PATCH v5 4/8] dt-bindings: mfd: Binding for RTL8231 Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 6/8] pinctrl: Add RTL8231 pin control and GPIO support Sander Vanheule
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

The RTL8231 is implemented as an MDIO device, and provides a regmap
interface for register access by the core and child devices.

The chip can also be a device on an SMI bus, an I2C-like bus by Realtek.
Since kernel support for SMI is limited, and no real-world SMI
implementations have been encountered for this device, this is currently
unimplemented. The use of the regmap interface should make any future
support relatively straightforward.

After reset, all pins are muxed to GPIO inputs before the pin drivers
are enabled. This is done to prevent accidental system resets, when a
pin is connected to the parent SoC's reset line.

To provide different read and write semantics for the GPIO data
registers, a secondary virtual register range is used to enable separate
caching properties of pin input and output values.

Signed-off-by: Sander Vanheule <sander@svanheule.net>

---
v5:
- Optional reset-gpios error handling
- Drop virtual registers, since custom reg_read/reg_write weren't used

v4:
- Define more bit masks
- Force writes to critical bit fields (reset, output enable)
- Add virtual addresses and caching

v3:
- Replace CONFIG_PM-ifdef'ery

v2:
- A missing MDIO_BUS dependency, as was reported by kernel test robot
  <lkp@intel.com>, is provided via REGMAP_MDIO.
  Link: https://lore.kernel.org/lkml/202105122003.JzBO0lrM-lkp@intel.com/
  Link: https://lore.kernel.org/lkml/202105122140.ZFyj5hQy-lkp@intel.com/
---
 drivers/mfd/Kconfig         |   9 ++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/rtl8231.c       | 186 ++++++++++++++++++++++++++++++++++++
 include/linux/mfd/rtl8231.h |  71 ++++++++++++++
 4 files changed, 267 insertions(+)
 create mode 100644 drivers/mfd/rtl8231.c
 create mode 100644 include/linux/mfd/rtl8231.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 5c7f2b100191..68f28a335b8c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1076,6 +1076,15 @@ config MFD_RDC321X
 	  southbridge which provides access to GPIOs and Watchdog using the
 	  southbridge PCI device configuration space.
 
+config MFD_RTL8231
+	tristate "Realtek RTL8231 GPIO and LED expander"
+	select MFD_CORE
+	select REGMAP_MDIO
+	help
+	  Support for the Realtek RTL8231 GPIO and LED expander.
+	  Provides up to 37 GPIOs, 88 LEDs, and one PWM output.
+	  When built as a module, this module will be named rtl8231.
+
 config MFD_RT5033
 	tristate "Richtek RT5033 Power Management IC"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 4f6d2b8a5f76..4b27c2486ccc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -234,6 +234,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_RTL8231)	+= rtl8231.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
 
diff --git a/drivers/mfd/rtl8231.c b/drivers/mfd/rtl8231.c
new file mode 100644
index 000000000000..36c735d0f556
--- /dev/null
+++ b/drivers/mfd/rtl8231.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/core.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/rtl8231.h>
+
+#define RTL8231_ALL_PINS_MASK	GENMASK(RTL8231_BITS_VAL - 1, 0)
+#define RTL8231_REAL_REG(reg)	(reg & GENMASK(4, 0))
+
+/*
+ * Only specify non-volatile registers that are non-zero or write-only.
+ * The data outputs also need to be defined, to avoid using the initial
+ * input values.
+ */
+static const struct reg_default rtl8231_reg_defaults[] = {
+	{ .reg = RTL8231_REG_PIN_MODE1,		.def = 0xf840	},
+	{ .reg = RTL8231_REG_GPIO_DATA0,	.def = 0x0000	},
+	{ .reg = RTL8231_REG_GPIO_DATA1,	.def = 0x0000	},
+	{ .reg = RTL8231_REG_GPIO_DATA2,	.def = 0x0000	},
+};
+
+static bool rtl8231_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	/*
+	 * Registers with self-clearing bits, strapping pin values.
+	 * Don't mark the data registers as volatile, since we need
+	 * caching for the output values.
+	 */
+	case RTL8231_REG_FUNC0:
+	case RTL8231_REG_FUNC1:
+	case RTL8231_REG_PIN_HI_CFG:
+	case RTL8231_REG_LED_END:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct reg_field RTL8231_FIELD_LED_START = REG_FIELD(RTL8231_REG_FUNC0, 1, 1);
+
+static const struct mfd_cell rtl8231_cells[] = {
+	{
+		.name = "rtl8231-pinctrl",
+	},
+	{
+		.name = "rtl8231-leds",
+		.of_compatible = "realtek,rtl8231-leds",
+	},
+};
+
+static int rtl8231_init(struct device *dev, struct regmap *map)
+{
+	unsigned int val;
+	int err;
+
+	err = regmap_read(map, RTL8231_REG_FUNC1, &val);
+	if (err) {
+		dev_err(dev, "failed to read READY_CODE\n");
+		return err;
+	}
+
+	val = FIELD_GET(RTL8231_FUNC1_READY_CODE_MASK, val);
+	if (val != RTL8231_FUNC1_READY_CODE_VALUE) {
+		dev_err(dev, "RTL8231 not present or ready 0x%x != 0x%x\n",
+			val, RTL8231_FUNC1_READY_CODE_VALUE);
+		return -ENODEV;
+	}
+
+	/* SOFT_RESET bit self-clears when done */
+	regmap_write_bits(map, RTL8231_REG_PIN_HI_CFG,
+		RTL8231_PIN_HI_CFG_SOFT_RESET, RTL8231_PIN_HI_CFG_SOFT_RESET);
+	err = regmap_read_poll_timeout(map, RTL8231_REG_PIN_HI_CFG, val,
+		!(val & RTL8231_PIN_HI_CFG_SOFT_RESET), 50, 1000);
+	if (err)
+		return err;
+
+	/*
+	 * Chip reset results in a pin configuration that is a mix of LED and GPIO outputs.
+	 * Select GPI functionality for all pins before enabling pin outputs.
+	 */
+	regmap_write(map, RTL8231_REG_PIN_MODE0, RTL8231_ALL_PINS_MASK);
+	regmap_write(map, RTL8231_REG_GPIO_DIR0, RTL8231_ALL_PINS_MASK);
+	regmap_write(map, RTL8231_REG_PIN_MODE1, RTL8231_ALL_PINS_MASK);
+	regmap_write(map, RTL8231_REG_GPIO_DIR1, RTL8231_ALL_PINS_MASK);
+	regmap_write(map, RTL8231_REG_PIN_HI_CFG,
+		RTL8231_PIN_HI_CFG_MODE_MASK | RTL8231_PIN_HI_CFG_DIR_MASK);
+
+	return 0;
+}
+
+static const struct regmap_config rtl8231_mdio_regmap_config = {
+	.val_bits = RTL8231_BITS_VAL,
+	.reg_bits = RTL8231_BITS_REG,
+	.volatile_reg = rtl8231_volatile_reg,
+	.max_register = RTL8231_REG_COUNT - 1,
+	.use_single_read = true,
+	.use_single_write = true,
+	.reg_format_endian = REGMAP_ENDIAN_BIG,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+	.cache_type = REGCACHE_FLAT,
+	.reg_defaults = rtl8231_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(rtl8231_reg_defaults),
+};
+
+static int rtl8231_mdio_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct regmap_field *led_start;
+	struct regmap *map;
+	int err;
+
+	map = devm_regmap_init_mdio(mdiodev, &rtl8231_mdio_regmap_config);
+	if (IS_ERR(map)) {
+		dev_err(dev, "failed to init regmap\n");
+		return PTR_ERR(map);
+	}
+
+	led_start = devm_regmap_field_alloc(dev, map, RTL8231_FIELD_LED_START);
+	if (IS_ERR(led_start))
+		return PTR_ERR(led_start);
+
+	dev_set_drvdata(dev, led_start);
+
+	mdiodev->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(mdiodev->reset_gpio))
+		return PTR_ERR(mdiodev->reset_gpio);
+
+	device_property_read_u32(dev, "reset-assert-delay", &mdiodev->reset_assert_delay);
+	device_property_read_u32(dev, "reset-deassert-delay", &mdiodev->reset_deassert_delay);
+
+	err = rtl8231_init(dev, map);
+	if (err)
+		return err;
+
+	/* LED_START enables power to output pins, and starts the LED engine */
+	regmap_field_force_write(led_start, 1);
+
+	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, rtl8231_cells,
+		ARRAY_SIZE(rtl8231_cells), NULL, 0, NULL);
+}
+
+__maybe_unused static int rtl8231_suspend(struct device *dev)
+{
+	struct regmap_field *led_start = dev_get_drvdata(dev);
+
+	return regmap_field_force_write(led_start, 0);
+}
+
+__maybe_unused static int rtl8231_resume(struct device *dev)
+{
+	struct regmap_field *led_start = dev_get_drvdata(dev);
+
+	return regmap_field_force_write(led_start, 1);
+}
+
+static SIMPLE_DEV_PM_OPS(rtl8231_pm_ops, rtl8231_suspend, rtl8231_resume);
+
+static const struct of_device_id rtl8231_of_match[] = {
+	{ .compatible = "realtek,rtl8231" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rtl8231_of_match);
+
+static struct mdio_driver rtl8231_mdio_driver = {
+	.mdiodrv.driver = {
+		.name = "rtl8231-expander",
+		.of_match_table	= rtl8231_of_match,
+		.pm = pm_ptr(&rtl8231_pm_ops),
+	},
+	.probe = rtl8231_mdio_probe,
+};
+mdio_module_driver(rtl8231_mdio_driver);
+
+MODULE_AUTHOR("Sander Vanheule <sander@svanheule.net>");
+MODULE_DESCRIPTION("Realtek RTL8231 GPIO and LED expander");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/rtl8231.h b/include/linux/mfd/rtl8231.h
new file mode 100644
index 000000000000..1f2ee5d13829
--- /dev/null
+++ b/include/linux/mfd/rtl8231.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Register definitions the RTL8231 GPIO and LED expander chip
+ */
+
+#ifndef __LINUX_MFD_RTL8231_H
+#define __LINUX_MFD_RTL8231_H
+
+#include <linux/bits.h>
+
+/*
+ * Registers addresses are 5 bit, values are 16 bit
+ * Also define a duplicated range of virtual addresses, to enable
+ * different read/write behaviour on the GPIO data registers
+ */
+#define RTL8231_BITS_VAL		16
+#define RTL8231_BITS_REG		5
+
+/* Chip control */
+#define RTL8231_REG_FUNC0		0x00
+#define RTL8231_FUNC0_SCAN_MODE		BIT(0)
+#define RTL8231_FUNC0_SCAN_SINGLE	0
+#define RTL8231_FUNC0_SCAN_BICOLOR	BIT(0)
+
+#define RTL8231_REG_FUNC1		0x01
+#define RTL8231_FUNC1_READY_CODE_VALUE	0x37
+#define RTL8231_FUNC1_READY_CODE_MASK	GENMASK(9, 4)
+#define RTL8231_FUNC1_DEBOUNCE_MASK	GENMASK(15, 10)
+
+/* Pin control */
+#define RTL8231_REG_PIN_MODE0		0x02
+#define RTL8231_REG_PIN_MODE1		0x03
+
+#define RTL8231_PIN_MODE_LED		0
+#define RTL8231_PIN_MODE_GPIO		1
+
+/* Pin high config: pin and GPIO control for pins 32-26 */
+#define RTL8231_REG_PIN_HI_CFG		0x04
+#define RTL8231_PIN_HI_CFG_MODE_MASK	GENMASK(4, 0)
+#define RTL8231_PIN_HI_CFG_DIR_MASK	GENMASK(9, 5)
+#define RTL8231_PIN_HI_CFG_INV_MASK	GENMASK(14, 10)
+#define RTL8231_PIN_HI_CFG_SOFT_RESET	BIT(15)
+
+/* GPIO control registers */
+#define RTL8231_REG_GPIO_DIR0		0x05
+#define RTL8231_REG_GPIO_DIR1		0x06
+#define RTL8231_REG_GPIO_INVERT0	0x07
+#define RTL8231_REG_GPIO_INVERT1	0x08
+
+#define RTL8231_GPIO_DIR_IN		1
+#define RTL8231_GPIO_DIR_OUT		0
+
+/*
+ * GPIO data registers
+ * Only the output data can be written to these registers, and only the input
+ * data can be read.
+ */
+#define RTL8231_REG_GPIO_DATA0		0x1c
+#define RTL8231_REG_GPIO_DATA1		0x1d
+#define RTL8231_REG_GPIO_DATA2		0x1e
+#define RTL8231_PIN_HI_DATA_MASK	GENMASK(4, 0)
+
+/* LED control base registers */
+#define RTL8231_REG_LED0_BASE		0x09
+#define RTL8231_REG_LED1_BASE		0x10
+#define RTL8231_REG_LED2_BASE		0x17
+#define RTL8231_REG_LED_END		0x1b
+
+#define RTL8231_REG_COUNT		0x1f
+
+#endif /* __LINUX_MFD_RTL8231_H */
-- 
2.31.1


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

* [PATCH v5 6/8] pinctrl: Add RTL8231 pin control and GPIO support
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
                   ` (4 preceding siblings ...)
  2021-06-12 21:12 ` [PATCH v5 5/8] mfd: Add RTL8231 core device Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 7/8] leds: Add support for RTL8231 LED scan matrix Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 8/8] MAINTAINERS: Add RTL8231 MFD driver Sander Vanheule
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

This driver implements the GPIO and pin muxing features provided by the
RTL8231. The device should be instantiated as an MFD child, where the
parent device has already configured the regmap used for register
access.

Debouncing is only available for the six highest GPIOs, and must be
emulated when other pins are used for (button) inputs. Although
described in the bindings, drive strength selection is currently not
implemented.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

---
v5:
- Add Andy's and Linus' review tags
- Use uintptr_t for integer stored in void* field
  Reported by: kernel test robot <lkp@intel.com>
- Replace virtual registers by GPIO_REGMAP_QUIRK_ALIASED_DATA

v4:
- Switch to pinmux_generic for pin functions
- Add pin debounce pinconf
- Virtual addresses and caching
- Use PRT_ERR_OR_ZERO in pinctrl/gpio probe
- Drop direction-first quirk for gpio-regmap

v3:
- Use static pin description for pin controller
- Fix gpio consumer resource leak

v2:
- Use gpio-regmap with direction-before-value output
---
 drivers/pinctrl/Kconfig           |  11 +
 drivers/pinctrl/Makefile          |   1 +
 drivers/pinctrl/pinctrl-rtl8231.c | 438 ++++++++++++++++++++++++++++++
 3 files changed, 450 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rtl8231.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index c2c7e7963ed0..a02c1befbee4 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -221,6 +221,17 @@ config PINCTRL_ROCKCHIP
 	help
           This support pinctrl and gpio driver for Rockchip SoCs.
 
+config PINCTRL_RTL8231
+	tristate "Realtek RTL8231 GPIO expander's pin controller"
+	depends on MFD_RTL8231
+	default MFD_RTL8231
+	select GPIO_REGMAP
+	select GENERIC_PINCONF
+	select GENERIC_PINMUX_FUNCTIONS
+	help
+	  Support for RTL8231 expander's GPIOs and pin controller.
+	  When built as a module, the module will be called pinctrl-rtl8231.
+
 config PINCTRL_SINGLE
 	tristate "One-register-per-pin type device tree based pinctrl driver"
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 5ef5334a797f..239603efb317 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
 obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
 obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
 obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
+obj-$(CONFIG_PINCTRL_RTL8231)	+= pinctrl-rtl8231.o
 obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
 obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
diff --git a/drivers/pinctrl/pinctrl-rtl8231.c b/drivers/pinctrl/pinctrl-rtl8231.c
new file mode 100644
index 000000000000..d3b52022f224
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rtl8231.c
@@ -0,0 +1,438 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bitfield.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/module.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "core.h"
+#include "pinmux.h"
+#include <linux/mfd/rtl8231.h>
+
+#define RTL8231_NUM_GPIOS		37
+#define RTL8231_DEBOUNCE_USEC		100000
+#define RTL8231_DEBOUNCE_MIN_OFFSET	31
+
+struct rtl8231_pin_ctrl {
+	struct pinctrl_desc pctl_desc;
+	struct regmap *map;
+};
+
+/*
+ * Pin controller functionality
+ */
+static const char * const rtl8231_pin_function_names[] = {
+	"gpio",
+	"led",
+	"pwm",
+};
+
+enum rtl8231_pin_function {
+	RTL8231_PIN_FUNCTION_GPIO = BIT(0),
+	RTL8231_PIN_FUNCTION_LED = BIT(1),
+	RTL8231_PIN_FUNCTION_PWM = BIT(2),
+};
+
+struct rtl8231_pin_desc {
+	const enum rtl8231_pin_function functions;
+	const u8 reg;
+	const u8 offset;
+	const u8 gpio_function_value;
+};
+
+#define RTL8231_PIN_DESC(_num, _func, _reg, _fld, _val)		\
+	[_num] = {						\
+		.functions = RTL8231_PIN_FUNCTION_GPIO | _func,	\
+		.reg = _reg,					\
+		.offset = _fld,					\
+		.gpio_function_value = _val,			\
+	}
+#define RTL8231_GPIO_PIN_DESC(_num, _reg, _fld)			\
+	RTL8231_PIN_DESC(_num, 0, _reg, _fld, RTL8231_PIN_MODE_GPIO)
+#define RTL8231_LED_PIN_DESC(_num, _reg, _fld)			\
+	RTL8231_PIN_DESC(_num, RTL8231_PIN_FUNCTION_LED, _reg, _fld, RTL8231_PIN_MODE_GPIO)
+#define RTL8231_PWM_PIN_DESC(_num, _reg, _fld)			\
+	RTL8231_PIN_DESC(_num, RTL8231_PIN_FUNCTION_PWM, _reg, _fld, 0)
+
+/*
+ * All pins have a GPIO/LED mux bit, but the bits for pins 35/36 are read-only. Use this bit
+ * for the GPIO-only pin instead of a placeholder, so the rest of the logic can stay generic.
+ */
+static struct rtl8231_pin_desc rtl8231_pin_data[RTL8231_NUM_GPIOS] = {
+	RTL8231_LED_PIN_DESC(0, RTL8231_REG_PIN_MODE0, 0),
+	RTL8231_LED_PIN_DESC(1, RTL8231_REG_PIN_MODE0, 1),
+	RTL8231_LED_PIN_DESC(2, RTL8231_REG_PIN_MODE0, 2),
+	RTL8231_LED_PIN_DESC(3, RTL8231_REG_PIN_MODE0, 3),
+	RTL8231_LED_PIN_DESC(4, RTL8231_REG_PIN_MODE0, 4),
+	RTL8231_LED_PIN_DESC(5, RTL8231_REG_PIN_MODE0, 5),
+	RTL8231_LED_PIN_DESC(6, RTL8231_REG_PIN_MODE0, 6),
+	RTL8231_LED_PIN_DESC(7, RTL8231_REG_PIN_MODE0, 7),
+	RTL8231_LED_PIN_DESC(8, RTL8231_REG_PIN_MODE0, 8),
+	RTL8231_LED_PIN_DESC(9, RTL8231_REG_PIN_MODE0, 9),
+	RTL8231_LED_PIN_DESC(10, RTL8231_REG_PIN_MODE0, 10),
+	RTL8231_LED_PIN_DESC(11, RTL8231_REG_PIN_MODE0, 11),
+	RTL8231_LED_PIN_DESC(12, RTL8231_REG_PIN_MODE0, 12),
+	RTL8231_LED_PIN_DESC(13, RTL8231_REG_PIN_MODE0, 13),
+	RTL8231_LED_PIN_DESC(14, RTL8231_REG_PIN_MODE0, 14),
+	RTL8231_LED_PIN_DESC(15, RTL8231_REG_PIN_MODE0, 15),
+	RTL8231_LED_PIN_DESC(16, RTL8231_REG_PIN_MODE1, 0),
+	RTL8231_LED_PIN_DESC(17, RTL8231_REG_PIN_MODE1, 1),
+	RTL8231_LED_PIN_DESC(18, RTL8231_REG_PIN_MODE1, 2),
+	RTL8231_LED_PIN_DESC(19, RTL8231_REG_PIN_MODE1, 3),
+	RTL8231_LED_PIN_DESC(20, RTL8231_REG_PIN_MODE1, 4),
+	RTL8231_LED_PIN_DESC(21, RTL8231_REG_PIN_MODE1, 5),
+	RTL8231_LED_PIN_DESC(22, RTL8231_REG_PIN_MODE1, 6),
+	RTL8231_LED_PIN_DESC(23, RTL8231_REG_PIN_MODE1, 7),
+	RTL8231_LED_PIN_DESC(24, RTL8231_REG_PIN_MODE1, 8),
+	RTL8231_LED_PIN_DESC(25, RTL8231_REG_PIN_MODE1, 9),
+	RTL8231_LED_PIN_DESC(26, RTL8231_REG_PIN_MODE1, 10),
+	RTL8231_LED_PIN_DESC(27, RTL8231_REG_PIN_MODE1, 11),
+	RTL8231_LED_PIN_DESC(28, RTL8231_REG_PIN_MODE1, 12),
+	RTL8231_LED_PIN_DESC(29, RTL8231_REG_PIN_MODE1, 13),
+	RTL8231_LED_PIN_DESC(30, RTL8231_REG_PIN_MODE1, 14),
+	RTL8231_LED_PIN_DESC(31, RTL8231_REG_PIN_MODE1, 15),
+	RTL8231_LED_PIN_DESC(32, RTL8231_REG_PIN_HI_CFG, 0),
+	RTL8231_LED_PIN_DESC(33, RTL8231_REG_PIN_HI_CFG, 1),
+	RTL8231_LED_PIN_DESC(34, RTL8231_REG_PIN_HI_CFG, 2),
+	RTL8231_PWM_PIN_DESC(35, RTL8231_REG_FUNC1, 3),
+	RTL8231_GPIO_PIN_DESC(36, RTL8231_REG_PIN_HI_CFG, 4),
+};
+
+#define RTL8231_PIN(_num)				\
+	{						\
+		.number = _num,				\
+		.name = "gpio" #_num,			\
+		.drv_data = &rtl8231_pin_data[_num]	\
+	}
+
+static const struct pinctrl_pin_desc rtl8231_pins[RTL8231_NUM_GPIOS] = {
+	RTL8231_PIN(0),
+	RTL8231_PIN(1),
+	RTL8231_PIN(2),
+	RTL8231_PIN(3),
+	RTL8231_PIN(4),
+	RTL8231_PIN(5),
+	RTL8231_PIN(6),
+	RTL8231_PIN(7),
+	RTL8231_PIN(8),
+	RTL8231_PIN(9),
+	RTL8231_PIN(10),
+	RTL8231_PIN(11),
+	RTL8231_PIN(12),
+	RTL8231_PIN(13),
+	RTL8231_PIN(14),
+	RTL8231_PIN(15),
+	RTL8231_PIN(16),
+	RTL8231_PIN(17),
+	RTL8231_PIN(18),
+	RTL8231_PIN(19),
+	RTL8231_PIN(20),
+	RTL8231_PIN(21),
+	RTL8231_PIN(22),
+	RTL8231_PIN(23),
+	RTL8231_PIN(24),
+	RTL8231_PIN(25),
+	RTL8231_PIN(26),
+	RTL8231_PIN(27),
+	RTL8231_PIN(28),
+	RTL8231_PIN(29),
+	RTL8231_PIN(30),
+	RTL8231_PIN(31),
+	RTL8231_PIN(32),
+	RTL8231_PIN(33),
+	RTL8231_PIN(34),
+	RTL8231_PIN(35),
+	RTL8231_PIN(36),
+};
+
+static int rtl8231_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(rtl8231_pins);
+}
+
+static const char *rtl8231_get_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+	return rtl8231_pins[selector].name;
+}
+
+static int rtl8231_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+	const unsigned int **pins, unsigned int *num_pins)
+{
+	if (selector >= ARRAY_SIZE(rtl8231_pins))
+		return -EINVAL;
+
+	*pins = &rtl8231_pins[selector].number;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const struct pinctrl_ops rtl8231_pinctrl_ops = {
+	.get_groups_count = rtl8231_get_groups_count,
+	.get_group_name = rtl8231_get_group_name,
+	.get_group_pins = rtl8231_get_group_pins,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+	.dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static int rtl8231_set_mux(struct pinctrl_dev *pctldev, unsigned int func_selector,
+	unsigned int group_selector)
+{
+	const struct function_desc *func = pinmux_generic_get_function(pctldev, func_selector);
+	const struct rtl8231_pin_desc *desc = rtl8231_pins[group_selector].drv_data;
+	const struct rtl8231_pin_ctrl *ctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int func_flag = (uintptr_t) func->data;
+	unsigned int function_mask;
+	unsigned int gpio_function;
+
+	if (!(desc->functions & func_flag))
+		return -EINVAL;
+
+	function_mask = BIT(desc->offset);
+	gpio_function = desc->gpio_function_value << desc->offset;
+
+	if (func_flag == RTL8231_PIN_FUNCTION_GPIO)
+		return regmap_update_bits(ctrl->map, desc->reg, function_mask, gpio_function);
+	else
+		return regmap_update_bits(ctrl->map, desc->reg, function_mask, ~gpio_function);
+}
+
+static int rtl8231_gpio_request_enable(struct pinctrl_dev *pctldev,
+	struct pinctrl_gpio_range *range, unsigned int offset)
+{
+	const struct rtl8231_pin_desc *desc = rtl8231_pins[offset].drv_data;
+	struct rtl8231_pin_ctrl *ctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int function_mask;
+	unsigned int gpio_function;
+
+	function_mask = BIT(desc->offset);
+	gpio_function = desc->gpio_function_value << desc->offset;
+
+	return regmap_update_bits(ctrl->map, desc->reg, function_mask, gpio_function);
+}
+
+static const struct pinmux_ops rtl8231_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = rtl8231_set_mux,
+	.gpio_request_enable = rtl8231_gpio_request_enable,
+	.strict = true,
+};
+
+static int rtl8231_pin_config_get(struct pinctrl_dev *pctldev, unsigned int offset,
+	unsigned long *config)
+{
+	struct rtl8231_pin_ctrl *ctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int param = pinconf_to_config_param(*config);
+	unsigned int arg;
+	int err;
+	int v;
+
+	switch (param) {
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		if (offset < RTL8231_DEBOUNCE_MIN_OFFSET)
+			return -EINVAL;
+
+		err = regmap_read(ctrl->map, RTL8231_REG_FUNC1, &v);
+		if (err)
+			return err;
+
+		v = FIELD_GET(RTL8231_FUNC1_DEBOUNCE_MASK, v);
+		if (v & BIT(offset - RTL8231_DEBOUNCE_MIN_OFFSET))
+			arg = RTL8231_DEBOUNCE_USEC;
+		else
+			arg = 0;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return 0;
+}
+
+static int rtl8231_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset,
+	unsigned long *configs, unsigned int num_configs)
+{
+	struct rtl8231_pin_ctrl *ctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int param, arg;
+	unsigned int pin_mask;
+	int err;
+	int i;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_INPUT_DEBOUNCE:
+			if (offset < RTL8231_DEBOUNCE_MIN_OFFSET)
+				return -EINVAL;
+
+			pin_mask = FIELD_PREP(RTL8231_FUNC1_DEBOUNCE_MASK,
+				BIT(offset - RTL8231_DEBOUNCE_MIN_OFFSET));
+
+			switch (arg) {
+			case 0:
+				err = regmap_update_bits(ctrl->map, RTL8231_REG_FUNC1,
+					pin_mask, 0);
+				break;
+			case RTL8231_DEBOUNCE_USEC:
+				err = regmap_update_bits(ctrl->map, RTL8231_REG_FUNC1,
+					pin_mask, pin_mask);
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+	}
+
+	return err;
+}
+
+static const struct pinconf_ops rtl8231_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = rtl8231_pin_config_get,
+	.pin_config_set = rtl8231_pin_config_set,
+};
+
+static int rtl8231_pinctrl_init_functions(struct pinctrl_dev *pctl, struct rtl8231_pin_ctrl *ctrl)
+{
+	const char *function_name;
+	const char **groups;
+	unsigned int f_idx;
+	unsigned int pin;
+	int num_groups;
+	int err;
+
+	for (f_idx = 0; f_idx < ARRAY_SIZE(rtl8231_pin_function_names); f_idx++) {
+		function_name = rtl8231_pin_function_names[f_idx];
+
+		for (pin = 0, num_groups = 0; pin < ctrl->pctl_desc.npins; pin++)
+			if (rtl8231_pin_data[pin].functions & BIT(f_idx))
+				num_groups++;
+
+		groups = devm_kcalloc(pctl->dev, num_groups, sizeof(*groups), GFP_KERNEL);
+		if (!groups)
+			return -ENOMEM;
+
+		for (pin = 0, num_groups = 0; pin < ctrl->pctl_desc.npins; pin++)
+			if (rtl8231_pin_data[pin].functions & BIT(f_idx))
+				groups[num_groups++] = rtl8231_pins[pin].name;
+
+		err = pinmux_generic_add_function(pctl, function_name, groups, num_groups,
+			(void *) BIT(f_idx));
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+
+static int rtl8231_pinctrl_init(struct device *dev, struct rtl8231_pin_ctrl *ctrl)
+{
+	struct pinctrl_dev *pctldev;
+	int err;
+
+	ctrl->pctl_desc.name = "rtl8231-pinctrl";
+	ctrl->pctl_desc.owner = THIS_MODULE;
+	ctrl->pctl_desc.confops = &rtl8231_pinconf_ops;
+	ctrl->pctl_desc.pctlops = &rtl8231_pinctrl_ops;
+	ctrl->pctl_desc.pmxops = &rtl8231_pinmux_ops;
+	ctrl->pctl_desc.npins = ARRAY_SIZE(rtl8231_pins);
+	ctrl->pctl_desc.pins = rtl8231_pins;
+
+	err = devm_pinctrl_register_and_init(dev->parent, &ctrl->pctl_desc, ctrl, &pctldev);
+	if (err) {
+		dev_err(dev, "failed to register pin controller\n");
+		return err;
+	}
+
+	err = rtl8231_pinctrl_init_functions(pctldev, ctrl);
+	if (err)
+		return err;
+
+	err = pinctrl_enable(pctldev);
+	if (err)
+		dev_err(dev, "failed to enable pin controller\n");
+
+	return err;
+}
+
+/*
+ * GPIO controller functionality
+ */
+static int rtl8231_gpio_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+	unsigned int offset, unsigned int *reg, unsigned int *mask)
+{
+	unsigned int pin_mask = BIT(offset % RTL8231_BITS_VAL);
+
+	if (base == RTL8231_REG_GPIO_DATA0 || offset < 32) {
+		*reg = base + offset / RTL8231_BITS_VAL;
+		*mask = pin_mask;
+	} else if (base == RTL8231_REG_GPIO_DIR0) {
+		*reg = RTL8231_REG_PIN_HI_CFG;
+		*mask = FIELD_PREP(RTL8231_PIN_HI_CFG_DIR_MASK, pin_mask);
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rtl8231_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rtl8231_pin_ctrl *ctrl;
+	struct gpio_regmap_config gpio_cfg = {};
+	int err;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->map = dev_get_regmap(dev->parent, NULL);
+	if (!ctrl->map)
+		return -ENODEV;
+
+	err = rtl8231_pinctrl_init(dev, ctrl);
+	if (err)
+		return err;
+
+	gpio_cfg.regmap = ctrl->map;
+	gpio_cfg.parent = dev->parent;
+	gpio_cfg.ngpio = RTL8231_NUM_GPIOS;
+	gpio_cfg.ngpio_per_reg = RTL8231_BITS_VAL;
+	gpio_cfg.quirks = GPIO_REGMAP_QUIRK_ALIASED_DATA;
+
+	gpio_cfg.reg_dat_base = GPIO_REGMAP_ADDR(RTL8231_REG_GPIO_DATA0);
+	gpio_cfg.reg_set_base = GPIO_REGMAP_ADDR(RTL8231_REG_GPIO_DATA0);
+	gpio_cfg.reg_dir_in_base = GPIO_REGMAP_ADDR(RTL8231_REG_GPIO_DIR0);
+
+	gpio_cfg.reg_mask_xlate = rtl8231_gpio_reg_mask_xlate;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_cfg));
+}
+
+static struct platform_driver rtl8231_pinctrl_driver = {
+	.driver = {
+		.name = "rtl8231-pinctrl",
+	},
+	.probe = rtl8231_pinctrl_probe,
+};
+module_platform_driver(rtl8231_pinctrl_driver);
+
+MODULE_AUTHOR("Sander Vanheule <sander@svanheule.net>");
+MODULE_DESCRIPTION("Realtek RTL8231 pin control and GPIO support");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1


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

* [PATCH v5 7/8] leds: Add support for RTL8231 LED scan matrix
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
                   ` (5 preceding siblings ...)
  2021-06-12 21:12 ` [PATCH v5 6/8] pinctrl: Add RTL8231 pin control and GPIO support Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  2021-06-12 21:12 ` [PATCH v5 8/8] MAINTAINERS: Add RTL8231 MFD driver Sander Vanheule
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

Both single and bi-color scanning modes are supported. The driver will
verify that the addresses are valid for the current mode, before
registering the LEDs. LEDs can be turned on, off, or toggled at one of
six predefined rates from 40ms to 1280ms.

Implements a platform device for use as a child device with RTL8231 MFD,
and uses the parent regmap to access the required registers.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

---
v5:
- Add Andy's review tag

v4:
- Rename variable addr_count -> err
- Use -EINVAL instead of -ENODEV

v3:
- Rename 'interval' to 'interval_ms'

v2:
- Use fwnode-calls instead of OF-calls
---
 drivers/leds/Kconfig        |  10 ++
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-rtl8231.c | 291 ++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/leds/leds-rtl8231.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 49d99cb084db..8cb869e8cd09 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -593,6 +593,16 @@ config LEDS_REGULATOR
 	help
 	  This option enables support for regulator driven LEDs.
 
+config LEDS_RTL8231
+	tristate "RTL8231 LED matrix support"
+	depends on LEDS_CLASS
+	depends on MFD_RTL8231
+	default MFD_RTL8231
+	help
+	  This option enables support for using the LED scanning matrix output
+	  of the RTL8231 GPIO and LED expander chip.
+	  When built as a module, this module will be named leds-rtl8231.
+
 config LEDS_BD2802
 	tristate "LED driver for BD2802 RGB LED"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7e604d3028c8..ce0f44a87dee 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_PM8058)		+= leds-pm8058.o
 obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
 obj-$(CONFIG_LEDS_PWM)			+= leds-pwm.o
 obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
+obj-$(CONFIG_LEDS_RTL8231)		+= leds-rtl8231.o
 obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_SGM3140)		+= leds-sgm3140.o
diff --git a/drivers/leds/leds-rtl8231.c b/drivers/leds/leds-rtl8231.c
new file mode 100644
index 000000000000..fb2b1ca419c9
--- /dev/null
+++ b/drivers/leds/leds-rtl8231.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/device.h>
+#include <linux/leds.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/rtl8231.h>
+
+/**
+ * struct led_toggle_rate - description of an LED blinking mode
+ * @interval_ms:	LED toggle rate in milliseconds
+ * @mode:		Register field value used to activate this mode
+ *
+ * For LED hardware accelerated blinking, with equal on and off delay.
+ * Both delays are given by @interval, so the interval at which the LED blinks
+ * (i.e. turn on and off once) is double this value.
+ */
+struct led_toggle_rate {
+	u16 interval_ms;
+	u8 mode;
+};
+
+/**
+ * struct led_modes - description of all LED modes
+ * @toggle_rates:	Array of led_toggle_rate values, sorted by ascending interval
+ * @num_toggle_rates:	Number of elements in @led_toggle_rate
+ * @off:		Register field value to turn LED off
+ * @on:			Register field value to turn LED on
+ */
+struct led_modes {
+	const struct led_toggle_rate *toggle_rates;
+	unsigned int num_toggle_rates;
+	u8 off;
+	u8 on;
+};
+
+struct rtl8231_led {
+	struct led_classdev led;
+	const struct led_modes *modes;
+	struct regmap_field *reg_field;
+};
+#define to_rtl8231_led(_cdev) container_of(_cdev, struct rtl8231_led, led)
+
+#define RTL8231_NUM_LEDS	3
+#define RTL8231_LED_PER_REG	5
+#define RTL8231_BITS_PER_LED	3
+
+static const unsigned int rtl8231_led_port_counts_single[RTL8231_NUM_LEDS] = {32, 32, 24};
+static const unsigned int rtl8231_led_port_counts_bicolor[RTL8231_NUM_LEDS] = {24, 24, 24};
+
+static const unsigned int rtl8231_led_base[RTL8231_NUM_LEDS] = {
+	RTL8231_REG_LED0_BASE,
+	RTL8231_REG_LED1_BASE,
+	RTL8231_REG_LED2_BASE,
+};
+
+#define RTL8231_DEFAULT_TOGGLE_INTERVAL_MS	500
+
+static const struct led_toggle_rate rtl8231_toggle_rates[] = {
+	{  40, 1},
+	{  80, 2},
+	{ 160, 3},
+	{ 320, 4},
+	{ 640, 5},
+	{1280, 6},
+};
+
+static const struct led_modes rtl8231_led_modes = {
+	.off = 0,
+	.on = 7,
+	.num_toggle_rates = ARRAY_SIZE(rtl8231_toggle_rates),
+	.toggle_rates = rtl8231_toggle_rates,
+};
+
+static void rtl8231_led_brightness_set(struct led_classdev *led_cdev,
+	enum led_brightness brightness)
+{
+	struct rtl8231_led *pled = to_rtl8231_led(led_cdev);
+
+	if (brightness)
+		regmap_field_write(pled->reg_field, pled->modes->on);
+	else
+		regmap_field_write(pled->reg_field, pled->modes->off);
+}
+
+static enum led_brightness rtl8231_led_brightness_get(struct led_classdev *led_cdev)
+{
+	struct rtl8231_led *pled = to_rtl8231_led(led_cdev);
+	u32 current_mode = pled->modes->off;
+
+	regmap_field_read(pled->reg_field, &current_mode);
+
+	if (current_mode == pled->modes->off)
+		return LED_OFF;
+	else
+		return LED_ON;
+}
+
+static unsigned int rtl8231_led_current_interval(struct rtl8231_led *pled)
+{
+	unsigned int mode;
+	unsigned int i;
+
+	if (regmap_field_read(pled->reg_field, &mode))
+		return 0;
+
+	for (i = 0; i < pled->modes->num_toggle_rates; i++)
+		if (mode == pled->modes->toggle_rates[i].mode)
+			return pled->modes->toggle_rates[i].interval_ms;
+
+	return 0;
+}
+
+static int rtl8231_led_blink_set(struct led_classdev *led_cdev, unsigned long *delay_on,
+	unsigned long *delay_off)
+{
+	struct rtl8231_led *pled = to_rtl8231_led(led_cdev);
+	const struct led_toggle_rate *rates = pled->modes->toggle_rates;
+	unsigned int num_rates = pled->modes->num_toggle_rates;
+	unsigned int interval_ms;
+	unsigned int i;
+	int err;
+
+	if (*delay_on == 0 && *delay_off == 0) {
+		interval_ms = RTL8231_DEFAULT_TOGGLE_INTERVAL_MS;
+	} else {
+		/*
+		 * If the current mode is blinking, choose the delay that (likely) changed.
+		 * Otherwise, choose the interval that would have the same total delay.
+		 */
+		interval_ms = rtl8231_led_current_interval(pled);
+		if (interval_ms > 0 && interval_ms == *delay_off)
+			interval_ms = *delay_on;
+		else if (interval_ms > 0 && interval_ms == *delay_on)
+			interval_ms = *delay_off;
+		else
+			interval_ms = (*delay_on + *delay_off) / 2;
+	}
+
+	/* Find clamped toggle interval */
+	for (i = 0; i < (num_rates - 1); i++)
+		if (interval_ms > rates[i].interval_ms)
+			break;
+
+	interval_ms = rates[i].interval_ms;
+
+	err = regmap_field_write(pled->reg_field, rates[i].mode);
+	if (err)
+		return err;
+
+	*delay_on = interval_ms;
+	*delay_off = interval_ms;
+
+	return 0;
+}
+
+static int rtl8231_led_read_address(struct fwnode_handle *fwnode, unsigned int *addr_port,
+	unsigned int *addr_led)
+{
+	u32 addr[2];
+	int err;
+
+	err = fwnode_property_count_u32(fwnode, "reg");
+	if (err < 0)
+		return err;
+	if (err != ARRAY_SIZE(addr))
+		return -EINVAL;
+
+	err = fwnode_property_read_u32_array(fwnode, "reg", addr, ARRAY_SIZE(addr));
+	if (err)
+		return err;
+
+	*addr_port = addr[0];
+	*addr_led = addr[1];
+
+	return 0;
+}
+
+static struct reg_field rtl8231_led_get_field(unsigned int port_index, unsigned int led_index)
+{
+	unsigned int offset, shift;
+	struct reg_field field;
+
+	offset = port_index / RTL8231_LED_PER_REG;
+	shift = (port_index % RTL8231_LED_PER_REG) * RTL8231_BITS_PER_LED;
+
+	field.reg = rtl8231_led_base[led_index] + offset;
+	field.lsb = shift;
+	field.msb = shift + RTL8231_BITS_PER_LED - 1;
+
+	return field;
+}
+
+static int rtl8231_led_probe_single(struct device *dev, struct regmap *map,
+	const unsigned int *port_counts, struct fwnode_handle *fwnode)
+{
+	struct led_init_data init_data = {};
+	struct rtl8231_led *pled;
+	unsigned int port_index;
+	unsigned int led_index;
+	struct reg_field field;
+	int err;
+
+	pled = devm_kzalloc(dev, sizeof(*pled), GFP_KERNEL);
+	if (!pled)
+		return -ENOMEM;
+
+	err = rtl8231_led_read_address(fwnode, &port_index, &led_index);
+	if (err) {
+		dev_err(dev, "LED address invalid\n");
+		return err;
+	}
+
+	if (led_index >= RTL8231_NUM_LEDS || port_index >= port_counts[led_index]) {
+		dev_err(dev, "LED address (%d.%d) invalid\n", port_index, led_index);
+		return -EINVAL;
+	}
+
+	field = rtl8231_led_get_field(port_index, led_index);
+	pled->reg_field = devm_regmap_field_alloc(dev, map, field);
+	if (IS_ERR(pled->reg_field))
+		return PTR_ERR(pled->reg_field);
+
+	pled->modes = &rtl8231_led_modes;
+
+	pled->led.max_brightness = 1;
+	pled->led.brightness_get = rtl8231_led_brightness_get;
+	pled->led.brightness_set = rtl8231_led_brightness_set;
+	pled->led.blink_set = rtl8231_led_blink_set;
+
+	init_data.fwnode = fwnode;
+
+	return devm_led_classdev_register_ext(dev, &pled->led, &init_data);
+}
+
+static int rtl8231_led_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const unsigned int *port_counts;
+	struct fwnode_handle *child;
+	struct regmap *map;
+	int err;
+
+	map = dev_get_regmap(dev->parent, NULL);
+	if (!map)
+		return -ENODEV;
+
+	if (device_property_match_string(dev, "realtek,led-scan-mode", "single-color") >= 0) {
+		port_counts = rtl8231_led_port_counts_single;
+		regmap_update_bits(map, RTL8231_REG_FUNC0,
+			RTL8231_FUNC0_SCAN_MODE, RTL8231_FUNC0_SCAN_SINGLE);
+	} else if (device_property_match_string(dev, "realtek,led-scan-mode", "bi-color") >= 0) {
+		port_counts = rtl8231_led_port_counts_bicolor;
+		regmap_update_bits(map, RTL8231_REG_FUNC0,
+			RTL8231_FUNC0_SCAN_MODE, RTL8231_FUNC0_SCAN_BICOLOR);
+	} else {
+		dev_err(dev, "scan mode missing or invalid\n");
+		return -EINVAL;
+	}
+
+	fwnode_for_each_available_child_node(dev->fwnode, child) {
+		err = rtl8231_led_probe_single(dev, map, port_counts, child);
+		if (err)
+			dev_warn(dev, "failed to register LED %pfwP\n", child);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id of_rtl8231_led_match[] = {
+	{ .compatible = "realtek,rtl8231-leds" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_rtl8231_led_match);
+
+static struct platform_driver rtl8231_led_driver = {
+	.driver = {
+		.name = "rtl8231-leds",
+		.of_match_table = of_rtl8231_led_match,
+	},
+	.probe = rtl8231_led_probe,
+};
+module_platform_driver(rtl8231_led_driver);
+
+MODULE_AUTHOR("Sander Vanheule <sander@svanheule.net>");
+MODULE_DESCRIPTION("Realtek RTL8231 LED support");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1


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

* [PATCH v5 8/8] MAINTAINERS: Add RTL8231 MFD driver
  2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
                   ` (6 preceding siblings ...)
  2021-06-12 21:12 ` [PATCH v5 7/8] leds: Add support for RTL8231 LED scan matrix Sander Vanheule
@ 2021-06-12 21:12 ` Sander Vanheule
  7 siblings, 0 replies; 14+ messages in thread
From: Sander Vanheule @ 2021-06-12 21:12 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, linux-leds, devicetree,
	linux-gpio
  Cc: Andrew Lunn, Andy Shevchenko, linux-kernel, Sander Vanheule

Add the files associated with the RTL8231 support, and list Sander
Vanheule (myself) as maintainer.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b706dd20ff2b..62d042c14158 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15505,6 +15505,16 @@ S:	Maintained
 F:	include/sound/rt*.h
 F:	sound/soc/codecs/rt*
 
+REALTEK RTL8231 MFD DRIVER
+M:	Sander Vanheule <sander@svanheule.net>
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/realtek,rtl8231-leds.yaml
+F:	Documentation/devicetree/bindings/mfd/realtek,rtl8231.yaml
+F:	drivers/leds/leds-rtl8231.c
+F:	drivers/mfd/rtl8231.c
+F:	drivers/pinctrl/pinctrl-rtl8231.c
+F:	include/linux/mfd/rtl8231.h
+
 REALTEK RTL83xx SMI DSA ROUTER CHIPS
 M:	Linus Walleij <linus.walleij@linaro.org>
 S:	Maintained
-- 
2.31.1


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

* Re: [PATCH v5 1/8] regmap: Support atomic forced uncached reads
  2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
@ 2021-06-12 21:23   ` Andy Shevchenko
  2021-06-14 10:33   ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-06-12 21:23 UTC (permalink / raw)
  To: Sander Vanheule
  Cc: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, Linux LED Subsystem,
	devicetree, open list:GPIO SUBSYSTEM, Andrew Lunn,
	Linux Kernel Mailing List

On Sun, Jun 13, 2021 at 12:13 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> When a user wants to read a single uncached register, cache bypassing
> can be enabled. However, this is not atomic unless an external lock is
> used for the regmap. When using regcache_cache_bypass, the original
> bypass state also cannot be restored.
>
> Add support to atomically read a single uncached value, bypassing any
> regmap cache.

> +int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val)

If this is acceptable in general, I will rather name the function like
regmap_nocache_read() to be aligned with the other API naming pattern
(see below).

>  int regmap_raw_write_async(struct regmap *map, unsigned int reg,
>                            const void *val, size_t val_len);
>  int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
> +int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val);
>  int regmap_raw_read(struct regmap *map, unsigned int reg,
>                     void *val, size_t val_len);
>  int regmap_noinc_read(struct regmap *map, unsigned int reg,


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers
  2021-06-12 21:12 ` [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers Sander Vanheule
@ 2021-06-12 21:29   ` Andy Shevchenko
  2021-07-07 20:53     ` Sander Vanheule
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-06-12 21:29 UTC (permalink / raw)
  To: Sander Vanheule
  Cc: Pavel Machek, Rob Herring, Lee Jones, Mark Brown,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, Linux LED Subsystem,
	devicetree, open list:GPIO SUBSYSTEM, Andrew Lunn,
	Linux Kernel Mailing List

On Sun, Jun 13, 2021 at 12:13 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> Some chips have the read-only input and write-only output data registers
> aliased to the same offset. As a result it is not possible to perform
> read-modify-writes on the output values, when a line is still configured
> as input.
>
> Add a quirk for aliased data registers, and document how the regmap
> should be set up for correct operation.

I still believe that there is no issue with gpio-regmap and we don't
need any quirk there.

The issue is in the regmap APIs (implementation) itself. Hardware with
the concept of reading X / writing Y at the same offset is okay per
se. regmaps doesn't support it properly and should be fixed (amended)
in a way that you provide this kind of register description thru
regmap configuration or so.

I expressed the idea of trying to implement regmap-8250 as an example
of the support for such hardware. And OTOH that this kind of hardware
is not unusual.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v5 1/8] regmap: Support atomic forced uncached reads
  2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
  2021-06-12 21:23   ` Andy Shevchenko
@ 2021-06-14 10:33   ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-06-14 10:33 UTC (permalink / raw)
  To: Sander Vanheule
  Cc: Pavel Machek, Rob Herring, Lee Jones, Greg Kroah-Hartman,
	Rafael J . Wysocki, Michael Walle, Linus Walleij,
	Bartosz Golaszewski, linux-leds, devicetree, linux-gpio,
	Andrew Lunn, Andy Shevchenko, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

On Sat, Jun 12, 2021 at 11:12:31PM +0200, Sander Vanheule wrote:

> When a user wants to read a single uncached register, cache bypassing
> can be enabled. However, this is not atomic unless an external lock is
> used for the regmap. When using regcache_cache_bypass, the original
> bypass state also cannot be restored.

> Add support to atomically read a single uncached value, bypassing any
> regmap cache.

The expectation here is that if there is a need to do this for some
reason the user can arrange to do this for itself - if something is
happening that makes a normally non-volatile register volatile then 
it probably needs higher level coordination.  What's the use case?

> +int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val)

Bypassed what?  I think Andy's naming suggestion was much better.

Please also keep to 80 columns if you can, I know the requirements got
relaxed a bit but no need to do it excessively.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers
  2021-06-12 21:29   ` Andy Shevchenko
@ 2021-07-07 20:53     ` Sander Vanheule
  2021-07-08 14:23       ` Mark Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Sander Vanheule @ 2021-07-07 20:53 UTC (permalink / raw)
  To: Andy Shevchenko, Mark Brown
  Cc: Pavel Machek, Rob Herring, Lee Jones, Greg Kroah-Hartman,
	Rafael J . Wysocki, Michael Walle, Linus Walleij,
	Bartosz Golaszewski, Linux LED Subsystem, devicetree,
	open list:GPIO SUBSYSTEM, Andrew Lunn, Linux Kernel Mailing List

Hi Andy, Mark,

My apologies for the delay in replying, work has been keeping me a bit busy.

On Sun, 2021-06-13 at 00:29 +0300, Andy Shevchenko wrote:
> On Sun, Jun 13, 2021 at 12:13 AM Sander Vanheule <sander@svanheule.net> wrote:
> > 
> > Some chips have the read-only input and write-only output data registers
> > aliased to the same offset. As a result it is not possible to perform
> > read-modify-writes on the output values, when a line is still configured
> > as input.
> > 
> > Add a quirk for aliased data registers, and document how the regmap
> > should be set up for correct operation.
> 
> I still believe that there is no issue with gpio-regmap and we don't
> need any quirk there.
> 
> The issue is in the regmap APIs (implementation) itself. Hardware with
> the concept of reading X / writing Y at the same offset is okay per
> se. regmaps doesn't support it properly and should be fixed (amended)
> in a way that you provide this kind of register description thru
> regmap configuration or so.

I've made an attempt at implementing a "regmap_aliased()", similar to
"regmap_volatile()". However, this meant I had to make _regmap_read() aware of
wether the read- or write-alias was being read (from cache), touching some parts
of the regmap code I'm not using myself. Furthermore, this "aliased" property
isn't really perpendicular to "volatile", since writes should never be volatile,
and non-volatile reads don't make much sense (to me) on a read-only register.

In addition to entire registers having different meanings on reads or writes,
individual bitfields could also have different properties. Some bits of a
register could be aliased, while other bits are just 'plain' volatile. This is
the case for the last register of the RTL8231, where the low bits are GPIO data
(so aliased), and the highest bit is a self-clearing "latch new data" command
bit (i.e. RW-volatile).

If a regmap_field could overwrite the specifiers of it's parent register, I
think this may provide quite a natural solution to the aliasing problem: just
create two regmap_field defintions. One definition would be 'write-only' (and
cached for RMW), the other 'volatile read-only'. All regmap_fields could still
rely on a single cached register value, I think. I didn't try to implement this
though, so maybe I'm missing some behaviour that would disqualify this solution.
Would you think this could be an acceptable way to move forward here?


> I expressed the idea of trying to implement regmap-8250 as an example
> of the support for such hardware. And OTOH that this kind of hardware
> is not unusual.

This implementation indeed requires the same aliasing support, in addition to
register paging even. I've never touched that subsystem before though, so I
would need some more time if I wanted to try this.

Best,
Sander


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

* Re: [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers
  2021-07-07 20:53     ` Sander Vanheule
@ 2021-07-08 14:23       ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-07-08 14:23 UTC (permalink / raw)
  To: Sander Vanheule
  Cc: Andy Shevchenko, Pavel Machek, Rob Herring, Lee Jones,
	Greg Kroah-Hartman, Rafael J . Wysocki, Michael Walle,
	Linus Walleij, Bartosz Golaszewski, Linux LED Subsystem,
	devicetree, open list:GPIO SUBSYSTEM, Andrew Lunn,
	Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

On Wed, Jul 07, 2021 at 10:53:19PM +0200, Sander Vanheule wrote:

> I've made an attempt at implementing a "regmap_aliased()", similar to
> "regmap_volatile()". However, this meant I had to make _regmap_read() aware of
> wether the read- or write-alias was being read (from cache), touching some parts
> of the regmap code I'm not using myself. Furthermore, this "aliased" property
> isn't really perpendicular to "volatile", since writes should never be volatile,
> and non-volatile reads don't make much sense (to me) on a read-only register.

As far as the abstractions in regmap are concerned these registers are
volatile, that's currently how regmap undertands registers where
readback won't give you the last written value.  Trying to convince the
framework to handle these registers as anything other than volatile is
going to of need be an invasive change.

> If a regmap_field could overwrite the specifiers of it's parent register, I
> think this may provide quite a natural solution to the aliasing problem: just
> create two regmap_field defintions. One definition would be 'write-only' (and
> cached for RMW), the other 'volatile read-only'. All regmap_fields could still
> rely on a single cached register value, I think. I didn't try to implement this
> though, so maybe I'm missing some behaviour that would disqualify this solution.
> Would you think this could be an acceptable way to move forward here?

This feels like a painful and potentially high overhead approach to
things - at the minute fields are layered on top of registers and are
totally invisible at the register level, pulling the two together would
touch a lot of places and make things tense, especially if we ended up
with two different fields aliasing each other.  I'd need to see code but
it feels like a difficult approach to take.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2021-07-08 14:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-12 21:12 [PATCH v5 0/8] RTL8231 GPIO expander support Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 1/8] regmap: Support atomic forced uncached reads Sander Vanheule
2021-06-12 21:23   ` Andy Shevchenko
2021-06-14 10:33   ` Mark Brown
2021-06-12 21:12 ` [PATCH v5 2/8] gpio: regmap: Add quirk for aliased data registers Sander Vanheule
2021-06-12 21:29   ` Andy Shevchenko
2021-07-07 20:53     ` Sander Vanheule
2021-07-08 14:23       ` Mark Brown
2021-06-12 21:12 ` [PATCH v5 3/8] dt-bindings: leds: Binding for RTL8231 scan matrix Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 4/8] dt-bindings: mfd: Binding for RTL8231 Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 5/8] mfd: Add RTL8231 core device Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 6/8] pinctrl: Add RTL8231 pin control and GPIO support Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 7/8] leds: Add support for RTL8231 LED scan matrix Sander Vanheule
2021-06-12 21:12 ` [PATCH v5 8/8] MAINTAINERS: Add RTL8231 MFD driver Sander Vanheule

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