[PATCH v3 0/4] pinctrl: Add support for the Rockchip RV1106

[PATCH v3 0/4] pinctrl: Add support for the Rockchip RV1106

[Simon Glass]

This series adds pinctrl support for the Rockchip RV1106 and its
RV1103 package variant, split out from the initial RV1106 enablement
series [1] following feedback to submit per subsystem.

On this SoC each GPIO bank has a dedicated IO control (IOC) register
block, unlike earlier Rockchip designs where the registers of all
banks share a GRF region. Following Jonas's review of v1, each bank
node now references the syscon for its own IOC block through a
rockchip,grf phandle and the driver uses a separate regmap per bank,
so no regmap crosses a block boundary.

This v3 is tested on a Luckfox Pico Mini B (RV1103): pinctrl and the
four GPIO banks probe, and the sdmmc pinctrl state is applied through
the per-bank IOC regmaps, with the SD card working.

The corresponding devicetree changes are part of the main RV1106
series, which goes through the Rockchip tree.

[1] https://patchwork.kernel.org/project/linux-rockchip/list/?series=1122658

Changes in v3:
- Add new patch moving the drive-strength decoding to the get function
- Declare the 8-bit level drive type on the banks so that reads decode
  correctly
- Return an error from the iomux functions when a bank has no IOC
  regmap, rather than falling back to the shared regmap

Changes in v2:
- Add new patch for the per-bank IOC reference
- Use a separate IOC regmap per bank, taken from the rockchip,grf
  phandle of each bank node and identified by the gpio alias, with
  block-relative register offsets
- Reject drive-strength requests for GPIO0 pins above 6, which have no
  drive-strength registers
- Specify only the first iomux offset for each bank, letting the driver
  calculate the increments

Simon Glass (4):
  pinctrl: rockchip: Decode drive strength in the get function
  dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property
  dt-bindings: pinctrl: rockchip: Add RV1106 compatible
  pinctrl: rockchip: Add RV1106 pinctrl support

 .../bindings/gpio/rockchip,gpio-bank.yaml     |   7 +
 .../bindings/pinctrl/rockchip,pinctrl.yaml    |   1 +
 drivers/pinctrl/pinctrl-rockchip.c            | 230 ++++++++++++++++--
 drivers/pinctrl/pinctrl-rockchip.h            |   4 +
 4 files changed, 223 insertions(+), 19 deletions(-)

---
base-commit: fc02acf6ac0ccde0c805c2daa9148683cdd01ba8
branch: rv1106b3

-- 
2.43.0

[PATCH v3 1/4] pinctrl: rockchip: Decode drive strength in the get function

[Simon Glass]

The decoding of the 2-bit and 8-bit level drive-strength values sits in
rockchip_set_drive_perpin(), where it is unreachable: the SoCs whose
banks declare these drive types (RK3506 and RV1103B) take the early
ctrl->type branch in the set path, and the read-and-decode logic in a
set function has no purpose. Meanwhile rockchip_get_drive_perpin()
lacks the decoding, so pin_config_get() and the debugfs output report
-EINVAL for these SoCs.

Move the two cases to rockchip_get_drive_perpin(), where they belong.

