linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] pinctrl: rockchip: add rk3308b SoC support
@ 2024-05-15 12:16 Dmitry Yashin
  2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-15 12:16 UTC (permalink / raw)
  To: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

This patch series fixes iomux routes on rk3308 and adds support for
pin controller found on rk3308b. According to rk3308b TRM, this pinctrl
much the same as rk3308's, but with additional iomux routes and 3bit
iomuxes selected via gpio##_sel_src_ctrl registers.

Downstream kernel [1] managed this SoC's with rk3308b_soc_data_init,
wich picked configuration based on cpuid. Upstream pinctrl patches
droped soc init function.

The function rk3308b_soc_sel_src_init sets up gpio##_sel_src_ctrl
registers, making SoC to use 3bit iomuxes over some 2bit old ones.

These patches have been tested on Radxa's ROCK Pi S, one based on rk3308
and the other on rk3308b (from the latest batches). For the new boards
fixes broken spi1 clk.

Similar effort [2] was made several years ago, but without keeping base
rk3308 SoC pinctrl support.

[1] https://github.com/radxa/kernel/blob/stable-4.4-rockpis/drivers/pinctrl/pinctrl-rockchip.c#L4388
[2] https://lore.kernel.org/linux-rockchip/20220930102620.1568864-1-jay.xu@rock-chips.com/

Dmitry Yashin (3):
  pinctrl: rockchip: update rk3308 iomux routes
  dt-bindings: pinctrl: rockchip: add rk3308b
  pinctrl: rockchip: add rk3308b SoC support

 .../bindings/pinctrl/rockchip,pinctrl.yaml    |   1 +
 drivers/pinctrl/pinctrl-rockchip.c            | 187 ++++++++++++++++++
 drivers/pinctrl/pinctrl-rockchip.h            |   1 +
 3 files changed, 189 insertions(+)


base-commit: ed30a4a51bb196781c8058073ea720133a65596f
-- 
2.39.2


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

* [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-15 12:16 [PATCH 0/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
@ 2024-05-15 12:16 ` Dmitry Yashin
  2024-05-28 11:29   ` Linus Walleij
  2024-05-28 11:50   ` Heiko Stübner
  2024-05-15 12:16 ` [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b Dmitry Yashin
  2024-05-15 12:16 ` [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
  2 siblings, 2 replies; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-15 12:16 UTC (permalink / raw)
  To: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
the rk3308b SoC. Remove them and correct i2c3 routes.

Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 3bedf36a0019..cc647db76927 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -870,9 +870,8 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
 	RK_MUXROUTE_SAME(0, RK_PC3, 1, 0x314, BIT(16 + 0) | BIT(0)), /* rtc_clk */
 	RK_MUXROUTE_SAME(1, RK_PC6, 2, 0x314, BIT(16 + 2) | BIT(16 + 3)), /* uart2_rxm0 */
 	RK_MUXROUTE_SAME(4, RK_PD2, 2, 0x314, BIT(16 + 2) | BIT(16 + 3) | BIT(2)), /* uart2_rxm1 */
-	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x608, BIT(16 + 8) | BIT(16 + 9)), /* i2c3_sdam0 */
-	RK_MUXROUTE_SAME(3, RK_PB4, 2, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(8)), /* i2c3_sdam1 */
-	RK_MUXROUTE_SAME(2, RK_PA0, 3, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(9)), /* i2c3_sdam2 */
+	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x314, BIT(16 + 4)), /* i2c3_sdam0 */
+	RK_MUXROUTE_SAME(3, RK_PB4, 2, 0x314, BIT(16 + 4) | BIT(4)), /* i2c3_sdam1 */
 	RK_MUXROUTE_SAME(1, RK_PA3, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclktxm0 */
 	RK_MUXROUTE_SAME(1, RK_PA4, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclkrxm0 */
 	RK_MUXROUTE_SAME(1, RK_PB5, 2, 0x308, BIT(16 + 3) | BIT(3)), /* i2s-8ch-1-sclktxm1 */
@@ -881,18 +880,6 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
 	RK_MUXROUTE_SAME(1, RK_PB6, 4, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* pdm-clkm1 */
 	RK_MUXROUTE_SAME(2, RK_PA6, 2, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* pdm-clkm2 */
 	RK_MUXROUTE_SAME(2, RK_PA4, 3, 0x600, BIT(16 + 2) | BIT(2)), /* pdm-clkm-m2 */
-	RK_MUXROUTE_SAME(3, RK_PB2, 3, 0x314, BIT(16 + 9)), /* spi1_miso */
-	RK_MUXROUTE_SAME(2, RK_PA4, 2, 0x314, BIT(16 + 9) | BIT(9)), /* spi1_miso_m1 */
-	RK_MUXROUTE_SAME(0, RK_PB3, 3, 0x314, BIT(16 + 10) | BIT(16 + 11)), /* owire_m0 */
-	RK_MUXROUTE_SAME(1, RK_PC6, 7, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(10)), /* owire_m1 */
-	RK_MUXROUTE_SAME(2, RK_PA2, 5, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(11)), /* owire_m2 */
-	RK_MUXROUTE_SAME(0, RK_PB3, 2, 0x314, BIT(16 + 12) | BIT(16 + 13)), /* can_rxd_m0 */
-	RK_MUXROUTE_SAME(1, RK_PC6, 5, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* can_rxd_m1 */
-	RK_MUXROUTE_SAME(2, RK_PA2, 4, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* can_rxd_m2 */
-	RK_MUXROUTE_SAME(1, RK_PC4, 3, 0x314, BIT(16 + 14)), /* mac_rxd0_m0 */
-	RK_MUXROUTE_SAME(4, RK_PA2, 2, 0x314, BIT(16 + 14) | BIT(14)), /* mac_rxd0_m1 */
-	RK_MUXROUTE_SAME(3, RK_PB4, 4, 0x314, BIT(16 + 15)), /* uart3_rx */
-	RK_MUXROUTE_SAME(0, RK_PC1, 3, 0x314, BIT(16 + 15) | BIT(15)), /* uart3_rx_m1 */
 };
 
 static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
-- 
2.39.2


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

* [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b
  2024-05-15 12:16 [PATCH 0/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
  2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
@ 2024-05-15 12:16 ` Dmitry Yashin
  2024-05-15 16:17   ` Conor Dooley
  2024-05-15 12:16 ` [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
  2 siblings, 1 reply; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-15 12:16 UTC (permalink / raw)
  To: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

Add compatible string for rk3308b pin controller.

Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
---
 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 20e806dce1ec..1b38b496cc8b 100644
--- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
@@ -41,6 +41,7 @@ properties:
       - rockchip,rk3228-pinctrl
       - rockchip,rk3288-pinctrl
       - rockchip,rk3308-pinctrl
+      - rockchip,rk3308b-pinctrl
       - rockchip,rk3328-pinctrl
       - rockchip,rk3368-pinctrl
       - rockchip,rk3399-pinctrl
-- 
2.39.2


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

* [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-15 12:16 [PATCH 0/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
  2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
  2024-05-15 12:16 ` [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b Dmitry Yashin
@ 2024-05-15 12:16 ` Dmitry Yashin
  2024-05-15 16:29   ` Luca Ceresoli
  2024-05-15 17:00   ` Christophe JAILLET
  2 siblings, 2 replies; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-15 12:16 UTC (permalink / raw)
  To: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

Add pinctrl support for rk3308b. This pin controller much the same as
rk3308's, but with additional iomux routes and 3bit iomuxes selected
via gpio##_sel_src_ctrl registers. Set them up in the function
rk3308b_soc_sel_src_init to use new 3bit iomuxes over some 2bit old ones.

Fixes: 1f3e25a06883 ("pinctrl: rockchip: fix RK3308 pinmux bits")
Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 200 +++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-rockchip.h |   1 +
 2 files changed, 201 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index cc647db76927..15d2045f929e 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -632,6 +632,115 @@ static struct rockchip_mux_recalced_data rk3308_mux_recalced_data[] = {
 	},
 };
 
+static struct rockchip_mux_recalced_data rk3308b_mux_recalced_data[] = {
+	{
+		/* gpio1b6_sel */
+		.num = 1,
+		.pin = 14,
+		.reg = 0x28,
+		.bit = 12,
+		.mask = 0xf
+	}, {
+		/* gpio1b7_sel */
+		.num = 1,
+		.pin = 15,
+		.reg = 0x2c,
+		.bit = 0,
+		.mask = 0x3
+	}, {
+		/* gpio1c2_sel */
+		.num = 1,
+		.pin = 18,
+		.reg = 0x30,
+		.bit = 4,
+		.mask = 0xf
+	}, {
+		/* gpio1c3_sel */
+		.num = 1,
+		.pin = 19,
+		.reg = 0x30,
+		.bit = 8,
+		.mask = 0xf
+	}, {
+		/* gpio1c4_sel */
+		.num = 1,
+		.pin = 20,
+		.reg = 0x30,
+		.bit = 12,
+		.mask = 0xf
+	}, {
+		/* gpio1c5_sel */
+		.num = 1,
+		.pin = 21,
+		.reg = 0x34,
+		.bit = 0,
+		.mask = 0xf
+	}, {
+		/* gpio1c6_sel */
+		.num = 1,
+		.pin = 22,
+		.reg = 0x34,
+		.bit = 4,
+		.mask = 0xf
+	}, {
+		/* gpio1c7_sel */
+		.num = 1,
+		.pin = 23,
+		.reg = 0x34,
+		.bit = 8,
+		.mask = 0xf
+	}, {
+		/* gpio2a2_sel_plus */
+		.num = 2,
+		.pin = 2,
+		.reg = 0x608,
+		.bit = 0,
+		.mask = 0x7
+	}, {
+		/* gpio2a3_sel_plus */
+		.num = 2,
+		.pin = 3,
+		.reg = 0x608,
+		.bit = 4,
+		.mask = 0x7
+	}, {
+		/* gpio2c0_sel_plus */
+		.num = 2,
+		.pin = 16,
+		.reg = 0x610,
+		.bit = 8,
+		.mask = 0x7
+	}, {
+		/* gpio3b2_sel_plus */
+		.num = 3,
+		.pin = 10,
+		.reg = 0x610,
+		.bit = 0,
+		.mask = 0x7
+	}, {
+		/* gpio3b3_sel_plus */
+		.num = 3,
+		.pin = 11,
+		.reg = 0x610,
+		.bit = 4,
+		.mask = 0x7
+	}, {
+		/* gpio3b4_sel */
+		.num = 3,
+		.pin = 12,
+		.reg = 0x68,
+		.bit = 8,
+		.mask = 0xf
+	}, {
+		/* gpio3b5_sel */
+		.num = 3,
+		.pin = 13,
+		.reg = 0x68,
+		.bit = 12,
+		.mask = 0xf
+	},
+};
+
 static struct rockchip_mux_recalced_data rk3328_mux_recalced_data[] = {
 	{
 		.num = 2,
@@ -882,6 +991,35 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
 	RK_MUXROUTE_SAME(2, RK_PA4, 3, 0x600, BIT(16 + 2) | BIT(2)), /* pdm-clkm-m2 */
 };
 
+static struct rockchip_mux_route_data rk3308b_mux_route_data[] = {
+	RK_MUXROUTE_SAME(0, RK_PC3, 1, 0x314, BIT(16 + 0) | BIT(0)), /* rtc_clk */
+	RK_MUXROUTE_SAME(1, RK_PC6, 2, 0x314, BIT(16 + 2) | BIT(16 + 3)), /* uart2_rxm0 */
+	RK_MUXROUTE_SAME(4, RK_PD2, 2, 0x314, BIT(16 + 2) | BIT(16 + 3) | BIT(2)), /* uart2_rxm1 */
+	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x608, BIT(16 + 8) | BIT(16 + 9)), /* i2c3_sdam0 */
+	RK_MUXROUTE_SAME(3, RK_PB4, 2, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(8)), /* i2c3_sdam1 */
+	RK_MUXROUTE_SAME(2, RK_PA0, 3, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(9)), /* i2c3_sdam2 */
+	RK_MUXROUTE_SAME(1, RK_PA3, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclktxm0 */
+	RK_MUXROUTE_SAME(1, RK_PA4, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclkrxm0 */
+	RK_MUXROUTE_SAME(1, RK_PB5, 2, 0x308, BIT(16 + 3) | BIT(3)), /* i2s-8ch-1-sclktxm1 */
+	RK_MUXROUTE_SAME(1, RK_PB6, 2, 0x308, BIT(16 + 3) | BIT(3)), /* i2s-8ch-1-sclkrxm1 */
+	RK_MUXROUTE_SAME(1, RK_PA4, 3, 0x308, BIT(16 + 12) | BIT(16 + 13)), /* pdm-clkm0 */
+	RK_MUXROUTE_SAME(1, RK_PB6, 4, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* pdm-clkm1 */
+	RK_MUXROUTE_SAME(2, RK_PA6, 2, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* pdm-clkm2 */
+	RK_MUXROUTE_SAME(2, RK_PA4, 3, 0x600, BIT(16 + 2) | BIT(2)), /* pdm-clkm-m2 */
+	RK_MUXROUTE_SAME(3, RK_PB2, 3, 0x314, BIT(16 + 9)), /* spi1_miso */
+	RK_MUXROUTE_SAME(2, RK_PA4, 2, 0x314, BIT(16 + 9) | BIT(9)), /* spi1_miso_m1 */
+	RK_MUXROUTE_SAME(0, RK_PB3, 3, 0x314, BIT(16 + 10) | BIT(16 + 11)), /* owire_m0 */
+	RK_MUXROUTE_SAME(1, RK_PC6, 7, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(10)), /* owire_m1 */
+	RK_MUXROUTE_SAME(2, RK_PA2, 5, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(11)), /* owire_m2 */
+	RK_MUXROUTE_SAME(0, RK_PB3, 2, 0x314, BIT(16 + 12) | BIT(16 + 13)), /* can_rxd_m0 */
+	RK_MUXROUTE_SAME(1, RK_PC6, 5, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* can_rxd_m1 */
+	RK_MUXROUTE_SAME(2, RK_PA2, 4, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* can_rxd_m2 */
+	RK_MUXROUTE_SAME(1, RK_PC4, 3, 0x314, BIT(16 + 14)), /* mac_rxd0_m0 */
+	RK_MUXROUTE_SAME(4, RK_PA2, 2, 0x314, BIT(16 + 14) | BIT(14)), /* mac_rxd0_m1 */
+	RK_MUXROUTE_SAME(3, RK_PB4, 4, 0x314, BIT(16 + 15)), /* uart3_rx */
+	RK_MUXROUTE_SAME(0, RK_PC1, 3, 0x314, BIT(16 + 15) | BIT(15)), /* uart3_rx_m1 */
+};
+
 static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
 	RK_MUXROUTE_SAME(1, RK_PA1, 2, 0x50, BIT(16) | BIT(16 + 1)), /* uart2dbg_rxm0 */
 	RK_MUXROUTE_SAME(2, RK_PA1, 1, 0x50, BIT(16) | BIT(16 + 1) | BIT(0)), /* uart2dbg_rxm1 */
@@ -2420,6 +2558,7 @@ static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
 	case RK3188:
 	case RK3288:
 	case RK3308:
+	case RK3308B:
 	case RK3368:
 	case RK3399:
 	case RK3568:
@@ -2478,6 +2617,7 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 	case RK3188:
 	case RK3288:
 	case RK3308:
+	case RK3308B:
 	case RK3368:
 	case RK3399:
 	case RK3568:
@@ -2740,6 +2880,7 @@ static bool rockchip_pinconf_pull_valid(struct rockchip_pin_ctrl *ctrl,
 	case RK3188:
 	case RK3288:
 	case RK3308:
+	case RK3308B:
 	case RK3368:
 	case RK3399:
 	case RK3568:
@@ -3338,6 +3479,42 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
 			 rockchip_pinctrl_resume);
 
+#define RK3308B_GRF_SOC_CON13			0x608
+#define RK3308B_GRF_SOC_CON15			0x610
+
+/* RK3308B_GRF_SOC_CON13 */
+#define RK3308B_GRF_I2C3_IOFUNC_SRC_CTRL	(BIT(16 + 10) | BIT(10))
+#define RK3308B_GRF_GPIO2A3_SEL_SRC_CTRL	(BIT(16 + 7)  | BIT(7))
+#define RK3308B_GRF_GPIO2A2_SEL_SRC_CTRL	(BIT(16 + 3)  | BIT(3))
+
+/* RK3308B_GRF_SOC_CON15 */
+#define RK3308B_GRF_GPIO2C0_SEL_SRC_CTRL	(BIT(16 + 11) | BIT(11))
+#define RK3308B_GRF_GPIO3B3_SEL_SRC_CTRL	(BIT(16 + 7)  | BIT(7))
+#define RK3308B_GRF_GPIO3B2_SEL_SRC_CTRL	(BIT(16 + 3)  | BIT(3))
+
+/*
+ * RK3308B has 3bit gpio##_sel_plus iomuxes over some 2bit old ones.
+ * Put them in use by initializing gpio##_sel_src_ctrl registers.
+ */
+static int rk3308b_soc_sel_src_init(struct rockchip_pinctrl *info)
+{
+	int ret;
+
+	ret = regmap_write(info->regmap_base, RK3308B_GRF_SOC_CON13,
+			   RK3308B_GRF_I2C3_IOFUNC_SRC_CTRL |
+			   RK3308B_GRF_GPIO2A3_SEL_SRC_CTRL |
+			   RK3308B_GRF_GPIO2A2_SEL_SRC_CTRL);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(info->regmap_base, RK3308B_GRF_SOC_CON15,
+			   RK3308B_GRF_GPIO2C0_SEL_SRC_CTRL |
+			   RK3308B_GRF_GPIO3B3_SEL_SRC_CTRL |
+			   RK3308B_GRF_GPIO3B2_SEL_SRC_CTRL);
+
+	return ret;
+};
+
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info;
@@ -3403,6 +3580,12 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 			return PTR_ERR(info->regmap_pmu);
 	}
 
+	if (ctrl->type == RK3308B) {
+		ret = rk3308b_soc_sel_src_init(info);
+		if (ret)
+			return ret;
+	}
+
 	ret = rockchip_pinctrl_register(pdev, info);
 	if (ret)
 		return ret;
@@ -3746,6 +3929,21 @@ static struct rockchip_pin_ctrl rk3308_pin_ctrl = {
 		.schmitt_calc_reg	= rk3308_calc_schmitt_reg_and_bit,
 };
 
+static struct rockchip_pin_ctrl rk3308b_pin_ctrl = {
+		.pin_banks		= rk3308_pin_banks,
+		.nr_banks		= ARRAY_SIZE(rk3308_pin_banks),
+		.label			= "RK3308b-GPIO",
+		.type			= RK3308B,
+		.grf_mux_offset		= 0x0,
+		.iomux_recalced		= rk3308b_mux_recalced_data,
+		.niomux_recalced	= ARRAY_SIZE(rk3308b_mux_recalced_data),
+		.iomux_routes		= rk3308b_mux_route_data,
+		.niomux_routes		= ARRAY_SIZE(rk3308b_mux_route_data),
+		.pull_calc_reg		= rk3308_calc_pull_reg_and_bit,
+		.drv_calc_reg		= rk3308_calc_drv_reg_and_bit,
+		.schmitt_calc_reg	= rk3308_calc_schmitt_reg_and_bit,
+};
+
 static struct rockchip_pin_bank rk3328_pin_banks[] = {
 	PIN_BANK_IOMUX_FLAGS(0, 32, "gpio0", 0, 0, 0, 0),
 	PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", 0, 0, 0, 0),
@@ -3952,6 +4150,8 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
 		.data = &rk3288_pin_ctrl },
 	{ .compatible = "rockchip,rk3308-pinctrl",
 		.data = &rk3308_pin_ctrl },
+	{ .compatible = "rockchip,rk3308b-pinctrl",
+		.data = &rk3308b_pin_ctrl },
 	{ .compatible = "rockchip,rk3328-pinctrl",
 		.data = &rk3328_pin_ctrl },
 	{ .compatible = "rockchip,rk3368-pinctrl",
diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h
index 4759f336941e..3af5b1bd626b 100644
--- a/drivers/pinctrl/pinctrl-rockchip.h
+++ b/drivers/pinctrl/pinctrl-rockchip.h
@@ -193,6 +193,7 @@ enum rockchip_pinctrl_type {
 	RK3188,
 	RK3288,
 	RK3308,
+	RK3308B,
 	RK3368,
 	RK3399,
 	RK3568,
-- 
2.39.2


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

* Re: [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b
  2024-05-15 12:16 ` [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b Dmitry Yashin
@ 2024-05-15 16:17   ` Conor Dooley
  0 siblings, 0 replies; 16+ messages in thread
From: Conor Dooley @ 2024-05-15 16:17 UTC (permalink / raw)
  To: Dmitry Yashin
  Cc: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring, Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel

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

On Wed, May 15, 2024 at 05:16:33PM +0500, Dmitry Yashin wrote:
> Add compatible string for rk3308b pin controller.
> 
> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

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

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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-15 12:16 ` [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
@ 2024-05-15 16:29   ` Luca Ceresoli
  2024-05-16 12:06     ` Dmitry Yashin
  2024-05-15 17:00   ` Christophe JAILLET
  1 sibling, 1 reply; 16+ messages in thread
From: Luca Ceresoli @ 2024-05-15 16:29 UTC (permalink / raw)
  To: Dmitry Yashin
  Cc: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel

Hello Dmitry,

On Wed, 15 May 2024 17:16:34 +0500
Dmitry Yashin <dmt.yashin@gmail.com> wrote:

> Add pinctrl support for rk3308b. This pin controller much the same as
> rk3308's, but with additional iomux routes and 3bit iomuxes selected
> via gpio##_sel_src_ctrl registers. Set them up in the function
> rk3308b_soc_sel_src_init to use new 3bit iomuxes over some 2bit old ones.
> 
> Fixes: 1f3e25a06883 ("pinctrl: rockchip: fix RK3308 pinmux bits")
> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>

Thanks for the effort! I have one high-level remark, see below.
Otherwise at a superficial look it looks good.

> @@ -3952,6 +4150,8 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
>  		.data = &rk3288_pin_ctrl },
>  	{ .compatible = "rockchip,rk3308-pinctrl",
>  		.data = &rk3308_pin_ctrl },
> +	{ .compatible = "rockchip,rk3308b-pinctrl",
> +		.data = &rk3308b_pin_ctrl },

I'm skeptical about this being bound to a new DT compatible. As far as I
know the RK3308 and RK3308B are mostly equivalent, so it looks as the
pinctrl implementation could be detected at runtime. This would let
products to be built with either chip version and work on any without
any DT change.

Code for reading the chip ID is in the RK3308 codec driver [0].

[0] https://lore.kernel.org/all/20240305-rk3308-audio-codec-v4-4-312acdbe628f@bootlin.com/ -> search "GRF_CHIP_ID"

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-15 12:16 ` [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
  2024-05-15 16:29   ` Luca Ceresoli
@ 2024-05-15 17:00   ` Christophe JAILLET
  1 sibling, 0 replies; 16+ messages in thread
From: Christophe JAILLET @ 2024-05-15 17:00 UTC (permalink / raw)
  To: Dmitry Yashin, Linus Walleij, Heiko Stuebner,
	Krzysztof Kozlowski, Conor Dooley, Rob Herring
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel

Le 15/05/2024 à 14:16, Dmitry Yashin a écrit :
> Add pinctrl support for rk3308b. This pin controller much the same as
> rk3308's, but with additional iomux routes and 3bit iomuxes selected
> via gpio##_sel_src_ctrl registers. Set them up in the function
> rk3308b_soc_sel_src_init to use new 3bit iomuxes over some 2bit old ones.
> 
> Fixes: 1f3e25a06883 ("pinctrl: rockchip: fix RK3308 pinmux bits")
> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
> ---
>   drivers/pinctrl/pinctrl-rockchip.c | 200 +++++++++++++++++++++++++++++
>   drivers/pinctrl/pinctrl-rockchip.h |   1 +
>   2 files changed, 201 insertions(+)

Hi,

> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index cc647db76927..15d2045f929e 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -632,6 +632,115 @@ static struct rockchip_mux_recalced_data rk3308_mux_recalced_data[] = {
>   	},
>   };
>   
> +static struct rockchip_mux_recalced_data rk3308b_mux_recalced_data[] = {

It is likely that it would require some other changes, but it looks to 
be a good candidate for static const struct.

> +	{
> +		/* gpio1b6_sel */
> +		.num = 1,
> +		.pin = 14,
> +		.reg = 0x28,
> +		.bit = 12,
> +		.mask = 0xf
> +	}, {

...

> @@ -882,6 +991,35 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
>   	RK_MUXROUTE_SAME(2, RK_PA4, 3, 0x600, BIT(16 + 2) | BIT(2)), /* pdm-clkm-m2 */
>   };
>   
> +static struct rockchip_mux_route_data rk3308b_mux_route_data[] = {

Same

> +	RK_MUXROUTE_SAME(0, RK_PC3, 1, 0x314, BIT(16 + 0) | BIT(0)), /* rtc_clk */
> +	RK_MUXROUTE_SAME(1, RK_PC6, 2, 0x314, BIT(16 + 2) | BIT(16 + 3)), /* uart2_rxm0 */
> +	RK_MUXROUTE_SAME(4, RK_PD2, 2, 0x314, BIT(16 + 2) | BIT(16 + 3) | BIT(2)), /* uart2_rxm1 */
> +	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x608, BIT(16 + 8) | BIT(16 + 9)), /* i2c3_sdam0 */

...

> @@ -3746,6 +3929,21 @@ static struct rockchip_pin_ctrl rk3308_pin_ctrl = {
>   		.schmitt_calc_reg	= rk3308_calc_schmitt_reg_and_bit,
>   };
>   
> +static struct rockchip_pin_ctrl rk3308b_pin_ctrl = {

This one could be constify without any other changes I think (and also 
makes more sense because of its function pointers)

Just my 2c.

CJ

> +		.pin_banks		= rk3308_pin_banks,
> +		.nr_banks		= ARRAY_SIZE(rk3308_pin_banks),
> +		.label			= "RK3308b-GPIO",
> +		.type			= RK3308B,
> +		.grf_mux_offset		= 0x0,
> +		.iomux_recalced		= rk3308b_mux_recalced_data,
> +		.niomux_recalced	= ARRAY_SIZE(rk3308b_mux_recalced_data),
> +		.iomux_routes		= rk3308b_mux_route_data,
> +		.niomux_routes		= ARRAY_SIZE(rk3308b_mux_route_data),
> +		.pull_calc_reg		= rk3308_calc_pull_reg_and_bit,
> +		.drv_calc_reg		= rk3308_calc_drv_reg_and_bit,
> +		.schmitt_calc_reg	= rk3308_calc_schmitt_reg_and_bit,
> +};

...


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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-15 16:29   ` Luca Ceresoli
@ 2024-05-16 12:06     ` Dmitry Yashin
  2024-05-17  6:58       ` Luca Ceresoli
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-16 12:06 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

Hi Luca,

On 15.05.24 21:29, Luca Ceresoli wrote:
> I'm skeptical about this being bound to a new DT compatible. As far as I
> know the RK3308 and RK3308B are mostly equivalent, so it looks as the
> pinctrl implementation could be detected at runtime. This would let
> products to be built with either chip version and work on any without
> any DT change.


Thanks for your feedback.

Indeed, these SoC's have a lot in common, but as I can see the rk3308b
has more blocks, like extra PWM's (rk3308 datasheet 1.5 [0] shows only
1x PWM 4ch, when rk3308b and rk3308b-s have 3x PWM 4ch), 1-wire and
CAN controller (mentioned in the TRM, but dropped from rk3308b
datasheet for some reason).

So, in my view, it really makes sense to add rk3308b.dtsi, where extra
PWM's, pinctrl compatible and its pin functions can be moved. And if
its not worth it, then I will try to adapt the entire series to runtime
config based on cpuid like you suggested.

Additional thoughts on this would be appreciated.

[0] https://rockchip.fr/RK3308%20datasheet%20V1.5.pdf

-- 
Thanks,
Dmitry


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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-16 12:06     ` Dmitry Yashin
@ 2024-05-17  6:58       ` Luca Ceresoli
  2024-05-28  8:17         ` Heiko Stübner
  0 siblings, 1 reply; 16+ messages in thread
From: Luca Ceresoli @ 2024-05-17  6:58 UTC (permalink / raw)
  To: Dmitry Yashin
  Cc: Linus Walleij, Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley,
	Rob Herring, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel

Hello Dmitry,

On Thu, 16 May 2024 17:06:46 +0500
Dmitry Yashin <dmt.yashin@gmail.com> wrote:

> Hi Luca,
> 
> On 15.05.24 21:29, Luca Ceresoli wrote:
> > I'm skeptical about this being bound to a new DT compatible. As far as I
> > know the RK3308 and RK3308B are mostly equivalent, so it looks as the
> > pinctrl implementation could be detected at runtime. This would let
> > products to be built with either chip version and work on any without
> > any DT change.  
> 
> 
> Thanks for your feedback.
> 
> Indeed, these SoC's have a lot in common, but as I can see the rk3308b
> has more blocks, like extra PWM's (rk3308 datasheet 1.5 [0] shows only
> 1x PWM 4ch, when rk3308b and rk3308b-s have 3x PWM 4ch), 1-wire and
> CAN controller (mentioned in the TRM, but dropped from rk3308b
> datasheet for some reason).
> 
> So, in my view, it really makes sense to add rk3308b.dtsi, where extra
> PWM's, pinctrl compatible and its pin functions can be moved. And if
> its not worth it, then I will try to adapt the entire series to runtime
> config based on cpuid like you suggested.

Having a rk3308b.dtsi would probably make sense, yes, as there are
several differences as you described. However for the pinctrl it seems
probably not necessary.

I've seen actual products being manufactured with two different RK3308
variants in different lots of production, but with the same DT that has
rockchip,rk3308-pinctrl in it. Those would need a _selective_ DT
upgrade in order to benefit from your changes.

And even if a product had always used the B variant, it would need DT
upgrade when upgrading to a kernel with your changes. Otherwise with
patch 1/3 of this series the pictrl driver would lose many routes after
upgrading the kernel (but not the DT): can this lead to
previously-working devices to stop working? I think this is a
fundamental question to reply.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-17  6:58       ` Luca Ceresoli
@ 2024-05-28  8:17         ` Heiko Stübner
  2024-05-28  8:43           ` Jonas Karlman
  0 siblings, 1 reply; 16+ messages in thread
From: Heiko Stübner @ 2024-05-28  8:17 UTC (permalink / raw)
  To: Dmitry Yashin, Luca Ceresoli
  Cc: Linus Walleij, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Jianqun Xu, devicetree, linux-gpio, linux-arm-kernel,
	linux-rockchip, linux-kernel

Am Freitag, 17. Mai 2024, 08:58:32 CEST schrieb Luca Ceresoli:
> Hello Dmitry,
> 
> On Thu, 16 May 2024 17:06:46 +0500
> Dmitry Yashin <dmt.yashin@gmail.com> wrote:
> 
> > Hi Luca,
> > 
> > On 15.05.24 21:29, Luca Ceresoli wrote:
> > > I'm skeptical about this being bound to a new DT compatible. As far as I
> > > know the RK3308 and RK3308B are mostly equivalent, so it looks as the
> > > pinctrl implementation could be detected at runtime. This would let
> > > products to be built with either chip version and work on any without
> > > any DT change.  
> > 
> > 
> > Thanks for your feedback.
> > 
> > Indeed, these SoC's have a lot in common, but as I can see the rk3308b
> > has more blocks, like extra PWM's (rk3308 datasheet 1.5 [0] shows only
> > 1x PWM 4ch, when rk3308b and rk3308b-s have 3x PWM 4ch), 1-wire and
> > CAN controller (mentioned in the TRM, but dropped from rk3308b
> > datasheet for some reason).
> > 
> > So, in my view, it really makes sense to add rk3308b.dtsi, where extra
> > PWM's, pinctrl compatible and its pin functions can be moved. And if
> > its not worth it, then I will try to adapt the entire series to runtime
> > config based on cpuid like you suggested.
> 
> Having a rk3308b.dtsi would probably make sense, yes, as there are
> several differences as you described. However for the pinctrl it seems
> probably not necessary.
> 
> I've seen actual products being manufactured with two different RK3308
> variants in different lots of production, but with the same DT that has
> rockchip,rk3308-pinctrl in it. Those would need a _selective_ DT
> upgrade in order to benefit from your changes.
> 
> And even if a product had always used the B variant, it would need DT
> upgrade when upgrading to a kernel with your changes. Otherwise with
> patch 1/3 of this series the pictrl driver would lose many routes after
> upgrading the kernel (but not the DT): can this lead to
> previously-working devices to stop working? I think this is a
> fundamental question to reply.

If things can be runtime-detectable, they should be detected at runtime.
So yes, while we need to know that it is a rk3308-something before
via the dt, if we can distinguish between the rk3308 variants at runtime
we should definitly do so.

Heiko



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

* Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
  2024-05-28  8:17         ` Heiko Stübner
@ 2024-05-28  8:43           ` Jonas Karlman
  0 siblings, 0 replies; 16+ messages in thread
From: Jonas Karlman @ 2024-05-28  8:43 UTC (permalink / raw)
  To: Heiko Stübner, Dmitry Yashin, Luca Ceresoli
  Cc: Linus Walleij, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Jianqun Xu, devicetree, linux-gpio, linux-arm-kernel,
	linux-rockchip, linux-kernel

On 2024-05-28 10:17, Heiko Stübner wrote:
> Am Freitag, 17. Mai 2024, 08:58:32 CEST schrieb Luca Ceresoli:
>> Hello Dmitry,
>>
>> On Thu, 16 May 2024 17:06:46 +0500
>> Dmitry Yashin <dmt.yashin@gmail.com> wrote:
>>
>>> Hi Luca,
>>>
>>> On 15.05.24 21:29, Luca Ceresoli wrote:
>>>> I'm skeptical about this being bound to a new DT compatible. As far as I
>>>> know the RK3308 and RK3308B are mostly equivalent, so it looks as the
>>>> pinctrl implementation could be detected at runtime. This would let
>>>> products to be built with either chip version and work on any without
>>>> any DT change.  
>>>
>>>
>>> Thanks for your feedback.
>>>
>>> Indeed, these SoC's have a lot in common, but as I can see the rk3308b
>>> has more blocks, like extra PWM's (rk3308 datasheet 1.5 [0] shows only
>>> 1x PWM 4ch, when rk3308b and rk3308b-s have 3x PWM 4ch), 1-wire and
>>> CAN controller (mentioned in the TRM, but dropped from rk3308b
>>> datasheet for some reason).
>>>
>>> So, in my view, it really makes sense to add rk3308b.dtsi, where extra
>>> PWM's, pinctrl compatible and its pin functions can be moved. And if
>>> its not worth it, then I will try to adapt the entire series to runtime
>>> config based on cpuid like you suggested.
>>
>> Having a rk3308b.dtsi would probably make sense, yes, as there are
>> several differences as you described. However for the pinctrl it seems
>> probably not necessary.
>>
>> I've seen actual products being manufactured with two different RK3308
>> variants in different lots of production, but with the same DT that has
>> rockchip,rk3308-pinctrl in it. Those would need a _selective_ DT
>> upgrade in order to benefit from your changes.
>>
>> And even if a product had always used the B variant, it would need DT
>> upgrade when upgrading to a kernel with your changes. Otherwise with
>> patch 1/3 of this series the pictrl driver would lose many routes after
>> upgrading the kernel (but not the DT): can this lead to
>> previously-working devices to stop working? I think this is a
>> fundamental question to reply.
> 
> If things can be runtime-detectable, they should be detected at runtime.
> So yes, while we need to know that it is a rk3308-something before
> via the dt, if we can distinguish between the rk3308 variants at runtime
> we should definitly do so.

The GRF_CHIP_ID reg (0xFF000800) can be used to identify what model is
used at runtime:

RK3308: 0xCEA (errata: chip id value is 32'h0cea (32'd3306))
RK3308B: 0x3308
RK3308BS: 0x3308C

Vendor U-Boot make use of this reg to identify what model is running:
https://github.com/rockchip-linux/u-boot/blob/next-dev/arch/arm/include/asm/arch-rockchip/cpu.h#L68-L82

I can only validate on real hw that the reg value is 0x3308 for RK3308B.

Regards,
Jonas

> 
> Heiko
> 


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

* Re: [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
@ 2024-05-28 11:29   ` Linus Walleij
  2024-05-28 11:52     ` Heiko Stübner
  2024-05-28 11:50   ` Heiko Stübner
  1 sibling, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2024-05-28 11:29 UTC (permalink / raw)
  To: Dmitry Yashin
  Cc: Heiko Stuebner, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel

On Wed, May 15, 2024 at 2:17 PM Dmitry Yashin <dmt.yashin@gmail.com> wrote:

> Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
> the rk3308b SoC. Remove them and correct i2c3 routes.
>
> Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>

While you guys are thinking about the RK3308B support, is this fix
something I can just apply?

Yours,
Linus Walleij

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

* Re: [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
  2024-05-28 11:29   ` Linus Walleij
@ 2024-05-28 11:50   ` Heiko Stübner
  1 sibling, 0 replies; 16+ messages in thread
From: Heiko Stübner @ 2024-05-28 11:50 UTC (permalink / raw)
  To: Linus Walleij, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Dmitry Yashin
  Cc: Luca Ceresoli, Jianqun Xu, devicetree, linux-gpio,
	linux-arm-kernel, linux-rockchip, linux-kernel, Dmitry Yashin

Am Mittwoch, 15. Mai 2024, 14:16:32 CEST schrieb Dmitry Yashin:
> Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
> the rk3308b SoC. Remove them and correct i2c3 routes.
> 
> Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 17 ++---------------
>  1 file changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 3bedf36a0019..cc647db76927 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -870,9 +870,8 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
>  	RK_MUXROUTE_SAME(0, RK_PC3, 1, 0x314, BIT(16 + 0) | BIT(0)), /* rtc_clk */
>  	RK_MUXROUTE_SAME(1, RK_PC6, 2, 0x314, BIT(16 + 2) | BIT(16 + 3)), /* uart2_rxm0 */
>  	RK_MUXROUTE_SAME(4, RK_PD2, 2, 0x314, BIT(16 + 2) | BIT(16 + 3) | BIT(2)), /* uart2_rxm1 */
> -	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x608, BIT(16 + 8) | BIT(16 + 9)), /* i2c3_sdam0 */
> -	RK_MUXROUTE_SAME(3, RK_PB4, 2, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(8)), /* i2c3_sdam1 */
> -	RK_MUXROUTE_SAME(2, RK_PA0, 3, 0x608, BIT(16 + 8) | BIT(16 + 9) | BIT(9)), /* i2c3_sdam2 */
> +	RK_MUXROUTE_SAME(0, RK_PB7, 2, 0x314, BIT(16 + 4)), /* i2c3_sdam0 */
> +	RK_MUXROUTE_SAME(3, RK_PB4, 2, 0x314, BIT(16 + 4) | BIT(4)), /* i2c3_sdam1 */

checked against TRM and no devicetree in the kernel currently enables i2c3 at all.

>  	RK_MUXROUTE_SAME(1, RK_PA3, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclktxm0 */
>  	RK_MUXROUTE_SAME(1, RK_PA4, 2, 0x308, BIT(16 + 3)), /* i2s-8ch-1-sclkrxm0 */
>  	RK_MUXROUTE_SAME(1, RK_PB5, 2, 0x308, BIT(16 + 3) | BIT(3)), /* i2s-8ch-1-sclktxm1 */
> @@ -881,18 +880,6 @@ static struct rockchip_mux_route_data rk3308_mux_route_data[] = {
>  	RK_MUXROUTE_SAME(1, RK_PB6, 4, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* pdm-clkm1 */
>  	RK_MUXROUTE_SAME(2, RK_PA6, 2, 0x308, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* pdm-clkm2 */
>  	RK_MUXROUTE_SAME(2, RK_PA4, 3, 0x600, BIT(16 + 2) | BIT(2)), /* pdm-clkm-m2 */
> -	RK_MUXROUTE_SAME(3, RK_PB2, 3, 0x314, BIT(16 + 9)), /* spi1_miso */
> -	RK_MUXROUTE_SAME(2, RK_PA4, 2, 0x314, BIT(16 + 9) | BIT(9)), /* spi1_miso_m1 */
> -	RK_MUXROUTE_SAME(0, RK_PB3, 3, 0x314, BIT(16 + 10) | BIT(16 + 11)), /* owire_m0 */
> -	RK_MUXROUTE_SAME(1, RK_PC6, 7, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(10)), /* owire_m1 */
> -	RK_MUXROUTE_SAME(2, RK_PA2, 5, 0x314, BIT(16 + 10) | BIT(16 + 11) | BIT(11)), /* owire_m2 */
> -	RK_MUXROUTE_SAME(0, RK_PB3, 2, 0x314, BIT(16 + 12) | BIT(16 + 13)), /* can_rxd_m0 */
> -	RK_MUXROUTE_SAME(1, RK_PC6, 5, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(12)), /* can_rxd_m1 */
> -	RK_MUXROUTE_SAME(2, RK_PA2, 4, 0x314, BIT(16 + 12) | BIT(16 + 13) | BIT(13)), /* can_rxd_m2 */

> -	RK_MUXROUTE_SAME(1, RK_PC4, 3, 0x314, BIT(16 + 14)), /* mac_rxd0_m0 */
> -	RK_MUXROUTE_SAME(4, RK_PA2, 2, 0x314, BIT(16 + 14) | BIT(14)), /* mac_rxd0_m1 */

> -	RK_MUXROUTE_SAME(3, RK_PB4, 4, 0x314, BIT(16 + 15)), /* uart3_rx */
> -	RK_MUXROUTE_SAME(0, RK_PC1, 3, 0x314, BIT(16 + 15) | BIT(15)), /* uart3_rx_m1 */

checked against the TRM too.
And as far as usage:
- spi1: not used at all
- owire: not used at all
- can: not used at all
- mac_rxd0: all boards use the (default) m0 variant
- uart3: not currently enabled anywhere


And I guess the rk3308 is niche enough that people either use the vendor-kernel
or have the dts in mainline, so we should have a ok grasp on not causing breakage
by some accidental rk3308b somewhere.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>




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

* Re: [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-28 11:29   ` Linus Walleij
@ 2024-05-28 11:52     ` Heiko Stübner
  2024-05-28 12:18       ` Dmitry Yashin
  0 siblings, 1 reply; 16+ messages in thread
From: Heiko Stübner @ 2024-05-28 11:52 UTC (permalink / raw)
  To: Dmitry Yashin, Linus Walleij
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring, Luca Ceresoli,
	Jianqun Xu, devicetree, linux-gpio, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Linus,

Am Dienstag, 28. Mai 2024, 13:29:12 CEST schrieb Linus Walleij:
> On Wed, May 15, 2024 at 2:17 PM Dmitry Yashin <dmt.yashin@gmail.com> wrote:
> 
> > Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
> > the rk3308b SoC. Remove them and correct i2c3 routes.
> >
> > Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
> > Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
> 
> While you guys are thinking about the RK3308B support, is this fix
> something I can just apply?

I'd think so. I've detailed stuff in my Review mail I just sent.
Both the soc itself and also the affected pin functions are niche
enough that this should not cause breakage.


Heiko



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

* Re: [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-28 11:52     ` Heiko Stübner
@ 2024-05-28 12:18       ` Dmitry Yashin
  2024-05-28 13:23         ` Heiko Stübner
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Yashin @ 2024-05-28 12:18 UTC (permalink / raw)
  To: Heiko Stübner, Linus Walleij
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring, Luca Ceresoli,
	Jianqun Xu, devicetree, linux-gpio, linux-arm-kernel,
	linux-rockchip, linux-kernel, Dmitry Yashin

Hi Heiko,

On 5/28/24 4:52 PM, Heiko Stübner wrote:
> Hi Linus,
>
> Am Dienstag, 28. Mai 2024, 13:29:12 CEST schrieb Linus Walleij:
>> On Wed, May 15, 2024 at 2:17 PM Dmitry Yashin <dmt.yashin@gmail.com> wrote:
>>
>>> Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
>>> the rk3308b SoC. Remove them and correct i2c3 routes.
>>>
>>> Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
>>> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
>> While you guys are thinking about the RK3308B support, is this fix
>> something I can just apply?
> I'd think so. I've detailed stuff in my Review mail I just sent.
> Both the soc itself and also the affected pin functions are niche
> enough that this should not cause breakage.
>
>
> Heiko
>
>
>

Should i just drop 1/3 from V2 then?

Thanks everyone for the feedback on this series. I'll prepare V2 based
on runtime chip detection with use of GRF_CHIP_ID.

-- 
Thanks,
Dmitry


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

* Re: [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes
  2024-05-28 12:18       ` Dmitry Yashin
@ 2024-05-28 13:23         ` Heiko Stübner
  0 siblings, 0 replies; 16+ messages in thread
From: Heiko Stübner @ 2024-05-28 13:23 UTC (permalink / raw)
  To: Linus Walleij, Dmitry Yashin
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring, Luca Ceresoli,
	Jianqun Xu, devicetree, linux-gpio, linux-arm-kernel,
	linux-rockchip, linux-kernel, Dmitry Yashin

Am Dienstag, 28. Mai 2024, 14:18:35 CEST schrieb Dmitry Yashin:
> Hi Heiko,
> 
> On 5/28/24 4:52 PM, Heiko Stübner wrote:
> > Hi Linus,
> >
> > Am Dienstag, 28. Mai 2024, 13:29:12 CEST schrieb Linus Walleij:
> >> On Wed, May 15, 2024 at 2:17 PM Dmitry Yashin <dmt.yashin@gmail.com> wrote:
> >>
> >>> Some of the rk3308 iomux routes in rk3308_mux_route_data belong to
> >>> the rk3308b SoC. Remove them and correct i2c3 routes.
> >>>
> >>> Fixes: 7825aeb7b208 ("pinctrl: rockchip: add rk3308 SoC support")
> >>> Signed-off-by: Dmitry Yashin <dmt.yashin@gmail.com>
> >> While you guys are thinking about the RK3308B support, is this fix
> >> something I can just apply?
> > I'd think so. I've detailed stuff in my Review mail I just sent.
> > Both the soc itself and also the affected pin functions are niche
> > enough that this should not cause breakage.
> >
> >
> > Heiko
> >
> >
> >
> 
> Should i just drop 1/3 from V2 then?

I guess just check the state of affairs once your v2 is ready ;-) .

I.e. if LinusW grabs patch1 before you post v2, just drop it, otherwise
send it along.


> Thanks everyone for the feedback on this series. I'll prepare V2 based
> on runtime chip detection with use of GRF_CHIP_ID.





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

end of thread, other threads:[~2024-05-28 13:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-15 12:16 [PATCH 0/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
2024-05-15 12:16 ` [PATCH 1/3] pinctrl: rockchip: update rk3308 iomux routes Dmitry Yashin
2024-05-28 11:29   ` Linus Walleij
2024-05-28 11:52     ` Heiko Stübner
2024-05-28 12:18       ` Dmitry Yashin
2024-05-28 13:23         ` Heiko Stübner
2024-05-28 11:50   ` Heiko Stübner
2024-05-15 12:16 ` [PATCH 2/3] dt-bindings: pinctrl: rockchip: add rk3308b Dmitry Yashin
2024-05-15 16:17   ` Conor Dooley
2024-05-15 12:16 ` [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support Dmitry Yashin
2024-05-15 16:29   ` Luca Ceresoli
2024-05-16 12:06     ` Dmitry Yashin
2024-05-17  6:58       ` Luca Ceresoli
2024-05-28  8:17         ` Heiko Stübner
2024-05-28  8:43           ` Jonas Karlman
2024-05-15 17:00   ` Christophe JAILLET

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