[PATCH 0/2] pinctrl: spacemit: support I/O power domain configuration

[PATCH 0/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

This series adds support for configuring IO power domain voltage for
dual-voltage GPIO banks on the Spacemit K1 SoC.

On K1, IO domain power control registers determine whether a GPIO bank
operates at 1.8V or 3.3V. These registers default to 3.3V operation,
which may lead to functional failures when GPIO banks are externally
supplied with 1.8V but internally remain configured for 3.3V.

The IO power domain registers are implemented as secure registers and
require an explicit unlock sequence via the AIB Secure Access Register
(ASAR), located in the APBC register space.

This series ensures that pin voltage configuration correctly reflects
hardware requirements.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
Troy Mitchell (2):
      dt-bindings: pinctrl: add syscon property
      pinctrl: spacemit: support I/O power domain configuration

 .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      |  11 ++
 arch/riscv/boot/dts/spacemit/k1.dtsi               |   4 +-
 drivers/pinctrl/spacemit/pinctrl-k1.c              | 131 ++++++++++++++++++++-
 3 files changed, 142 insertions(+), 4 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251223-kx-pinctrl-aib-io-pwr-domain-b02da255f95c
prerequisite-change-id: 20251125-02-k3-pinctrl-738cbddbe49d:v1
prerequisite-patch-id: bfa2def3e13eeaff0bb88e5b3411041d4e2a61ca
prerequisite-patch-id: 3a1590265f1222f1497fb55608a09388df3efdef
prerequisite-patch-id: 26d923faae23d9248bbf650f5f8ceb02479466f4

Best regards,
-- 
Troy Mitchell <troy.mitchell@linux.spacemit.com>

[PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Troy Mitchell]

In order to access the protected IO power domain registers, a valid
unlock sequence must be performed by writing the required keys to the
AIB Secure Access Register (ASAR).

The ASAR register resides within the APBC register address space.
A corresponding syscon property is added to allow the pinctrl driver
to access this register.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 .../devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
index c5b0218ad6251f97b1f27089ffff724a7b0f69ae..4dc49c2cc1d52008ad89896ae0419091802cd2c4 100644
--- a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
@@ -32,6 +32,15 @@ properties:
   resets:
     maxItems: 1
 
+  spacemit,apbc:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to syscon that access the protected register
+          - description: offset of access secure registers
+    description:
+      A phandle to syscon with byte offset to access the protected register
+
 patternProperties:
   '-cfg$':
     type: object
@@ -111,6 +120,7 @@ required:
   - reg
   - clocks
   - clock-names
+  - spacemit,apbc
 
 additionalProperties: false
 
@@ -128,6 +138,7 @@ examples:
             clocks = <&syscon_apbc 42>,
                      <&syscon_apbc 94>;
             clock-names = "func", "bus";
+            spacemit,apbc = <&syscon_apbc 0x50>;
 
             uart0_2_cfg: uart0-2-cfg {
                 uart0-2-pins {

-- 
2.52.0

[PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

IO domain power control registers are used to configure the operating
voltage of dual-voltage GPIO banks. By default, these registers are
configured for 3.3V operation. As a result, even when a GPIO bank is
externally supplied with 1.8V, the internal logic continues to
operate in the 3.3V domain, which may lead to functional failures.

This patch adds support for programming the IO domain power control
registers, allowing dual-voltage GPIO banks to be explicitly configured
for 1.8V operation when required.

Care must be taken when configuring these registers. If a GPIO bank is
externally supplied with 3.3V while the corresponding IO power domain
is configured for 1.8V, external current injection (back-powering)
may occur, potentially causing damage to the GPIO pin.

Due to these hardware constraints and safety considerations, the IO
domain power control registers are implemented as secure registers.
Access to these registers requires unlocking via the AIB Secure Access
Register (ASAR) in the APBC block before a single read or write
operation can be performed.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
 drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
 2 files changed, 131 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
 
 		pinctrl: pinctrl@d401e000 {
 			compatible = "spacemit,k1-pinctrl";
-			reg = <0x0 0xd401e000 0x0 0x400>;
+			reg = <0x0 0xd401e000 0x0 0x400>,
+			      <0x0 0xd401e800 0x0 0x34>;
 			clocks = <&syscon_apbc CLK_AIB>,
 				 <&syscon_apbc CLK_AIB_BUS>;
 			clock-names = "func", "bus";
+			spacemit,apbc = <&syscon_apbc 0x50>;
 		};
 
 		pwm8: pwm@d4020000 {
diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
index 8ca247fb8ba0321c02423f9739130e03277d1053..b3ffb32f88a79ebf6b64e62a7846df60b92799fe 100644
--- a/drivers/pinctrl/spacemit/pinctrl-k1.c
+++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
@@ -7,8 +7,10 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/seq_file.h>
 #include <linux/spinlock.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 
@@ -47,6 +49,25 @@
 #define PAD_PULLUP		BIT(14)
 #define PAD_PULL_EN		BIT(15)
 
+#define IO_PWR_DOMAIN_GPIO2_Kx  0x0c
+#define IO_PWR_DOMAIN_MMC_Kx    0x1c
+
+#define IO_PWR_DOMAIN_GPIO3_K1  0x10
+#define IO_PWR_DOMAIN_QSPI_K1   0x20
+
+#define IO_PWR_DOMAIN_GPIO1_K3  0x04
+#define IO_PWR_DOMAIN_GPIO5_K3  0x10
+#define IO_PWR_DOMAIN_GPIO4_K3  0x20
+#define IO_PWR_DOMAIN_QSPI_K3   0x2c
+
+#define IO_PWR_DOMAIN_V18EN	BIT(2)
+
+#define APBC_ASFAR		0x00
+#define APBC_ASSAR		0x04
+
+#define APBC_ASFAR_AKEY		0xbaba
+#define APBC_ASSAR_AKEY		0xeb10
+
 struct spacemit_pin_drv_strength {
 	u8		val;
 	u32		mA;
@@ -78,6 +99,10 @@ struct spacemit_pinctrl {
 	raw_spinlock_t				lock;
 
 	void __iomem				*regs;
+	void __iomem				*io_pd_reg;
+
+	struct regmap				*regmap_apbc;
+	u32					regmap_apbc_offset;
 };
 
 struct spacemit_pinctrl_data {
@@ -85,6 +110,7 @@ struct spacemit_pinctrl_data {
 	const struct spacemit_pin	*data;
 	u16				npins;
 	unsigned int			(*pin_to_offset)(unsigned int pin);
+	unsigned int			(*pin_to_io_pd_offset)(unsigned int pin);
 	const struct spacemit_pinctrl_dconf	*dconf;
 };
 
@@ -146,6 +172,56 @@ static unsigned int spacemit_k3_pin_to_offset(unsigned int pin)
 	return offset << 2;
 }
 
+static unsigned int spacemit_k1_pin_to_io_pd_offset(unsigned int pin)
+{
+	unsigned int offset = 0;
+
+	switch (pin) {
+	case 47 ... 52:
+		offset = IO_PWR_DOMAIN_GPIO3_K1;
+		break;
+	case 75 ... 80:
+		offset = IO_PWR_DOMAIN_GPIO2_Kx;
+		break;
+	case 98 ... 103:
+		offset = IO_PWR_DOMAIN_QSPI_K1;
+		break;
+	case 104 ... 109:
+		offset = IO_PWR_DOMAIN_MMC_Kx;
+		break;
+	}
+
+	return offset;
+}
+
+static unsigned int spacemit_k3_pin_to_io_pd_offset(unsigned int pin)
+{
+	unsigned int offset = 0;
+
+	switch (pin) {
+	case 0 ... 20:
+		offset = IO_PWR_DOMAIN_GPIO1_K3;
+		break;
+	case 21 ... 41:
+		offset = IO_PWR_DOMAIN_GPIO2_Kx;
+		break;
+	case 76 ... 98:
+		offset = IO_PWR_DOMAIN_GPIO4_K3;
+		break;
+	case 99 ... 127:
+		offset = IO_PWR_DOMAIN_GPIO5_K3;
+		break;
+	case 132 ... 137:
+		offset = IO_PWR_DOMAIN_MMC_Kx;
+		break;
+	case 138 ... 144:
+		offset = IO_PWR_DOMAIN_QSPI_K3;
+		break;
+	}
+
+	return offset;
+}
+
 static inline void __iomem *spacemit_pin_to_reg(struct spacemit_pinctrl *pctrl,
 						unsigned int pin)
 {
@@ -365,6 +441,38 @@ static int spacemit_pctrl_check_power(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static void spacemit_set_io_pwr_domain(struct spacemit_pinctrl *pctrl,
+				      const struct spacemit_pin *spin,
+				      const enum spacemit_pin_io_type type)
+{
+	u32 offset = pctrl->data->pin_to_io_pd_offset(spin->pin);
+	u32 val = 0;
+
+	/* Other bits are reserved so don't need to save them */
+	if (type == IO_TYPE_1V8)
+		val = IO_PWR_DOMAIN_V18EN;
+
+	/*
+	 * IO power domain registers are protected and cannot be accessed
+	 * directly. Before performing any read or write to the IO power
+	 * domain registers, an explicit unlock sequence must be issued
+	 * via the AIB Secure Access Register (ASAR).
+	 *
+	 * The unlock sequence allows exactly one subsequent access to the
+	 * IO power domain registers. After that access completes, the ASAR
+	 * keys are automatically cleared, and the registers become locked
+	 * again.
+	 *
+	 * This mechanism ensures that IO power domain configuration is
+	 * performed intentionally, as incorrect voltage settings may
+	 * result in functional failures or hardware damage.
+	 */
+	regmap_write(pctrl->regmap_apbc, pctrl->regmap_apbc_offset + APBC_ASFAR, APBC_ASFAR_AKEY);
+	regmap_write(pctrl->regmap_apbc, pctrl->regmap_apbc_offset + APBC_ASSAR, APBC_ASSAR_AKEY);
+
+	writel_relaxed(val, pctrl->io_pd_reg + offset);
+}
+
 static int spacemit_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 					 struct device_node *np,
 					 struct pinctrl_map **maps,
@@ -572,7 +680,8 @@ static int spacemit_pinconf_get(struct pinctrl_dev *pctldev,
 
 #define ENABLE_DRV_STRENGTH	BIT(1)
 #define ENABLE_SLEW_RATE	BIT(2)
-static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
+static int spacemit_pinconf_generate_config(struct spacemit_pinctrl *pctrl,
+					    const struct spacemit_pin *spin,
 					    const struct spacemit_pinctrl_dconf *dconf,
 					    unsigned long *configs,
 					    unsigned int num_configs,
@@ -646,6 +755,7 @@ static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
 			default:
 				return -EINVAL;
 			}
+			spacemit_set_io_pwr_domain(pctrl, spin, type);
 		}
 
 		val = spacemit_get_driver_strength(type, dconf, drv_strength);
@@ -701,7 +811,7 @@ static int spacemit_pinconf_set(struct pinctrl_dev *pctldev,
 	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
 	u32 value;
 
-	if (spacemit_pinconf_generate_config(spin, pctrl->data->dconf,
+	if (spacemit_pinconf_generate_config(pctrl, spin, pctrl->data->dconf,
 					     configs, num_configs, &value))
 		return -EINVAL;
 
@@ -724,7 +834,7 @@ static int spacemit_pinconf_group_set(struct pinctrl_dev *pctldev,
 		return -EINVAL;
 
 	spin = spacemit_get_pin(pctrl, group->grp.pins[0]);
-	if (spacemit_pinconf_generate_config(spin, pctrl->data->dconf,
+	if (spacemit_pinconf_generate_config(pctrl, spin, pctrl->data->dconf,
 					     configs, num_configs, &value))
 		return -EINVAL;
 
@@ -795,6 +905,7 @@ static const struct pinconf_ops spacemit_pinconf_ops = {
 
 static int spacemit_pinctrl_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	struct spacemit_pinctrl *pctrl;
 	struct clk *func_clk, *bus_clk;
@@ -816,6 +927,18 @@ static int spacemit_pinctrl_probe(struct platform_device *pdev)
 	if (IS_ERR(pctrl->regs))
 		return PTR_ERR(pctrl->regs);
 
+	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(pctrl->io_pd_reg))
+		return PTR_ERR(pctrl->io_pd_reg);
+
+	pctrl->regmap_apbc =
+		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
+						     &pctrl->regmap_apbc_offset);
+
+	if (IS_ERR(pctrl->regmap_apbc))
+		return dev_err_probe(dev, PTR_ERR(pctrl->regmap_apbc),
+				     "failed to get syscon\n");
+
 	func_clk = devm_clk_get_enabled(dev, "func");
 	if (IS_ERR(func_clk))
 		return dev_err_probe(dev, PTR_ERR(func_clk), "failed to get func clock\n");
@@ -1118,6 +1241,7 @@ static const struct spacemit_pinctrl_data k1_pinctrl_data = {
 	.data = k1_pin_data,
 	.npins = ARRAY_SIZE(k1_pin_desc),
 	.pin_to_offset = spacemit_k1_pin_to_offset,
+	.pin_to_io_pd_offset = spacemit_k1_pin_to_io_pd_offset,
 	.dconf = &k1_drive_conf,
 };
 
@@ -1455,6 +1579,7 @@ static const struct spacemit_pinctrl_data k3_pinctrl_data = {
 	.data = k3_pin_data,
 	.npins = ARRAY_SIZE(k3_pin_desc),
 	.pin_to_offset = spacemit_k3_pin_to_offset,
+	.pin_to_io_pd_offset = spacemit_k3_pin_to_io_pd_offset,
 	.dconf = &k3_drive_conf,
 };
 

-- 
2.52.0

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Yixun Lan]

Hi Troy,

On 17:11 Tue 23 Dec     , Troy Mitchell wrote:
> IO domain power control registers are used to configure the operating
> voltage of dual-voltage GPIO banks. By default, these registers are
> configured for 3.3V operation. As a result, even when a GPIO bank is
> externally supplied with 1.8V, the internal logic continues to
> operate in the 3.3V domain, which may lead to functional failures.
> 
> This patch adds support for programming the IO domain power control
> registers, allowing dual-voltage GPIO banks to be explicitly configured
> for 1.8V operation when required.
> 
> Care must be taken when configuring these registers. If a GPIO bank is
> externally supplied with 3.3V while the corresponding IO power domain
> is configured for 1.8V, external current injection (back-powering)
> may occur, potentially causing damage to the GPIO pin.
> 
> Due to these hardware constraints and safety considerations, the IO
> domain power control registers are implemented as secure registers.
> Access to these registers requires unlocking via the AIB Secure Access
> Register (ASAR) in the APBC block before a single read or write
> operation can be performed.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
>  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
>  2 files changed, 131 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
dtsi should go as separated patch, then route to SoC tree

> @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
>  
>  		pinctrl: pinctrl@d401e000 {
>  			compatible = "spacemit,k1-pinctrl";
> -			reg = <0x0 0xd401e000 0x0 0x400>;
> +			reg = <0x0 0xd401e000 0x0 0x400>,
> +			      <0x0 0xd401e800 0x0 0x34>;
>  			clocks = <&syscon_apbc CLK_AIB>,
>  				 <&syscon_apbc CLK_AIB_BUS>;
>  			clock-names = "func", "bus";
> +			spacemit,apbc = <&syscon_apbc 0x50>;
>  		};
>  
>  		pwm8: pwm@d4020000 {
> diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> index 8ca247fb8ba0321c02423f9739130e03277d1053..b3ffb32f88a79ebf6b64e62a7846df60b92799fe 100644
> --- a/drivers/pinctrl/spacemit/pinctrl-k1.c
> +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> @@ -7,8 +7,10 @@
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/seq_file.h>
>  #include <linux/spinlock.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  
> @@ -47,6 +49,25 @@
>  #define PAD_PULLUP		BIT(14)
>  #define PAD_PULL_EN		BIT(15)
>  
> +#define IO_PWR_DOMAIN_GPIO2_Kx  0x0c
> +#define IO_PWR_DOMAIN_MMC_Kx    0x1c
> +
> +#define IO_PWR_DOMAIN_GPIO3_K1  0x10
> +#define IO_PWR_DOMAIN_QSPI_K1   0x20
> +
> +#define IO_PWR_DOMAIN_GPIO1_K3  0x04
> +#define IO_PWR_DOMAIN_GPIO5_K3  0x10
> +#define IO_PWR_DOMAIN_GPIO4_K3  0x20
> +#define IO_PWR_DOMAIN_QSPI_K3   0x2c
> +
> +#define IO_PWR_DOMAIN_V18EN	BIT(2)
> +
> +#define APBC_ASFAR		0x00
> +#define APBC_ASSAR		0x04
> +
> +#define APBC_ASFAR_AKEY		0xbaba
> +#define APBC_ASSAR_AKEY		0xeb10
> +
>  struct spacemit_pin_drv_strength {
>  	u8		val;
>  	u32		mA;
> @@ -78,6 +99,10 @@ struct spacemit_pinctrl {
>  	raw_spinlock_t				lock;
>  
>  	void __iomem				*regs;
> +	void __iomem				*io_pd_reg;
> +
> +	struct regmap				*regmap_apbc;
> +	u32					regmap_apbc_offset;
>  };
>  
>  struct spacemit_pinctrl_data {
> @@ -85,6 +110,7 @@ struct spacemit_pinctrl_data {
>  	const struct spacemit_pin	*data;
>  	u16				npins;
>  	unsigned int			(*pin_to_offset)(unsigned int pin);
> +	unsigned int			(*pin_to_io_pd_offset)(unsigned int pin);
>  	const struct spacemit_pinctrl_dconf	*dconf;
>  };
>  
> @@ -146,6 +172,56 @@ static unsigned int spacemit_k3_pin_to_offset(unsigned int pin)
>  	return offset << 2;
>  }
>  
> +static unsigned int spacemit_k1_pin_to_io_pd_offset(unsigned int pin)
> +{
> +	unsigned int offset = 0;
> +
> +	switch (pin) {
> +	case 47 ... 52:
> +		offset = IO_PWR_DOMAIN_GPIO3_K1;
> +		break;
> +	case 75 ... 80:
> +		offset = IO_PWR_DOMAIN_GPIO2_Kx;
> +		break;
> +	case 98 ... 103:
> +		offset = IO_PWR_DOMAIN_QSPI_K1;
> +		break;
> +	case 104 ... 109:
> +		offset = IO_PWR_DOMAIN_MMC_Kx;
> +		break;
> +	}
> +
> +	return offset;
> +}
> +
> +static unsigned int spacemit_k3_pin_to_io_pd_offset(unsigned int pin)
> +{
> +	unsigned int offset = 0;
> +
> +	switch (pin) {
> +	case 0 ... 20:
> +		offset = IO_PWR_DOMAIN_GPIO1_K3;
> +		break;
> +	case 21 ... 41:
> +		offset = IO_PWR_DOMAIN_GPIO2_Kx;
> +		break;
> +	case 76 ... 98:
> +		offset = IO_PWR_DOMAIN_GPIO4_K3;
> +		break;
> +	case 99 ... 127:
> +		offset = IO_PWR_DOMAIN_GPIO5_K3;
> +		break;
> +	case 132 ... 137:
> +		offset = IO_PWR_DOMAIN_MMC_Kx;
> +		break;
> +	case 138 ... 144:
> +		offset = IO_PWR_DOMAIN_QSPI_K3;
> +		break;
> +	}
> +
> +	return offset;
> +}
> +
>  static inline void __iomem *spacemit_pin_to_reg(struct spacemit_pinctrl *pctrl,
>  						unsigned int pin)
>  {
> @@ -365,6 +441,38 @@ static int spacemit_pctrl_check_power(struct pinctrl_dev *pctldev,
>  	return 0;
>  }
>  
> +static void spacemit_set_io_pwr_domain(struct spacemit_pinctrl *pctrl,
> +				      const struct spacemit_pin *spin,
> +				      const enum spacemit_pin_io_type type)
> +{
> +	u32 offset = pctrl->data->pin_to_io_pd_offset(spin->pin);
> +	u32 val = 0;
> +
> +	/* Other bits are reserved so don't need to save them */
> +	if (type == IO_TYPE_1V8)
> +		val = IO_PWR_DOMAIN_V18EN;
> +
> +	/*
> +	 * IO power domain registers are protected and cannot be accessed
> +	 * directly. Before performing any read or write to the IO power
> +	 * domain registers, an explicit unlock sequence must be issued
> +	 * via the AIB Secure Access Register (ASAR).
> +	 *
> +	 * The unlock sequence allows exactly one subsequent access to the
> +	 * IO power domain registers. After that access completes, the ASAR
> +	 * keys are automatically cleared, and the registers become locked
> +	 * again.
> +	 *
> +	 * This mechanism ensures that IO power domain configuration is
> +	 * performed intentionally, as incorrect voltage settings may
> +	 * result in functional failures or hardware damage.
> +	 */
> +	regmap_write(pctrl->regmap_apbc, pctrl->regmap_apbc_offset + APBC_ASFAR, APBC_ASFAR_AKEY);
> +	regmap_write(pctrl->regmap_apbc, pctrl->regmap_apbc_offset + APBC_ASSAR, APBC_ASSAR_AKEY);
> +
> +	writel_relaxed(val, pctrl->io_pd_reg + offset);
> +}
> +
>  static int spacemit_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
>  					 struct device_node *np,
>  					 struct pinctrl_map **maps,
> @@ -572,7 +680,8 @@ static int spacemit_pinconf_get(struct pinctrl_dev *pctldev,
>  
>  #define ENABLE_DRV_STRENGTH	BIT(1)
>  #define ENABLE_SLEW_RATE	BIT(2)
> -static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
> +static int spacemit_pinconf_generate_config(struct spacemit_pinctrl *pctrl,
> +					    const struct spacemit_pin *spin,
>  					    const struct spacemit_pinctrl_dconf *dconf,
>  					    unsigned long *configs,
>  					    unsigned int num_configs,
> @@ -646,6 +755,7 @@ static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
>  			default:
>  				return -EINVAL;
>  			}
> +			spacemit_set_io_pwr_domain(pctrl, spin, type);
>  		}
>  
>  		val = spacemit_get_driver_strength(type, dconf, drv_strength);
> @@ -701,7 +811,7 @@ static int spacemit_pinconf_set(struct pinctrl_dev *pctldev,
>  	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
>  	u32 value;
>  
> -	if (spacemit_pinconf_generate_config(spin, pctrl->data->dconf,
> +	if (spacemit_pinconf_generate_config(pctrl, spin, pctrl->data->dconf,
>  					     configs, num_configs, &value))
>  		return -EINVAL;
>  
> @@ -724,7 +834,7 @@ static int spacemit_pinconf_group_set(struct pinctrl_dev *pctldev,
>  		return -EINVAL;
>  
>  	spin = spacemit_get_pin(pctrl, group->grp.pins[0]);
> -	if (spacemit_pinconf_generate_config(spin, pctrl->data->dconf,
> +	if (spacemit_pinconf_generate_config(pctrl, spin, pctrl->data->dconf,
>  					     configs, num_configs, &value))
>  		return -EINVAL;
>  
> @@ -795,6 +905,7 @@ static const struct pinconf_ops spacemit_pinconf_ops = {
>  
>  static int spacemit_pinctrl_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct device *dev = &pdev->dev;
>  	struct spacemit_pinctrl *pctrl;
>  	struct clk *func_clk, *bus_clk;
> @@ -816,6 +927,18 @@ static int spacemit_pinctrl_probe(struct platform_device *pdev)
>  	if (IS_ERR(pctrl->regs))
>  		return PTR_ERR(pctrl->regs);
>  
> +	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(pctrl->io_pd_reg))
> +		return PTR_ERR(pctrl->io_pd_reg);
> +
> +	pctrl->regmap_apbc =
> +		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> +						     &pctrl->regmap_apbc_offset);
Can you simply use syscon_regmap_lookup_by_phandle(), then define 
#define APBC_ASFAR		0x50
#define APBC_ASSAR		0x54

> +
> +	if (IS_ERR(pctrl->regmap_apbc))
> +		return dev_err_probe(dev, PTR_ERR(pctrl->regmap_apbc),
> +				     "failed to get syscon\n");
> +
>  	func_clk = devm_clk_get_enabled(dev, "func");
>  	if (IS_ERR(func_clk))
>  		return dev_err_probe(dev, PTR_ERR(func_clk), "failed to get func clock\n");
> @@ -1118,6 +1241,7 @@ static const struct spacemit_pinctrl_data k1_pinctrl_data = {
>  	.data = k1_pin_data,
>  	.npins = ARRAY_SIZE(k1_pin_desc),
>  	.pin_to_offset = spacemit_k1_pin_to_offset,
> +	.pin_to_io_pd_offset = spacemit_k1_pin_to_io_pd_offset,
>  	.dconf = &k1_drive_conf,
>  };
>  
> @@ -1455,6 +1579,7 @@ static const struct spacemit_pinctrl_data k3_pinctrl_data = {
>  	.data = k3_pin_data,
>  	.npins = ARRAY_SIZE(k3_pin_desc),
>  	.pin_to_offset = spacemit_k3_pin_to_offset,
> +	.pin_to_io_pd_offset = spacemit_k3_pin_to_io_pd_offset,
>  	.dconf = &k3_drive_conf,
>  };
>  
> 
> -- 
> 2.52.0
> 

-- 
Yixun Lan (dlan)

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Inochi Amaoto]

On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> IO domain power control registers are used to configure the operating
> voltage of dual-voltage GPIO banks. By default, these registers are
> configured for 3.3V operation. As a result, even when a GPIO bank is
> externally supplied with 1.8V, the internal logic continues to
> operate in the 3.3V domain, which may lead to functional failures.
> 
> This patch adds support for programming the IO domain power control
> registers, allowing dual-voltage GPIO banks to be explicitly configured
> for 1.8V operation when required.
> 
> Care must be taken when configuring these registers. If a GPIO bank is
> externally supplied with 3.3V while the corresponding IO power domain
> is configured for 1.8V, external current injection (back-powering)
> may occur, potentially causing damage to the GPIO pin.
> 
> Due to these hardware constraints and safety considerations, the IO
> domain power control registers are implemented as secure registers.
> Access to these registers requires unlocking via the AIB Secure Access
> Register (ASAR) in the APBC block before a single read or write
> operation can be performed.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
>  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
>  2 files changed, 131 insertions(+), 4 deletions(-)
> 

> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
>  
>  		pinctrl: pinctrl@d401e000 {
>  			compatible = "spacemit,k1-pinctrl";
> -			reg = <0x0 0xd401e000 0x0 0x400>;
> +			reg = <0x0 0xd401e000 0x0 0x400>,
> +			      <0x0 0xd401e800 0x0 0x34>;
>  			clocks = <&syscon_apbc CLK_AIB>,
>  				 <&syscon_apbc CLK_AIB_BUS>;
>  			clock-names = "func", "bus";
> +			spacemit,apbc = <&syscon_apbc 0x50>;
>  		};

This change breaks binding, can we use something like <0x0 0xd401e000 0x0 0x1000>?
If you insist on a new reg field, you should change the binding as well.

Regards,
Inochi

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

On Tue, Dec 23, 2025 at 05:32:28PM +0800, Yixun Lan wrote:
> Hi Troy,
> 
> On 17:11 Tue 23 Dec     , Troy Mitchell wrote:
> > IO domain power control registers are used to configure the operating
> > voltage of dual-voltage GPIO banks. By default, these registers are
> > configured for 3.3V operation. As a result, even when a GPIO bank is
> > externally supplied with 1.8V, the internal logic continues to
> > operate in the 3.3V domain, which may lead to functional failures.
> > 
> > This patch adds support for programming the IO domain power control
> > registers, allowing dual-voltage GPIO banks to be explicitly configured
> > for 1.8V operation when required.
> > 
> > Care must be taken when configuring these registers. If a GPIO bank is
> > externally supplied with 3.3V while the corresponding IO power domain
> > is configured for 1.8V, external current injection (back-powering)
> > may occur, potentially causing damage to the GPIO pin.
> > 
> > Due to these hardware constraints and safety considerations, the IO
> > domain power control registers are implemented as secure registers.
> > Access to these registers requires unlocking via the AIB Secure Access
> > Register (ASAR) in the APBC block before a single read or write
> > operation can be performed.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > ---
> >  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
> >  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
> >  2 files changed, 131 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> dtsi should go as separated patch, then route to SoC tree
OH I forgot that. Thanks.
> 
> > @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
> >  
> >  		pinctrl: pinctrl@d401e000 {
> >  			compatible = "spacemit,k1-pinctrl";
> > -			reg = <0x0 0xd401e000 0x0 0x400>;
> > +			reg = <0x0 0xd401e000 0x0 0x400>,
> > +			      <0x0 0xd401e800 0x0 0x34>;
> >  			clocks = <&syscon_apbc CLK_AIB>,
> >  				 <&syscon_apbc CLK_AIB_BUS>;
> >  			clock-names = "func", "bus";
> > +			spacemit,apbc = <&syscon_apbc 0x50>;
> >  		};
> >  static int spacemit_pinctrl_probe(struct platform_device *pdev)
> >  {
> > +	struct device_node *np = pdev->dev.of_node;
> >  	struct device *dev = &pdev->dev;
> >  	struct spacemit_pinctrl *pctrl;
> >  	struct clk *func_clk, *bus_clk;
> > @@ -816,6 +927,18 @@ static int spacemit_pinctrl_probe(struct platform_device *pdev)
> >  	if (IS_ERR(pctrl->regs))
> >  		return PTR_ERR(pctrl->regs);
> >  
> > +	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> > +	if (IS_ERR(pctrl->io_pd_reg))
> > +		return PTR_ERR(pctrl->io_pd_reg);
> > +
> > +	pctrl->regmap_apbc =
> > +		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> > +						     &pctrl->regmap_apbc_offset);
> Can you simply use syscon_regmap_lookup_by_phandle(), then define 
> #define APBC_ASFAR		0x50
> #define APBC_ASSAR		0x54
I think it just a minor issue. I will keep it.
But if anyone else thinks the same way as Yixun, please let me know.

                      - Troy

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

On Tue, Dec 23, 2025 at 05:42:26PM +0800, Inochi Amaoto wrote:
> On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> > IO domain power control registers are used to configure the operating
> > voltage of dual-voltage GPIO banks. By default, these registers are
> > configured for 3.3V operation. As a result, even when a GPIO bank is
> > externally supplied with 1.8V, the internal logic continues to
> > operate in the 3.3V domain, which may lead to functional failures.
> > 
> > This patch adds support for programming the IO domain power control
> > registers, allowing dual-voltage GPIO banks to be explicitly configured
> > for 1.8V operation when required.
> > 
> > Care must be taken when configuring these registers. If a GPIO bank is
> > externally supplied with 3.3V while the corresponding IO power domain
> > is configured for 1.8V, external current injection (back-powering)
> > may occur, potentially causing damage to the GPIO pin.
> > 
> > Due to these hardware constraints and safety considerations, the IO
> > domain power control registers are implemented as secure registers.
> > Access to these registers requires unlocking via the AIB Secure Access
> > Register (ASAR) in the APBC block before a single read or write
> > operation can be performed.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > ---
> >  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
> >  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
> >  2 files changed, 131 insertions(+), 4 deletions(-)
> > 
> 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
> >  
> >  		pinctrl: pinctrl@d401e000 {
> >  			compatible = "spacemit,k1-pinctrl";
> > -			reg = <0x0 0xd401e000 0x0 0x400>;
> > +			reg = <0x0 0xd401e000 0x0 0x400>,
> > +			      <0x0 0xd401e800 0x0 0x34>;
> >  			clocks = <&syscon_apbc CLK_AIB>,
> >  				 <&syscon_apbc CLK_AIB_BUS>;
> >  			clock-names = "func", "bus";
> > +			spacemit,apbc = <&syscon_apbc 0x50>;
> >  		};
> 
> If you insist on a new reg field, you should change the binding as well.
Yes, I forgot to modify the binding.

> This change breaks binding, can we use something like <0x0 0xd401e000 0x0 0x1000>?
I'll double check this. Thanks!

                            - Troy
> 
> Regards,
> Inochi
> 

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Yao Zi]

On Tue, Dec 23, 2025 at 05:50:08PM +0800, Troy Mitchell wrote:
> On Tue, Dec 23, 2025 at 05:42:26PM +0800, Inochi Amaoto wrote:
> > On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> > > IO domain power control registers are used to configure the operating
> > > voltage of dual-voltage GPIO banks. By default, these registers are
> > > configured for 3.3V operation. As a result, even when a GPIO bank is
> > > externally supplied with 1.8V, the internal logic continues to
> > > operate in the 3.3V domain, which may lead to functional failures.
> > > 
> > > This patch adds support for programming the IO domain power control
> > > registers, allowing dual-voltage GPIO banks to be explicitly configured
> > > for 1.8V operation when required.
> > > 
> > > Care must be taken when configuring these registers. If a GPIO bank is
> > > externally supplied with 3.3V while the corresponding IO power domain
> > > is configured for 1.8V, external current injection (back-powering)
> > > may occur, potentially causing damage to the GPIO pin.
> > > 
> > > Due to these hardware constraints and safety considerations, the IO
> > > domain power control registers are implemented as secure registers.
> > > Access to these registers requires unlocking via the AIB Secure Access
> > > Register (ASAR) in the APBC block before a single read or write
> > > operation can be performed.
> > > 
> > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > > ---
> > >  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
> > >  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
> > >  2 files changed, 131 insertions(+), 4 deletions(-)
> > > 
> > 
> > > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> > > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
> > >  
> > >  		pinctrl: pinctrl@d401e000 {
> > >  			compatible = "spacemit,k1-pinctrl";
> > > -			reg = <0x0 0xd401e000 0x0 0x400>;
> > > +			reg = <0x0 0xd401e000 0x0 0x400>,
> > > +			      <0x0 0xd401e800 0x0 0x34>;
> > >  			clocks = <&syscon_apbc CLK_AIB>,
> > >  				 <&syscon_apbc CLK_AIB_BUS>;
> > >  			clock-names = "func", "bus";
> > > +			spacemit,apbc = <&syscon_apbc 0x50>;
> > >  		};
> > 
> > If you insist on a new reg field, you should change the binding as well.
> Yes, I forgot to modify the binding.

This will also break ABI compatibility with older devicetrees, I
strongly suggest against a new item in reg property.

Furthermore, it's unreasonable to describe d401_e000 - d401_e400 and
d401_e800 - d401_e834 as separate memory regions. TRM claims the region
starting from 0xd401_e000 with length 0xc00 is "Pad Configuration". So I
think this separation neither simplifies anything nor matches the
hardware.

> > This change breaks binding, can we use something like <0x0 0xd401e000 0x0 0x1000>?

If the TRM is correct, we probably can and should.

> I'll double check this. Thanks!
> 
>                             - Troy
> > 
> > Regards,
> > Inochi

Regards,
Yao Zi

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

On Tue, Dec 23, 2025 at 01:03:27PM +0000, Yao Zi wrote:
> On Tue, Dec 23, 2025 at 05:50:08PM +0800, Troy Mitchell wrote:
> > On Tue, Dec 23, 2025 at 05:42:26PM +0800, Inochi Amaoto wrote:
> > > On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> > > > IO domain power control registers are used to configure the operating
> > > > voltage of dual-voltage GPIO banks. By default, these registers are
> > > > configured for 3.3V operation. As a result, even when a GPIO bank is
> > > > externally supplied with 1.8V, the internal logic continues to
> > > > operate in the 3.3V domain, which may lead to functional failures.
> > > > 
> > > > This patch adds support for programming the IO domain power control
> > > > registers, allowing dual-voltage GPIO banks to be explicitly configured
> > > > for 1.8V operation when required.
> > > > 
> > > > Care must be taken when configuring these registers. If a GPIO bank is
> > > > externally supplied with 3.3V while the corresponding IO power domain
> > > > is configured for 1.8V, external current injection (back-powering)
> > > > may occur, potentially causing damage to the GPIO pin.
> > > > 
> > > > Due to these hardware constraints and safety considerations, the IO
> > > > domain power control registers are implemented as secure registers.
> > > > Access to these registers requires unlocking via the AIB Secure Access
> > > > Register (ASAR) in the APBC block before a single read or write
> > > > operation can be performed.
> > > > 
> > > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > > > ---
> > > >  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-
> > > >  drivers/pinctrl/spacemit/pinctrl-k1.c | 131 +++++++++++++++++++++++++++++++++-
> > > >  2 files changed, 131 insertions(+), 4 deletions(-)
> > > > 
> > > 
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > index 7818ca4979b6a7755722919a5958512aa11950ab..23ecb19624f227f3c39de35bf3078379f7a2490e 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > @@ -565,10 +565,12 @@ i2c8: i2c@d401d800 {
> > > >  
> > > >  		pinctrl: pinctrl@d401e000 {
> > > >  			compatible = "spacemit,k1-pinctrl";
> > > > -			reg = <0x0 0xd401e000 0x0 0x400>;
> > > > +			reg = <0x0 0xd401e000 0x0 0x400>,
> > > > +			      <0x0 0xd401e800 0x0 0x34>;
> > > >  			clocks = <&syscon_apbc CLK_AIB>,
> > > >  				 <&syscon_apbc CLK_AIB_BUS>;
> > > >  			clock-names = "func", "bus";
> > > > +			spacemit,apbc = <&syscon_apbc 0x50>;
> > > >  		};
> > > 
> > > If you insist on a new reg field, you should change the binding as well.
> > Yes, I forgot to modify the binding.
> 
> This will also break ABI compatibility with older devicetrees, I
> strongly suggest against a new item in reg property.
> 
> Furthermore, it's unreasonable to describe d401_e000 - d401_e400 and
> d401_e800 - d401_e834 as separate memory regions. TRM claims the region
> starting from 0xd401_e000 with length 0xc00 is "Pad Configuration". So I
> think this separation neither simplifies anything nor matches the
> hardware.
> 
> > > This change breaks binding, can we use something like <0x0 0xd401e000 0x0 0x1000>?
> 
> If the TRM is correct, we probably can and should.
I have double checked. Inochi is correct.
I'll fix it in the next version.
Thanks!

                              - Troy

Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Krzysztof Kozlowski]

On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> In order to access the protected IO power domain registers, a valid
> unlock sequence must be performed by writing the required keys to the
> AIB Secure Access Register (ASAR).
> 
> The ASAR register resides within the APBC register address space.
> A corresponding syscon property is added to allow the pinctrl driver
> to access this register.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  .../devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> index c5b0218ad6251f97b1f27089ffff724a7b0f69ae..4dc49c2cc1d52008ad89896ae0419091802cd2c4 100644
> --- a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> @@ -32,6 +32,15 @@ properties:
>    resets:
>      maxItems: 1
>  
> +  spacemit,apbc:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that access the protected register
> +          - description: offset of access secure registers
> +    description:
> +      A phandle to syscon with byte offset to access the protected register

Say here for what purpose.

> +
>  patternProperties:
>    '-cfg$':
>      type: object
> @@ -111,6 +120,7 @@ required:
>    - reg
>    - clocks
>    - clock-names
> +  - spacemit,apbc

That's ABI break without justification.

>  
>  additionalProperties: false
>  
> @@ -128,6 +138,7 @@ examples:
>              clocks = <&syscon_apbc 42>,
>                       <&syscon_apbc 94>;
>              clock-names = "func", "bus";
> +            spacemit,apbc = <&syscon_apbc 0x50>;
>  
>              uart0_2_cfg: uart0-2-cfg {
>                  uart0-2-pins {
> 
> -- 
> 2.52.0
> 

Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Krzysztof Kozlowski]

On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> In order to access the protected IO power domain registers, a valid
> unlock sequence must be performed by writing the required keys to the
> AIB Secure Access Register (ASAR).
> 
> The ASAR register resides within the APBC register address space.
> A corresponding syscon property is added to allow the pinctrl driver
> to access this register.
>

Also:

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

You are not adding syscon to all pinctrls.

Best regards,
Krzysztof

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Krzysztof Kozlowski]

On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> IO domain power control registers are used to configure the operating
> voltage of dual-voltage GPIO banks. By default, these registers are
> configured for 3.3V operation. As a result, even when a GPIO bank is
> externally supplied with 1.8V, the internal logic continues to
> operate in the 3.3V domain, which may lead to functional failures.
> 
> This patch adds support for programming the IO domain power control

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94

> registers, allowing dual-voltage GPIO banks to be explicitly configured
> for 1.8V operation when required.
> 
> Care must be taken when configuring these registers. If a GPIO bank is
> externally supplied with 3.3V while the corresponding IO power domain
> is configured for 1.8V, external current injection (back-powering)
> may occur, potentially causing damage to the GPIO pin.
> 
> Due to these hardware constraints and safety considerations, the IO
> domain power control registers are implemented as secure registers.
> Access to these registers requires unlocking via the AIB Secure Access
> Register (ASAR) in the APBC block before a single read or write
> operation can be performed.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  arch/riscv/boot/dts/spacemit/k1.dtsi  |   4 +-

No, this never, never comes together with driver code. You cannot fix
non-bisectability and ABI break that way.

Read carefully maintainers soc profile and submitting patches in DT dir.

...


>  static int spacemit_pinctrl_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct device *dev = &pdev->dev;
>  	struct spacemit_pinctrl *pctrl;
>  	struct clk *func_clk, *bus_clk;
> @@ -816,6 +927,18 @@ static int spacemit_pinctrl_probe(struct platform_device *pdev)
>  	if (IS_ERR(pctrl->regs))
>  		return PTR_ERR(pctrl->regs);
>  
> +	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(pctrl->io_pd_reg))
> +		return PTR_ERR(pctrl->io_pd_reg);
> +
> +	pctrl->regmap_apbc =
> +		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> +						     &pctrl->regmap_apbc_offset);
> +
> +	if (IS_ERR(pctrl->regmap_apbc))
> +		return dev_err_probe(dev, PTR_ERR(pctrl->regmap_apbc),
> +				     "failed to get syscon\n");

Actual ABI break.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Linus Walleij]

Hi Troy,

thanks for your patch!

On Tue, Dec 23, 2025 at 10:11 AM Troy Mitchell
<troy.mitchell@linux.spacemit.com> wrote:

> +  spacemit,apbc:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that access the protected register
> +          - description: offset of access secure registers
(...)
> +            spacemit,apbc = <&syscon_apbc 0x50>;

Isn't the offset implicit from the compatible of the pin controller or
the syscon or any other compatible?

It's easy to check compatibles in the device tree and just say
this offset is 0x50 if compatible is so-or-so, and something else
for another compatible and just give an error if an unknown
compatible appears.

So: please try to avoid to put things the code can easily look
up into the device tree.

Yours,
Linus Walleij

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Linus Walleij]

Hi Troy,

thanks for your patch!

On Tue, Dec 23, 2025 at 10:43 AM Troy Mitchell
<troy.mitchell@linux.spacemit.com> wrote:

> > > +   pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> > > +   if (IS_ERR(pctrl->io_pd_reg))
> > > +           return PTR_ERR(pctrl->io_pd_reg);
> > > +
> > > +   pctrl->regmap_apbc =
> > > +           syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> > > +                                                &pctrl->regmap_apbc_offset);
> > Can you simply use syscon_regmap_lookup_by_phandle(), then define
> > #define APBC_ASFAR            0x50
> > #define APBC_ASSAR            0x54

> I think it just a minor issue. I will keep it.
> But if anyone else thinks the same way as Yixun, please let me know.

It's not minor because it adds stuff to the DT binding we don't need
and this avoids it.

Please go with Yixun's suggestion.

Yours,
Linus Walleij

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Yixun Lan]

Hi Troy, Krzysztof, 

On 14:00 Sat 27 Dec     , Krzysztof Kozlowski wrote:
> On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> > IO domain power control registers are used to configure the operating
> > voltage of dual-voltage GPIO banks. By default, these registers are
> > configured for 3.3V operation. As a result, even when a GPIO bank is
> > externally supplied with 1.8V, the internal logic continues to
> > operate in the 3.3V domain, which may lead to functional failures.
> > 
..
> > +	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> > +	if (IS_ERR(pctrl->io_pd_reg))
> > +		return PTR_ERR(pctrl->io_pd_reg);
> > +
> > +	pctrl->regmap_apbc =
> > +		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> > +						     &pctrl->regmap_apbc_offset);
> > +
> > +	if (IS_ERR(pctrl->regmap_apbc))
> > +		return dev_err_probe(dev, PTR_ERR(pctrl->regmap_apbc),
> > +				     "failed to get syscon\n");
> 
> Actual ABI break.
> 
Indeed, there will be a ABI break.

so, how about not abort in probe() if no "spacemit,apbc" phandle found?
and then do it in a compatible way as old behevior

We may still need to drop this property from "required" section in DT

> Best regards,
> Krzysztof
> 
> 

-- 
Yixun Lan (dlan)

Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Troy Mitchell]

On Thu, Jan 01, 2026 at 11:54:02PM +0100, Linus Walleij wrote:
> Hi Troy,
> 
> thanks for your patch!
> 
> On Tue, Dec 23, 2025 at 10:11 AM Troy Mitchell
> <troy.mitchell@linux.spacemit.com> wrote:
> 
> > +  spacemit,apbc:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      - items:
> > +          - description: phandle to syscon that access the protected register
> > +          - description: offset of access secure registers
> (...)
> > +            spacemit,apbc = <&syscon_apbc 0x50>;
> 
> Isn't the offset implicit from the compatible of the pin controller or
> the syscon or any other compatible?
> 
> It's easy to check compatibles in the device tree and just say
> this offset is 0x50 if compatible is so-or-so, and something else
> for another compatible and just give an error if an unknown
> compatible appears.
> 
> So: please try to avoid to put things the code can easily look
> up into the device tree.
Thanks for you pointing it out. I'll remove it.

                          - Troy
> 
> Yours,
> Linus Walleij
> 

Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property

[Troy Mitchell]

On Sat, Dec 27, 2025 at 01:58:52PM +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> > In order to access the protected IO power domain registers, a valid
> > unlock sequence must be performed by writing the required keys to the
> > AIB Secure Access Register (ASAR).
> > 
> > The ASAR register resides within the APBC register address space.
> > A corresponding syscon property is added to allow the pinctrl driver
> > to access this register.
> >
> 
> Also:
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> You are not adding syscon to all pinctrls.
I lost "spacemit" prefix. I'll add it in the next version.
Thanks!

                        - Troy
> 
> Best regards,
> Krzysztof
> 
> 

Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration

[Troy Mitchell]

On Thu, Jan 08, 2026 at 12:27:53PM +0800, Yixun Lan wrote:
> Hi Troy, Krzysztof, 
> 
> On 14:00 Sat 27 Dec     , Krzysztof Kozlowski wrote:
> > On Tue, Dec 23, 2025 at 05:11:12PM +0800, Troy Mitchell wrote:
> > > IO domain power control registers are used to configure the operating
> > > voltage of dual-voltage GPIO banks. By default, these registers are
> > > configured for 3.3V operation. As a result, even when a GPIO bank is
> > > externally supplied with 1.8V, the internal logic continues to
> > > operate in the 3.3V domain, which may lead to functional failures.
> > > 
> ..
> > > +	pctrl->io_pd_reg = devm_platform_ioremap_resource(pdev, 1);
> > > +	if (IS_ERR(pctrl->io_pd_reg))
> > > +		return PTR_ERR(pctrl->io_pd_reg);
> > > +
> > > +	pctrl->regmap_apbc =
> > > +		syscon_regmap_lookup_by_phandle_args(np, "spacemit,apbc", 1,
> > > +						     &pctrl->regmap_apbc_offset);
> > > +
> > > +	if (IS_ERR(pctrl->regmap_apbc))
> > > +		return dev_err_probe(dev, PTR_ERR(pctrl->regmap_apbc),
> > > +				     "failed to get syscon\n");
> > 
> > Actual ABI break.
> > 
> Indeed, there will be a ABI break.
> 
> so, how about not abort in probe() if no "spacemit,apbc" phandle found?
> and then do it in a compatible way as old behevior
will in the next version.
> 
> We may still need to drop this property from "required" section in DT
Yes. Thanks!

                          - Troy
> 
> > Best regards,
> > Krzysztof
> > 
> > 
> 
> -- 
> Yixun Lan (dlan)
>