Fixes: dbd2317d7b9f ("pinctrl: rockchip: Add rk3506 pinctrl support")
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch moving the drive-strength decoding to the get function

 drivers/pinctrl/pinctrl-rockchip.c | 38 +++++++++++++++---------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 7e0fcd45fd26..08a46a04a815 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3267,6 +3267,25 @@ static int rockchip_get_drive_perpin(struct rockchip_pin_bank *bank,
 	case DRV_TYPE_IO_1V8_ONLY:
 		rmask_bits = RK3288_DRV_BITS_PER_PIN;
 		break;
+	case DRV_TYPE_IO_LEVEL_2_BIT:
+		ret = regmap_read(regmap, reg, &data);
+		if (ret)
+			return ret;
+		data >>= bit;
+
+		return data & 0x3;
+	case DRV_TYPE_IO_LEVEL_8_BIT:
+		ret = regmap_read(regmap, reg, &data);
+		if (ret)
+			return ret;
+		data >>= bit;
+		data &= (1 << 8) - 1;
+
+		ret = hweight8(data);
+		if (ret > 0)
+			return ret - 1;
+		else
+			return -EINVAL;
 	default:
 		dev_err(dev, "unsupported pinctrl drive type: %d\n", drv_type);
 		return -EINVAL;
@@ -3390,25 +3409,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	case DRV_TYPE_IO_1V8_ONLY:
 		rmask_bits = RK3288_DRV_BITS_PER_PIN;
 		break;
-	case DRV_TYPE_IO_LEVEL_2_BIT:
-		ret = regmap_read(regmap, reg, &data);
-		if (ret)
-			return ret;
-		data >>= bit;
-
-		return data & 0x3;
-	case DRV_TYPE_IO_LEVEL_8_BIT:
-		ret = regmap_read(regmap, reg, &data);
-		if (ret)
-			return ret;
-		data >>= bit;
-		data &= (1 << 8) - 1;
-
-		ret = hweight8(data);
-		if (ret > 0)
-			return ret - 1;
-		else
-			return -EINVAL;
 	default:
 		dev_err(dev, "unsupported pinctrl drive type: %d\n", drv_type);
 		return -EINVAL;
-- 
2.43.0

[PATCH v3 2/4] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property

[Simon Glass]

Some Rockchip SoCs, such as the RV1106, give each GPIO bank its own
IO control (IOC) register block rather than grouping the registers of
all banks into a shared GRF region. Add an optional rockchip,grf
property to the gpio-bank binding so that each bank node can reference
the syscon for its own IOC block.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- Add new patch for the per-bank IOC reference

 .../devicetree/bindings/gpio/rockchip,gpio-bank.yaml       | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml b/Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml
index bdd83f42615c..774e9c7de606 100644
--- a/Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml
+++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml
@@ -44,6 +44,13 @@ properties:
   power-domains:
     maxItems: 1
 
+  rockchip,grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node managing the IO control registers
+      of this bank, on SoCs such as the RV1106 where each GPIO bank has
+      its own IOC block.
+
 patternProperties:
   "^.+-hog(-[0-9]+)?$":
     type: object
-- 
2.43.0

[PATCH v3 3/4] dt-bindings: pinctrl: rockchip: Add RV1106 compatible

[Simon Glass]

Add the compatible for the pin controller of the Rockchip RV1106 and
its RV1103 package variant.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---

(no changes since v1)

 Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
index 9b3cbeb54fed..81747bb53056 100644
--- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
@@ -51,6 +51,7 @@ properties:
       - rockchip,rk3576-pinctrl
       - rockchip,rk3588-pinctrl
       - rockchip,rv1103b-pinctrl
+      - rockchip,rv1106-pinctrl
       - rockchip,rv1108-pinctrl
       - rockchip,rv1126-pinctrl
 
-- 
2.43.0

[PATCH v3 4/4] pinctrl: rockchip: Add RV1106 pinctrl support

[Simon Glass]

Add pinctrl support for the Rockchip RV1106, based on the vendor
kernel in the Luckfox Pico SDK [1] at commit 824b817f8 (a Linux
5.10.160 kernel tree). Each GPIO bank has its own IO control (IOC)
register block, referenced by the rockchip,grf phandle of the bank
node; the register offsets are relative to the bank's own block. The
drive strength uses the RK3568-style exponential encoding and only
pins 0-6 of GPIO0 have drive-strength registers.

The RV1103 is a package variant of the RV1106 with fewer pins and uses
the same pin controller.

[1] https://github.com/LuckfoxTECH/luckfox-pico

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---

Changes in v3:
- Declare the 8-bit level drive type on the banks so that reads decode
  correctly
- Return an error from the iomux functions when a bank has no IOC
  regmap, rather than falling back to the shared regmap

Changes in v2:
- Use a separate IOC regmap per bank, taken from the rockchip,grf
  phandle of each bank node and identified by the gpio alias, with
  block-relative register offsets
- Reject drive-strength requests for GPIO0 pins above 6, which have no
  drive-strength registers
- Specify only the first iomux offset for each bank, letting the driver
  calculate the increments

 drivers/pinctrl/pinctrl-rockchip.c | 192 +++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-rockchip.h |   4 +
 2 files changed, 196 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 08a46a04a815..4e5e4434ba4e 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -1188,6 +1188,12 @@ static int rockchip_get_mux(struct rockchip_pin_bank *bank, int pin)
 	else
 		regmap = info->regmap_base;
 
+	/* Banks with their own IOC block use its regmap for the iomux */
+	if (bank->regmap_ioc)
+		regmap = bank->regmap_ioc;
+	else if (ctrl->type == RV1106)
+		return -EINVAL;
+
 	if (ctrl->type == RV1103B && bank->bank_num == 2 && pin >= 12)
 		return 0;
 
@@ -1317,6 +1323,12 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 	else
 		regmap = info->regmap_base;
 
+	/* Banks with their own IOC block use its regmap for the iomux */
+	if (bank->regmap_ioc)
+		regmap = bank->regmap_ioc;
+	else if (ctrl->type == RV1106)
+		return -EINVAL;
+
 	if (ctrl->type == RV1103B && bank->bank_num == 2 && pin >= 12)
 		return 0;
 
@@ -1725,6 +1737,78 @@ static int rv1103b_calc_schmitt_reg_and_bit(struct rockchip_pin_bank *bank,
 	return 0;
 }
 
+#define RV1106_DRV_BITS_PER_PIN		8
+#define RV1106_DRV_PINS_PER_REG		2
+#define RV1106_PULL_BITS_PER_PIN	2
+#define RV1106_PULL_PINS_PER_REG	8
+#define RV1106_SMT_BITS_PER_PIN		1
+#define RV1106_SMT_PINS_PER_REG		8
+
+/*
+ * Each bank has its own IOC block, referenced by the rockchip,grf
+ * phandle of the bank node. The offsets below are relative to the
+ * bank's own block.
+ */
+static const int rv1106_drv_offsets[] = { 0x10, 0x80, 0xc0, 0x100, 0x20 };
+static const int rv1106_pull_offsets[] = { 0x38, 0x1c0, 0x1d0, 0x1e0, 0x70 };
+static const int rv1106_smt_offsets[] = { 0x40, 0x280, 0x290, 0x2a0, 0xa0 };
+
+static int rv1106_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank,
+				       int pin_num, struct regmap **regmap,
+				       int *reg, u8 *bit)
+{
+	if (bank->bank_num >= ARRAY_SIZE(rv1106_drv_offsets) ||
+	    !bank->regmap_ioc)
+		return -EINVAL;
+
+	/* Only pins 0-6 of GPIO0 have drive-strength registers */
+	if (bank->bank_num == 0 && pin_num > 6)
+		return -ENOTSUPP;
+
+	*regmap = bank->regmap_ioc;
+	*reg = rv1106_drv_offsets[bank->bank_num];
+	*reg += ((pin_num / RV1106_DRV_PINS_PER_REG) * 4);
+	*bit = pin_num % RV1106_DRV_PINS_PER_REG;
+	*bit *= RV1106_DRV_BITS_PER_PIN;
+
+	return 0;
+}
+
+static int rv1106_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank,
+					int pin_num, struct regmap **regmap,
+					int *reg, u8 *bit)
+{
+	if (bank->bank_num >= ARRAY_SIZE(rv1106_pull_offsets) ||
+	    !bank->regmap_ioc)
+		return -EINVAL;
+
+	*regmap = bank->regmap_ioc;
+	*reg = rv1106_pull_offsets[bank->bank_num];
+	*reg += ((pin_num / RV1106_PULL_PINS_PER_REG) * 4);
+	*bit = pin_num % RV1106_PULL_PINS_PER_REG;
+	*bit *= RV1106_PULL_BITS_PER_PIN;
+
+	return 0;
+}
+
+static int rv1106_calc_schmitt_reg_and_bit(struct rockchip_pin_bank *bank,
+					   int pin_num,
+					   struct regmap **regmap,
+					   int *reg, u8 *bit)
+{
+	if (bank->bank_num >= ARRAY_SIZE(rv1106_smt_offsets) ||
+	    !bank->regmap_ioc)
+		return -EINVAL;
+
+	*regmap = bank->regmap_ioc;
+	*reg = rv1106_smt_offsets[bank->bank_num];
+	*reg += ((pin_num / RV1106_SMT_PINS_PER_REG) * 4);
+	*bit = pin_num % RV1106_SMT_PINS_PER_REG;
+	*bit *= RV1106_SMT_BITS_PER_PIN;
+
+	return 0;
+}
+
 #define RV1108_PULL_PMU_OFFSET		0x10
 #define RV1108_PULL_OFFSET		0x110
 #define RV1108_PULL_PINS_PER_REG	8
