Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/3] pinctrl: Add support for the Rockchip RV1106
@ 2026-07-14 19:25 Simon Glass
  2026-07-14 19:25 ` [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Simon Glass @ 2026-07-14 19:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Heiko Stuebner, Rob Herring, Jonas Karlman, Conor Dooley,
	linux-gpio, linux-rockchip, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, Simon Glass, Bartosz Golaszewski, Jeffy Chen,
	huang lin, linux-kernel

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 v2 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 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 (3):
  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            | 168 ++++++++++++++++++
 drivers/pinctrl/pinctrl-rockchip.h            |   4 +
 4 files changed, 180 insertions(+)

---
base-commit: 3b029c035b34bbc693405ddf759f0e9b920c27f1
branch: rv1106b2

-- 
2.43.0


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

* [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property
  2026-07-14 19:25 [PATCH v2 0/3] pinctrl: Add support for the Rockchip RV1106 Simon Glass
@ 2026-07-14 19:25 ` Simon Glass
  2026-07-21  8:13   ` Krzysztof Kozlowski
  2026-07-14 19:25 ` [PATCH v2 2/3] dt-bindings: pinctrl: rockchip: Add RV1106 compatible Simon Glass
  2026-07-14 19:25 ` [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support Simon Glass
  2 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2026-07-14 19:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Heiko Stuebner, Rob Herring, Jonas Karlman, Conor Dooley,
	linux-gpio, linux-rockchip, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, Simon Glass, Bartosz Golaszewski, linux-kernel

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

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


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

* [PATCH v2 2/3] dt-bindings: pinctrl: rockchip: Add RV1106 compatible
  2026-07-14 19:25 [PATCH v2 0/3] pinctrl: Add support for the Rockchip RV1106 Simon Glass
  2026-07-14 19:25 ` [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property Simon Glass
@ 2026-07-14 19:25 ` Simon Glass
  2026-07-14 19:25 ` [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support Simon Glass
  2 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2026-07-14 19:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Heiko Stuebner, Rob Herring, Jonas Karlman, Conor Dooley,
	linux-gpio, linux-rockchip, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, Simon Glass, Jeffy Chen, huang lin, linux-kernel

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

Signed-off-by: Simon Glass <sjg@chromium.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


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

* [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support
  2026-07-14 19:25 [PATCH v2 0/3] pinctrl: Add support for the Rockchip RV1106 Simon Glass
  2026-07-14 19:25 ` [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property Simon Glass
  2026-07-14 19:25 ` [PATCH v2 2/3] dt-bindings: pinctrl: rockchip: Add RV1106 compatible Simon Glass
@ 2026-07-14 19:25 ` Simon Glass
  2026-07-14 19:44   ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2026-07-14 19:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Heiko Stuebner, Rob Herring, Jonas Karlman, Conor Dooley,
	linux-gpio, linux-rockchip, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, Simon Glass, Jeffy Chen, huang lin, linux-kernel

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 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 | 168 +++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-rockchip.h |   4 +
 2 files changed, 172 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 7e0fcd45fd26..0589a7a0879f 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -1188,6 +1188,10 @@ 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;
+
 	if (ctrl->type == RV1103B && bank->bank_num == 2 && pin >= 12)
 		return 0;
 
@@ -1317,6 +1321,10 @@ 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;
+
 	if (ctrl->type == RV1103B && bank->bank_num == 2 && pin >= 12)
 		return 0;
 
@@ -1725,6 +1733,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
@@ -3310,6 +3390,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 +3563,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 +3629,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 +3926,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 +4536,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 +4590,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 +4746,49 @@ 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(0, 32, "gpio0",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0, -1, -1, -1),
+	PIN_BANK_IOMUX_FLAGS_OFFSET(1, 32, "gpio1",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0, -1, -1, -1),
+	PIN_BANK_IOMUX_FLAGS_OFFSET(2, 32, "gpio2",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0x20, -1, -1, -1),
+	PIN_BANK_IOMUX_FLAGS_OFFSET(3, 32, "gpio3",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0x40, -1, -1, -1),
+	PIN_BANK_IOMUX_FLAGS_OFFSET(4, 24, "gpio4",
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    IOMUX_WIDTH_4BIT,
+				    0,
+				    0, -1, -1, -1),
+};
+
+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 +5427,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


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

* Re: [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support
  2026-07-14 19:25 ` [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support Simon Glass
@ 2026-07-14 19:44   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-14 19:44 UTC (permalink / raw)
  To: Simon Glass; +Cc: robh, conor+dt, devicetree

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

Pre-existing issues:
- [Medium] `rockchip_get_drive_perpin()` lacks decoding logic for the exponential drive strength encoding used by RV1106, returning heavily truncated and incorrect values.
--

--- Patch [3]: [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support ---
commit 665d35cf92d5b01cd2a3d9d6d9a796fd3a3aea13
Author: Simon Glass <sjg@chromium.org>

pinctrl: rockchip: Add RV1106 pinctrl support
    
This commit adds pinctrl support for the Rockchip RV1106. Each GPIO bank
has its own IO control (IOC) register block. The drive strength uses the
RK3568-style exponential encoding.

> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 7e0fcd45fd265..0589a7a0879f7 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
[ ... ]
> @@ -3310,6 +3390,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 ||
>  		   ctrl->type == RK3568) {
>  		rmask_bits = RK3568_DRV_BITS_PER_PIN;
>  		ret = (1 << (strength + 1)) - 1;

[Severity: Medium]
This is a pre-existing issue, but does rockchip_get_drive_perpin() need to be
updated to handle this exponential encoding?

While rockchip_set_drive_perpin() correctly applies the 8-bit exponential
encoding for RV1106 here, rockchip_get_drive_perpin() seems to lack the
corresponding decoding logic.

When pin_config_get() is called for these pins, it appears to fall back to
DRV_TYPE_IO_DEFAULT, truncate the hardware register value to 2 bits (since
rmask_bits defaults to RK3288_DRV_BITS_PER_PIN), and look it up in the legacy
rockchip_perpin_drv_list array:

drivers/pinctrl/pinctrl-rockchip.c:rockchip_get_drive_perpin() {
	...
	data >>= bit;
	data &= (1 << rmask_bits) - 1;

	return rockchip_perpin_drv_list[drv_type][data];
}

Could this lead to incorrect readouts for RV1106 (and other modern SoCs) via
debugfs or pin_config_get()?

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

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

* Re: [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property
  2026-07-14 19:25 ` [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property Simon Glass
@ 2026-07-21  8:13   ` Krzysztof Kozlowski
  2026-07-21 12:45     ` Heiko Stübner
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-21  8:13 UTC (permalink / raw)
  To: Simon Glass
  Cc: Linus Walleij, Heiko Stuebner, Rob Herring, Jonas Karlman,
	Conor Dooley, linux-gpio, linux-rockchip, Krzysztof Kozlowski,
	linux-arm-kernel, devicetree, Bartosz Golaszewski, linux-kernel

On Tue, Jul 14, 2026 at 01:25:29PM -0600, Simon Glass wrote:
> 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>
> ---
> 
> 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.

I do not see usage of it in patchset linked in cover letter with DTS.

I have doubts that whil having one GRF region you have GPIO banks
pointing to different GRF regions.

It's possible if you would have multiple GRFs, but you do not. You have
one GRF, right?

Best regards,
Krzysztof


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

* Re: [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property
  2026-07-21  8:13   ` Krzysztof Kozlowski
@ 2026-07-21 12:45     ` Heiko Stübner
  0 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2026-07-21 12:45 UTC (permalink / raw)
  To: Simon Glass, Krzysztof Kozlowski
  Cc: Linus Walleij, Rob Herring, Jonas Karlman, Conor Dooley,
	linux-gpio, linux-rockchip, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, Bartosz Golaszewski, linux-kernel

Am Dienstag, 21. Juli 2026, 10:13:38 Mitteleuropäische Sommerzeit schrieb Krzysztof Kozlowski:
> On Tue, Jul 14, 2026 at 01:25:29PM -0600, Simon Glass wrote:
> > 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>
> > ---
> > 
> > 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.
> 
> I do not see usage of it in patchset linked in cover letter with DTS.
> 
> I have doubts that whil having one GRF region you have GPIO banks
> pointing to different GRF regions.
> 
> It's possible if you would have multiple GRFs, but you do not. You have
> one GRF, right?

per my reply to the grf binding patch, Rockchip invented yet another way
to describe their pin-config - in completely separate blocks strewn accross
the whole io-space this time.

So describing the associated pinconfig grf per bank, does sound pretty
nice for that problem.

In the early beginning Rockchip had one really big GRF for all of those
settings bits - which included the iomux settings.
Later on they moved _parts_ of that to another big GRF that still wasn't
dedicated to pin-config.
Recently pin-config got its own GRF - for all pins
And now it seems we're at one GRF per pinbank ;-) .

When I asked some years ago - when there even was usbphy control in here
(and those regs+bits also moved all the time), the _paraphrased_ answer
was "because hardware-designers find it nicer" ;-) .


Heiko



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

end of thread, other threads:[~2026-07-21 12:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 19:25 [PATCH v2 0/3] pinctrl: Add support for the Rockchip RV1106 Simon Glass
2026-07-14 19:25 ` [PATCH v2 1/3] dt-bindings: gpio: rockchip,gpio-bank: Add rockchip,grf property Simon Glass
2026-07-21  8:13   ` Krzysztof Kozlowski
2026-07-21 12:45     ` Heiko Stübner
2026-07-14 19:25 ` [PATCH v2 2/3] dt-bindings: pinctrl: rockchip: Add RV1106 compatible Simon Glass
2026-07-14 19:25 ` [PATCH v2 3/3] pinctrl: rockchip: Add RV1106 pinctrl support Simon Glass
2026-07-14 19:44   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox