All of lore.kernel.org
 help / color / mirror / Atom feed
From: Troy Mitchell <troy.mitchell@linux.spacemit.com>
To: Yao Zi <me@ziyao.cc>,
	Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	Inochi Amaoto <inochiama@gmail.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Yixun Lan <dlan@gentoo.org>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Linus Walleij <linusw@kernel.org>
Cc: devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
	spacemit@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration
Date: Thu, 25 Dec 2025 15:07:33 +0800	[thread overview]
Message-ID: <47781E30EC71C60F+aUzitZG78aGBxHOG@kernel.org> (raw)
In-Reply-To: <aUqTH_fis1-g42SU@pie>

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

WARNING: multiple messages have this Message-ID (diff)
From: Troy Mitchell <troy.mitchell@linux.spacemit.com>
To: Yao Zi <me@ziyao.cc>,
	Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	Inochi Amaoto <inochiama@gmail.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Yixun Lan <dlan@gentoo.org>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Linus Walleij <linusw@kernel.org>
Cc: devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
	spacemit@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration
Date: Thu, 25 Dec 2025 15:07:33 +0800	[thread overview]
Message-ID: <47781E30EC71C60F+aUzitZG78aGBxHOG@kernel.org> (raw)
In-Reply-To: <aUqTH_fis1-g42SU@pie>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2025-12-25  7:08 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-23  9:11 [PATCH 0/2] pinctrl: spacemit: support I/O power domain configuration Troy Mitchell
2025-12-23  9:11 ` Troy Mitchell
2025-12-23  9:11 ` [PATCH 1/2] dt-bindings: pinctrl: add syscon property Troy Mitchell
2025-12-23  9:11   ` Troy Mitchell
2025-12-27 12:58   ` Krzysztof Kozlowski
2025-12-27 12:58     ` Krzysztof Kozlowski
2025-12-27 12:58   ` Krzysztof Kozlowski
2025-12-27 12:58     ` Krzysztof Kozlowski
2026-01-08  6:04     ` Troy Mitchell
2026-01-08  6:04       ` Troy Mitchell
2026-01-01 22:54   ` Linus Walleij
2026-01-01 22:54     ` Linus Walleij
2026-01-08  6:04     ` Troy Mitchell
2026-01-08  6:04       ` Troy Mitchell
2025-12-23  9:11 ` [PATCH 2/2] pinctrl: spacemit: support I/O power domain configuration Troy Mitchell
2025-12-23  9:11   ` Troy Mitchell
2025-12-23  9:32   ` Yixun Lan
2025-12-23  9:32     ` Yixun Lan
2025-12-23  9:42     ` Troy Mitchell
2025-12-23  9:42       ` Troy Mitchell
2026-01-01 22:58       ` Linus Walleij
2026-01-01 22:58         ` Linus Walleij
2025-12-23  9:42   ` Inochi Amaoto
2025-12-23  9:42     ` Inochi Amaoto
2025-12-23  9:50     ` Troy Mitchell
2025-12-23  9:50       ` Troy Mitchell
2025-12-23 13:03       ` Yao Zi
2025-12-23 13:03         ` Yao Zi
2025-12-25  7:07         ` Troy Mitchell [this message]
2025-12-25  7:07           ` Troy Mitchell
2025-12-27 13:00   ` Krzysztof Kozlowski
2025-12-27 13:00     ` Krzysztof Kozlowski
2026-01-08  4:27     ` Yixun Lan
2026-01-08  4:27       ` Yixun Lan
2026-01-08  6:06       ` Troy Mitchell
2026-01-08  6:06         ` Troy Mitchell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47781E30EC71C60F+aUzitZG78aGBxHOG@kernel.org \
    --to=troy.mitchell@linux.spacemit.com \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlan@gentoo.org \
    --cc=inochiama@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=me@ziyao.cc \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=robh@kernel.org \
    --cc=spacemit@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.