@@ -3329,6 +3413,7 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		ret = strength;
 		goto config;
 	} else if (ctrl->type == RV1103B ||
+		   ctrl->type == RV1106 ||
 		   ctrl->type == RK3506 ||
 		   ctrl->type == RK3528 ||
 		   ctrl->type == RK3562 ||
@@ -3482,6 +3567,7 @@ static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
 				: PIN_CONFIG_BIAS_DISABLE;
 	case PX30:
 	case RV1103B:
+	case RV1106:
 	case RV1108:
 	case RK3188:
 	case RK3288:
@@ -3547,6 +3633,7 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 		break;
 	case PX30:
 	case RV1103B:
+	case RV1106:
 	case RV1108:
 	case RV1126:
 	case RK3188:
@@ -3843,6 +3930,7 @@ static bool rockchip_pinconf_pull_valid(struct rockchip_pin_ctrl *ctrl,
 		return pull ? false : true;
 	case PX30:
 	case RV1103B:
+	case RV1106:
 	case RV1108:
 	case RV1126:
 	case RK3188:
@@ -4452,6 +4540,7 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *base;
 	int ret;
+	int i;
 
 	if (!dev->of_node)
 		return dev_err_probe(dev, -ENODEV, "device tree node not found\n");
@@ -4505,6 +4594,44 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	/* try to find the optional reference to the ioc1 syscon */
 	info->regmap_ioc1 = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,ioc1");
 
+	/*
+	 * On SoCs where each GPIO bank has its own IOC block, the bank nodes
+	 * carry a rockchip,grf phandle pointing at it. The bank number comes
+	 * from the gpio alias, as used by the gpio driver, falling back to
+	 * the node position for devicetrees without aliases. The fallback is
+	 * wrong when an SoC variant omits a bank, so aliases are needed
+	 * there.
+	 */
+	i = 0;
+	for_each_child_of_node_scoped(np, child) {
+		struct rockchip_pin_bank *bank = NULL;
+		int id, j;
+
+		if (!of_match_node(rockchip_bank_match, child))
+			continue;
+
+		id = of_alias_get_id(child, "gpio");
+		if (id < 0)
+			id = i;
+		i++;
+
+		for (j = 0; j < ctrl->nr_banks; j++) {
+			if (ctrl->pin_banks[j].bank_num == id) {
+				bank = &ctrl->pin_banks[j];
+				break;
+			}
+		}
+		if (!bank)
+			continue;
+
+		bank->regmap_ioc = syscon_regmap_lookup_by_phandle_optional(
+							child, "rockchip,grf");
+		if (IS_ERR(bank->regmap_ioc))
+			return dev_err_probe(dev, PTR_ERR(bank->regmap_ioc),
+					     "%pOFn: failed to look up bank ioc\n",
+					     child);
+	}
+
 	ret = rockchip_pinctrl_register(pdev, info);
 	if (ret)
 		return ret;
@@ -4623,6 +4750,69 @@ static struct rockchip_pin_ctrl rv1103b_pin_ctrl __maybe_unused = {
 	.schmitt_calc_reg	= rv1103b_calc_schmitt_reg_and_bit,
 };
 
+static struct rockchip_pin_bank rv1106_pin_banks[] = {
+	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(0, 32, "gpio0",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0, -1, -1, -1,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT),
+	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(1, 32, "gpio1",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0, -1, -1, -1,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT),
+	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(2, 32, "gpio2",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0x20, -1, -1, -1,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT),
+	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(3, 32, "gpio3",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0x40, -1, -1, -1,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT),
+	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(4, 24, "gpio4",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0,
+				    0, -1, -1, -1,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT,
+				    DRV_TYPE_IO_LEVEL_8_BIT),
+};
+
+static struct rockchip_pin_ctrl rv1106_pin_ctrl __maybe_unused = {
+	.pin_banks		= rv1106_pin_banks,
+	.nr_banks		= ARRAY_SIZE(rv1106_pin_banks),
+	.label			= "RV1106-GPIO",
+	.type			= RV1106,
+	.pull_calc_reg		= rv1106_calc_pull_reg_and_bit,
+	.drv_calc_reg		= rv1106_calc_drv_reg_and_bit,
+	.schmitt_calc_reg	= rv1106_calc_schmitt_reg_and_bit,
+};
+
 static struct rockchip_pin_bank rv1108_pin_banks[] = {
 	PIN_BANK_IOMUX_FLAGS(0, 32, "gpio0", IOMUX_SOURCE_PMU,
 					     IOMUX_SOURCE_PMU,
@@ -5261,6 +5451,8 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
 		.data = &px30_pin_ctrl },
 	{ .compatible = "rockchip,rv1103b-pinctrl",
 		.data = &rv1103b_pin_ctrl },
+	{ .compatible = "rockchip,rv1106-pinctrl",
+		.data = &rv1106_pin_ctrl },
 	{ .compatible = "rockchip,rv1108-pinctrl",
 		.data = &rv1108_pin_ctrl },
 	{ .compatible = "rockchip,rv1126-pinctrl",
diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h
index bb0e803e3b8a..f62213d99792 100644
--- a/drivers/pinctrl/pinctrl-rockchip.h
+++ b/drivers/pinctrl/pinctrl-rockchip.h
@@ -186,6 +186,7 @@
 enum rockchip_pinctrl_type {
 	PX30,
 	RV1103B,
+	RV1106,
 	RV1108,
 	RV1126,
 	RK2928,
@@ -295,6 +296,8 @@ struct rockchip_drv {
  * @dev: the pinctrl device bind to the bank
  * @reg_base: register base of the gpio bank
  * @regmap_pull: optional separate register for additional pull settings
+ * @regmap_ioc: optional per-bank IO control regmap, for SoCs where each
+ *	    bank has its own IOC block
  * @clk: clock of the gpio bank
  * @db_clk: clock of the gpio debounce
  * @irq: interrupt of the gpio bank
@@ -323,6 +326,7 @@ struct rockchip_pin_bank {
 	struct device			*dev;
 	void __iomem			*reg_base;
 	struct regmap			*regmap_pull;
+	struct regmap			*regmap_ioc;
 	struct clk			*clk;
 	struct clk			*db_clk;
 	int				irq;
-- 
2.43.0

Re: [PATCH v3 1/4] pinctrl: rockchip: Decode drive strength in the get function

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Activating a dormant bug: incorrect drive type override for RV1103B lacks a bank_num check, leading to silent data corruption for 8-bit pins.
--

--- Patch [1]: [PATCH v3 1/4] pinctrl: rockchip: Decode drive strength in the get function ---
commit b8824e91fa66d4af5685a314e266ea1b47feaa8a
Author: Simon Glass <sjg@chromium.org>

pinctrl: rockchip: Decode drive strength in the get function

This commit moves the decoding logic for 2-bit and 8-bit level
drive-strength values from the set function to the get function, where
it is actually utilized to report pin configuration properly.

> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3267,6 +3267,25 @@ static int rockchip_get_drive_perpin(struct rockchip_pin_bank *bank,
>  	case DRV_TYPE_IO_1V8_ONLY:
>  		rmask_bits = RK3288_DRV_BITS_PER_PIN;
>  		break;
> +	case DRV_TYPE_IO_LEVEL_2_BIT:
> +		ret = regmap_read(regmap, reg, &data);
> +		if (ret)
> +			return ret;
> +		data >>= bit;
> +
> +		return data & 0x3;

[Severity: Medium]
Does this new case inadvertently introduce a regression causing silent data
corruption for 8-bit pins on RV1103B?

Earlier in rockchip_get_drive_perpin(), there is an existing override:

	if (ctrl->type == RV1103B && pin_num >= 12)
		drv_type = DRV_TYPE_IO_LEVEL_2_BIT;

This override lacks a check for bank->bank_num == 2. This forces banks 0
and 1 (which have 8-bit drive types) to be evaluated as 2-bit.

Before this patch, the incorrectly forced 2-bit type safely dropped down to
the default block and failed with -EINVAL.

By intercepting DRV_TYPE_IO_LEVEL_2_BIT here and returning early, the
function will now silently return truncated values (data & 0x3) instead of
properly decoded 8-bit thermometer values for bank 0 and 1 pins >= 12 on
RV1103B.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729132736.3807082-1-sjg@chromium.org?part=1

Re: [PATCH v3 2/4] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added decoding for `DRV_TYPE_IO_LEVEL_2_BIT` in `rockchip_get_drive_perpin()` exposes an overly broad pre-existing override, causing it to return corrupted drive strength values for 8-bit pins on RV1103B.
--

--- Patch [2]: [PATCH v3 2/4] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property ---
commit b8824e91fa66d4af5685a314e266ea1b47feaa8a
Author: Simon Glass <sjg@chromium.org>

pinctrl: rockchip: Decode drive strength in the get function

This commit moves the decoding logic for 2-bit and 8-bit drive-strength
values from rockchip_set_drive_perpin() to rockchip_get_drive_perpin()
where it can actually be used to report configuration properly.

> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 7e0fcd45fd265..08a46a04a8157 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
[ ... ]
> @@ -3267,6 +3267,25 @@ static int rockchip_get_drive_perpin(struct rockchip_pin_bank *bank,
>  	case DRV_TYPE_IO_1V8_ONLY:
>  		rmask_bits = RK3288_DRV_BITS_PER_PIN;
>  		break;
> +	case DRV_TYPE_IO_LEVEL_2_BIT:
> +		ret = regmap_read(regmap, reg, &data);
> +		if (ret)
> +			return ret;
> +		data >>= bit;
> +
> +		return data & 0x3;

[Severity: Medium]
This is a pre-existing issue, but this newly added decoding exposes an
overly broad override earlier in rockchip_get_drive_perpin():

drivers/pinctrl/pinctrl-rockchip.c:rockchip_get_drive_perpin() {
    ...
    if (ctrl->type == RV1103B && pin_num >= 12)
        drv_type = DRV_TYPE_IO_LEVEL_2_BIT;
    ...
}

Since that check does not restrict itself to a specific bank, such as
bank->bank_num == 2, does it erroneously force Bank 0 and Bank 1 pins to
be evaluated as DRV_TYPE_IO_LEVEL_2_BIT?

Before this patch, DRV_TYPE_IO_LEVEL_2_BIT fell through to the default
case and safely returned -EINVAL. With this case now wired up, it silently
intercepts these pins and returns a truncated 2-bit value for 8-bit
register fields. Can this result in corrupted drive strength readbacks
for those other banks?

> +	case DRV_TYPE_IO_LEVEL_8_BIT:
> +		ret = regmap_read(regmap, reg, &data);
> +		if (ret)
> +			return ret;
> +		data >>= bit;
> +		data &= (1 << 8) - 1;
> +
> +		ret = hweight8(data);
> +		if (ret > 0)
> +			return ret - 1;
> +		else
> +			return -EINVAL;
>  	default:
>  		dev_err(dev, "unsupported pinctrl drive type: %d\n", drv_type);
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729132736.3807082-1-sjg@chromium.org?part=2

Re: [PATCH v3 4/4] pinctrl: rockchip: Add RV1106 pinctrl support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Hardware register aliasing in RV1106 Bank 0 due to incorrect pin count and missing bounds checks.

Pre-existing issues:
- [Critical] State corruption and out-of-bounds access due to static `rockchip_pin_ctrl` mutation on probe deferral.
--

--- Patch [4]: [PATCH v3 4/4] pinctrl: rockchip: Add RV1106 pinctrl support ---
commit 5db525b718839289847048c3ede5984a9fb138b8
Author: Simon Glass <sjg@chromium.org>

pinctrl: rockchip: Add RV1106 pinctrl support

Add pinctrl support for the Rockchip RV1106. Each GPIO bank has its own
IO control (IOC) register block, referenced by the rockchip,grf phandle
of the bank node; the register offsets are relative to the bank's own block.

Link: https://github.com/LuckfoxTECH/luckfox-pico

> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 08a46a04a8157..4e5e4434ba4e7 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c

[ ... ]

> @@ -4505,6 +4594,44 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  	/* try to find the optional reference to the ioc1 syscon */
>  	info->regmap_ioc1 = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,ioc1");
>  
> +	/*
> +	 * On SoCs where each GPIO bank has its own IOC block, the bank nodes
> +	 * carry a rockchip,grf phandle pointing at it. The bank number comes
> +	 * from the gpio alias, as used by the gpio driver, falling back to
> +	 * the node position for devicetrees without aliases. The fallback is
> +	 * wrong when an SoC variant omits a bank, so aliases are needed
> +	 * there.
> +	 */
> +	i = 0;
> +	for_each_child_of_node_scoped(np, child) {
> +		struct rockchip_pin_bank *bank = NULL;
> +		int id, j;

[ ... ]

> +		bank->regmap_ioc = syscon_regmap_lookup_by_phandle_optional(
> +							child, "rockchip,grf");
> +		if (IS_ERR(bank->regmap_ioc))
> +			return dev_err_probe(dev, PTR_ERR(bank->regmap_ioc),
> +					     "%pOFn: failed to look up bank ioc\n",
> +					     child);
> +	}

[Severity: Critical]
This is a pre-existing issue, but does this new syscon lookup reliably
trigger state corruption across probe deferrals?

Looking at rockchip_pinctrl_get_soc_data() called earlier in probe, it
mutates the static rockchip_pin_ctrl structure:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_pinctrl_get_soc_data() {
    ...
    bank->pin_base = ctrl->nr_pins;
    ctrl->nr_pins += bank->nr_pins;
    ...
}

If syscon_regmap_lookup_by_phandle_optional() returns -EPROBE_DEFER, the
probe fails without resetting this static state. On subsequent probes,
ctrl->nr_pins accumulates, inflating bank->pin_base.

When a pin is later accessed, does this inflated pin_base cause
pin_to_bank() to return the wrong bank, leading to a large negative
integer for the pin?

drivers/pinctrl/pinctrl-rockchip.c:pin_to_bank() {
    ...
    while (pin >= (b->pin_base + b->nr_pins))
        b++;
    return b;
}

Would this negative pin value act as a negative array index for
bank->iomux[iomux_num], reading garbage offsets and corrupting arbitrary
registers?

> +
>  	ret = rockchip_pinctrl_register(pdev, info);
>  	if (ret)
>  		return ret;

[ ... ]

> @@ -4623,6 +4750,69 @@ static struct rockchip_pin_ctrl rv1103b_pin_ctrl __maybe_unused = {
>  	.schmitt_calc_reg	= rv1103b_calc_schmitt_reg_and_bit,
>  };
>  
> +static struct rockchip_pin_bank rv1106_pin_banks[] = {
> +	PIN_BANK_IOMUX_FLAGS_OFFSET_DRV_FLAGS(0, 32, "gpio0",
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,

[Severity: High]
Since only pins 0-6 of GPIO0 have drive-strength registers, does declaring
bank 0 with 32 pins expose hardware register aliasing?

Without bounds checking for higher pins, if pin 16 is accessed, the offset
calculation in rv1106_calc_pull_reg_and_bit() evaluates to 0x40:

drivers/pinctrl/pinctrl-rockchip.c:rv1106_calc_pull_reg_and_bit() {
    ...
    *reg = rv1106_pull_offsets[bank->bank_num];
    *reg += ((pin_num / RV1106_PULL_PINS_PER_REG) * 4);
    ...
}

Because rv1106_smt_offsets[0] is also 0x40, wouldn't setting the pull
configuration for pin 16 silently overwrite the schmitt trigger registers
for pins 0-7?

> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    0, -1, -1, -1,
> +				    DRV_TYPE_IO_LEVEL_8_BIT,
> +				    DRV_TYPE_IO_LEVEL_8_BIT,
> +				    DRV_TYPE_IO_LEVEL_8_BIT,
> +				    DRV_TYPE_IO_LEVEL_8_BIT),

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729132736.3807082-1-sjg@chromium.org?part=4

Re: [PATCH v3 3/4] dt-bindings: pinctrl: rockchip: Add RV1106 compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing pinctrl driver match support for 'rockchip,rv1106-pinctrl'
- [High] Silent truncation and unreachable dead code in rockchip_get_drive_perpin()
- [Medium] Unused 'rockchip,grf' property in 'gpio-bank' child nodes
--

--- Patch [3]: [PATCH v3 3/4] dt-bindings: pinctrl: rockchip: Add RV1106 compatible ---
commit f508d7fe3fdaea1487deaf572cd3b667691010ce
Author: Simon Glass <sjg@chromium.org>

dt-bindings: pinctrl: rockchip: Add RV1106 compatible

Add the compatible for the pin controller of the Rockchip RV1106 and
its RV1103 package variant.

> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> index 9b3cbeb54fed4..81747bb530569 100644
> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> @@ -51,6 +51,7 @@ properties:
>        - rockchip,rk3576-pinctrl
>        - rockchip,rk3588-pinctrl
>        - rockchip,rv1103b-pinctrl
> +      - rockchip,rv1106-pinctrl

[Severity: High]
Does this compatible string need to be added to the driver match table as well?

The schema strictly enforces 'rockchip,rv1106-pinctrl' without an allowable
fallback compatible, but it appears to be missing from the driver:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_pinctrl_dt_match[] {
	static const struct of_device_id rockchip_pinctrl_dt_match[] = {
		{ .compatible = "rockchip,px30-pinctrl",
    ...
}

Will the pinctrl driver silently fail to probe for any device tree using this
new compatible string?

[Severity: Medium]
Will this new hardware function correctly without parsing the rockchip,grf
property in gpio-bank child nodes?

The property appears to be correctly documented for gpio-bank subnodes:

Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml {
    ...
      rockchip,grf:
        $ref: /schemas/types.yaml#/definitions/phandle
    ...
}

But looking at the driver, it only parses rockchip,grf from the parent
pinctrl node and seems to ignore the subnodes entirely:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_pinctrl_probe() {
    ...
	node = of_parse_phandle(np, "rockchip,grf", 0);
    ...
}

Could this cause the hardware feature for per-bank IOC control to be
non-functional?

[Severity: High]
Can the RV1103B override logic lead to silent truncation and unreachable code?

Looking at rockchip_get_drive_perpin(), an override forces
DRV_TYPE_IO_LEVEL_2_BIT:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_get_drive_perpin() {
    ...
	if (ctrl->type == RV1103B && pin_num >= 12)
		drv_type = DRV_TYPE_IO_LEVEL_2_BIT;
    ...
}

This hits the 2-bit case block, which introduces an early return and performs a
2-bit truncation for Banks 0 and 1:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_get_drive_perpin() {
    ...
	case DRV_TYPE_IO_LEVEL_2_BIT:
		ret = regmap_read(regmap, reg, &data);
		if (ret)
			return ret;
		data >>= bit;

		return data & 0x3;
    ...
}

Because of this early return, does the fallback logic for RV1103B at the
bottom of the function become structurally dead?

drivers/pinctrl/pinctrl-rockchip.c:rockchip_get_drive_perpin() {
    ...
	if (ctrl->type == RV1103B && bank->bank_num == 2 && pin_num >= 12) {
		data = data >> 10;
		return data & 0x3;
	}
    ...
}

Since the override lacks a bank number check, won't 8-bit drive strength pins
on Banks 0 and 1 with pin_num >= 12 now incorrectly hit the new 2-bit case
block and be silently bitwise-truncated to a 2-bit value?

>        - rockchip,rv1108-pinctrl
>        - rockchip,rv1126-pinctrl

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729132736.3807082-1-sjg@chromium.org?part=3