[PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[Peter Griffin]

Hi Krzysztof,

This series adds support for syscon-reboot and syscon-poweroff to gs101/Oriole.
It has been tested with reboot and poweroff commands respectively.

Note the syscon-reboot/poweroff has *runtime* dependencies on the exynos-pmu
and clk-gs101 patches in this series, as well as [1] which was just queued by
Lee. As you are the maintainer for exynos-pmu and samsung clock drivers I've
included those patches in this series.

For the moment I have left out syscon-reboot-mode dt node as whilst it uses the
same SYSIP_DAT0 register as exynosautov9, and setting this the reboot mode is
correctly reported in the Pixel bootloader. The bootloader is also checking a
scratch register in max77779 chip that isn't currently enabled upstream, and
thus the bootloader doesn't actually enter fastboot.

Kind regards,

Peter

[1] https://lore.kernel.org/linux-arm-kernel/20240621115544.1655458-1-peter.griffin@linaro.org/

Peter Griffin (3):
  arm64: dts: exynos: gs101: add syscon-poweroff and syscon-reboot nodes
  soc: samsung: exynos-pmu: add support for PMU_ALIVE non atomic
    registers
  clk: samsung: gs101: mark gout_hsi2_ufs_embd_i_clk_unipro as critical

 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 15 +++++++++++++++
 drivers/clk/samsung/clk-gs101.c              |  2 +-
 drivers/soc/samsung/exynos-pmu.c             | 16 ++++++++++++++--
 include/linux/soc/samsung/exynos-regs-pmu.h  |  4 ++++
 4 files changed, 34 insertions(+), 3 deletions(-)

-- 
2.45.2.741.gdbec12cfda-goog

[PATCH 1/3] arm64: dts: exynos: gs101: add syscon-poweroff and syscon-reboot nodes

[Peter Griffin]

Reboot of gs101 SoC can be handled by setting the
bit(SWRESET_SYSTEM[1]) of SYSTEM_CONFIGURATION register(PMU + 0x3a00).

Poweroff of gs101 SoC can be handled by setting bit(DATA[8]) of
PAD_CTRL_PWR_HOLD register (PMU + 0x3e9c).

Tested using "reboot" and "poweroff -p" commands.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
This feature has a runtime dependency on the following series
https://lore.kernel.org/linux-arm-kernel/20240621115544.1655458-1-peter.griffin@linaro.org/
and also the exynos-pmu and clk-gs101 patches in this series to be functional.
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index a66e996666b8..ce24b80985a2 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -1394,6 +1394,21 @@ sysreg_apm: syscon@174204e0 {
 		pmu_system_controller: system-controller@17460000 {
 			compatible = "google,gs101-pmu", "syscon";
 			reg = <0x17460000 0x10000>;
+
+			poweroff: syscon-poweroff {
+				compatible = "syscon-poweroff";
+				regmap = <&pmu_system_controller>;
+				offset = <0x3e9c>; /* PAD_CTRL_PWR_HOLD */
+				mask = <0x100>; /* reset value */
+			};
+
+			reboot: syscon-reboot {
+				compatible = "syscon-reboot";
+				regmap = <&pmu_system_controller>;
+				offset = <0x3a00>; /* SYSTEM_CONFIGURATION */
+				mask = <0x2>; /* SWRESET_SYSTEM */
+				value = <0x2>; /* reset value */
+			};
 		};
 
 		pinctrl_gpio_alive: pinctrl@174d0000 {
-- 
2.45.2.741.gdbec12cfda-goog

Re: [PATCH 1/3] arm64: dts: exynos: gs101: add syscon-poweroff and syscon-reboot nodes

[William McVicker]

On 06/26/2024, Peter Griffin wrote:
> Reboot of gs101 SoC can be handled by setting the
> bit(SWRESET_SYSTEM[1]) of SYSTEM_CONFIGURATION register(PMU + 0x3a00).
> 
> Poweroff of gs101 SoC can be handled by setting bit(DATA[8]) of
> PAD_CTRL_PWR_HOLD register (PMU + 0x3e9c).
> 
> Tested using "reboot" and "poweroff -p" commands.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Tested-by: Will McVicker <willmcvicker@google.com>

Thanks Peter. I verified the device reboots and powers off with these updates.

--Will

[PATCH 2/3] soc: samsung: exynos-pmu: add support for PMU_ALIVE non atomic registers

[Peter Griffin]

Not all registers in PMU_ALIVE block support atomic set/clear operations.
GS101_SYSIP_DAT0 and GS101_SYSTEM_CONFIGURATION registers are two regs
where attempting atomic access fails.

As documentation on exactly which registers support atomic operations is
not forthcoming. We default to atomic access, unless the register is
explicitly added to the tensor_is_atomic() function. Update the comment
to reflect this as well.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c            | 16 ++++++++++++++--
 include/linux/soc/samsung/exynos-regs-pmu.h |  4 ++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 624324f4001c..5556acc7c092 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -129,14 +129,26 @@ static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
 	return ret;
 }
 
+static bool tensor_is_atomic(unsigned int reg)
+{
+	switch (reg) {
+	case GS101_SYSIP_DAT0:
+	case GS101_SYSTEM_CONFIGURATION:
+		return false;
+	default:
+		return true;
+	}
+}
+
 static int tensor_sec_update_bits(void *ctx, unsigned int reg,
 				  unsigned int mask, unsigned int val)
 {
 	/*
 	 * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
-	 * as the target registers can be accessed by multiple masters.
+	 * as the target registers can be accessed by multiple masters. Some
+	 * SFRs don't support this however as reported by tensor_is_atomic()
 	 */
-	if (reg > PMUALIVE_MASK)
+	if (reg > PMUALIVE_MASK || !tensor_is_atomic(reg))
 		return tensor_sec_reg_rmw(ctx, reg, mask, val);
 
 	return tensor_set_bits_atomic(ctx, reg, val, mask);
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index aa840ed043e1..f411c176536d 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -657,4 +657,8 @@
 #define EXYNOS5433_PAD_RETENTION_UFS_OPTION			(0x3268)
 #define EXYNOS5433_PAD_RETENTION_FSYSGENIO_OPTION		(0x32A8)
 
+/* For Tensor GS101 */
+#define GS101_SYSIP_DAT0					(0x810)
+#define GS101_SYSTEM_CONFIGURATION				(0x3A00)
+
 #endif /* __LINUX_SOC_EXYNOS_REGS_PMU_H */
-- 
2.45.2.741.gdbec12cfda-goog

Re: [PATCH 2/3] soc: samsung: exynos-pmu: add support for PMU_ALIVE non atomic registers

[William McVicker]

On 06/26/2024, Peter Griffin wrote:
> Not all registers in PMU_ALIVE block support atomic set/clear operations.
> GS101_SYSIP_DAT0 and GS101_SYSTEM_CONFIGURATION registers are two regs
> where attempting atomic access fails.
> 
> As documentation on exactly which registers support atomic operations is
> not forthcoming. We default to atomic access, unless the register is
> explicitly added to the tensor_is_atomic() function. Update the comment
> to reflect this as well.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Reviewed-by: Will McVicker <willmcvicker@google.com>
Tested-by: Will McVicker <willmcvicker@google.com>

I verified reboot and power off on my Pixel 6 Pro.

> ---
>  drivers/soc/samsung/exynos-pmu.c            | 16 ++++++++++++++--
>  include/linux/soc/samsung/exynos-regs-pmu.h |  4 ++++
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index 624324f4001c..5556acc7c092 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -129,14 +129,26 @@ static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
>  	return ret;
>  }
>  
> +static bool tensor_is_atomic(unsigned int reg)
> +{
> +	switch (reg) {
> +	case GS101_SYSIP_DAT0:
> +	case GS101_SYSTEM_CONFIGURATION:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
>  static int tensor_sec_update_bits(void *ctx, unsigned int reg,
>  				  unsigned int mask, unsigned int val)
>  {
>  	/*
>  	 * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
> -	 * as the target registers can be accessed by multiple masters.
> +	 * as the target registers can be accessed by multiple masters. Some
> +	 * SFRs don't support this however as reported by tensor_is_atomic()
>  	 */
> -	if (reg > PMUALIVE_MASK)
> +	if (reg > PMUALIVE_MASK || !tensor_is_atomic(reg))

nit: Should we just move all the logic, e.g. `reg > PMUALIVE_MASK` into `tensor_is_atomic()`?

[...]

Thanks,
Will

[PATCH 3/3] clk: samsung: gs101: mark gout_hsi2_ufs_embd_i_clk_unipro as critical

[Peter Griffin]

The system hangs on poweroff when this UFS clock is turned off, meaning
the system never powers down. For the moment mark the clock as critical.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index ba9570f7a5fa..85098c61c15e 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -2846,7 +2846,7 @@ static const struct samsung_gate_clock hsi2_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_HSI2_UFS_EMBD_I_CLK_UNIPRO,
 	     "gout_hsi2_ufs_embd_i_clk_unipro", "mout_hsi2_ufs_embd_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UFS_EMBD_IPCLKPORT_I_CLK_UNIPRO,
-	     21, 0, 0),
+	     21, CLK_IS_CRITICAL, 0),
 	GATE(CLK_GOUT_HSI2_UFS_EMBD_I_FMP_CLK,
 	     "gout_hsi2_ufs_embd_i_fmp_clk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UFS_EMBD_IPCLKPORT_I_FMP_CLK,
-- 
2.45.2.741.gdbec12cfda-goog

Re: [PATCH 3/3] clk: samsung: gs101: mark gout_hsi2_ufs_embd_i_clk_unipro as critical

[William McVicker]

On 06/26/2024, Peter Griffin wrote:
> The system hangs on poweroff when this UFS clock is turned off, meaning
> the system never powers down. For the moment mark the clock as critical.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Reviewed-by: Will McVicker <willmcvicker@google.com>
Tested-by: Will McVicker <willmcvicker@google.com>

[...]

Thanks! I verified my Pixel 6 Pro reboots and powers off fine with these
changes.

Regards,
Will

Re: [PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[Krzysztof Kozlowski]

On 26/06/2024 21:42, Peter Griffin wrote:
> Hi Krzysztof,
> 
> This series adds support for syscon-reboot and syscon-poweroff to gs101/Oriole.
> It has been tested with reboot and poweroff commands respectively.
> 
> Note the syscon-reboot/poweroff has *runtime* dependencies on the exynos-pmu

How does the runtime dependency manifests? Something get broken if there
are no dependencies? Or maybe reboot does not work, but probably it did
not work before, either?

Best regards,
Krzysztof

Re: [PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[Peter Griffin]

Hi Krzysztof,

On Thu, 27 Jun 2024 at 11:47, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 26/06/2024 21:42, Peter Griffin wrote:
> > Hi Krzysztof,
> >
> > This series adds support for syscon-reboot and syscon-poweroff to gs101/Oriole.
> > It has been tested with reboot and poweroff commands respectively.
> >
> > Note the syscon-reboot/poweroff has *runtime* dependencies on the exynos-pmu
>
> How does the runtime dependency manifests? Something get broken if there
> are no dependencies? Or maybe reboot does not work, but probably it did
> not work before, either?

Without [1] but with this series applied you will get an Serror
interrupt and hang on poweroff, and reboot commands, as it will use a
mmio syscon to write the protected register.

[   74.680240][    T1] reboot: Restarting system
[   74.680322][    C0] SError Interrupt on CPU0, code
0x00000000be000011 -- SError
[   74.680329][    C0] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
6.10.0-rc3-next-20240613-00009-g6ca503bd3c2b #476
[   74.680336][    C0] Hardware name: Oriole (DT)
[   74.680338][    C0] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT
-SSBS BTYPE=--)
[   74.680345][    C0] pc : _raw_spin_unlock_irqrestore+0x10/0x44
[   74.680363][    C0] lr : regmap_unlock_spinlock+0x14/0x20
[   74.680373][    C0] sp : ffff80008009bb40
[   74.680375][    C0] x29: ffff80008009bb40 x28: ffff00da4587e158
x27: ffffa38d3bc74708
[   74.680386][    C0] x26: ffffa38d3d9c9ca8 x25: 0000000000000000
x24: 0000000000000000
[   74.680394][    C0] x23: 0000000000000000 x22: 0000000000000002
x21: 0000000000000002
[   74.680400][    C0] x20: 0000000000003a00 x19: 0000000000000000
x18: ffffffffffffffff
[   74.680407][    C0] x17: 0000000000000000 x16: ffffa38d3c6cf438
x15: ffff80008009b6a0
[   74.680414][    C0] x14: 0000000000000000 x13: ffff00dbb6980000
x12: 00000000000007d4
[   74.680421][    C0] x11: 000000000000029c x10: ffff00dbb6c40000 x9
: ffff00dbb6980000
[   74.680427][    C0] x8 : ffff80008009bc28 x7 : 0000000000000000 x6
: 0000000000000000
[   74.680434][    C0] x5 : ffffa38d3b83e074 x4 : 0000000000003a00 x3
: 0000000000000000
[   74.680440][    C0] x2 : ffffa38d3b831764 x1 : ffff00da40230000 x0
: 0000000100000001
[   74.680447][    C0] Kernel panic - not syncing: Asynchronous SError Interrupt

and

[   49.448590][    T1] reboot: Power down
[   49.448747][    C0] SError Interrupt on CPU0, code
0x00000000be000011 -- SError
[..]

Without the clk-gs101 critical clock change, poweroff command will
hang, when shutting down UFS.

Without the exynos-pmu support for !atomic registers, the reboot
command won't work when the DT is present as the register write to
SYSTEM_CONFIGURATION register will fail.

[  114.525217][    T1] reboot: Restarting system
[  114.525290][    T1] tensor_sec_reg_write(): SMC failed: -22
[  115.525434][    T1] Unable to restart system
[  116.025576][    T1] Reboot failed -- System halted

For poweroff, you are correct this has not been supported on Pixel 6
so far upstream.

However `echo b > /proc/sysrq-trigger ` has worked in previous kernel
releases (it would reset the system using Watchdog). With the
syscon-reboot DT present, but without [1] this will cause a Serror and
no longer restart the system which would be a regression.

Thanks,

Peter.

[1] https://lore.kernel.org/linux-arm-kernel/20240621115544.1655458-1-peter.griffin@linaro.org/

Re: [PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[Krzysztof Kozlowski]

On 27/06/2024 14:10, Peter Griffin wrote:
> Hi Krzysztof,
> 
> On Thu, 27 Jun 2024 at 11:47, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 26/06/2024 21:42, Peter Griffin wrote:
>>> Hi Krzysztof,
>>>
>>> This series adds support for syscon-reboot and syscon-poweroff to gs101/Oriole.
>>> It has been tested with reboot and poweroff commands respectively.
>>>
>>> Note the syscon-reboot/poweroff has *runtime* dependencies on the exynos-pmu
>>
>> How does the runtime dependency manifests? Something get broken if there
>> are no dependencies? Or maybe reboot does not work, but probably it did
>> not work before, either?
> 
> Without [1] but with this series applied you will get an Serror
> interrupt and hang on poweroff, and reboot commands, as it will use a
> mmio syscon to write the protected register.
> 
> [   74.680240][    T1] reboot: Restarting system
> [   74.680322][    C0] SError Interrupt on CPU0, code
> 0x00000000be000011 -- SError
> [   74.680329][    C0] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
> 6.10.0-rc3-next-20240613-00009-g6ca503bd3c2b #476
> [   74.680336][    C0] Hardware name: Oriole (DT)
> [   74.680338][    C0] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT
> -SSBS BTYPE=--)
> [   74.680345][    C0] pc : _raw_spin_unlock_irqrestore+0x10/0x44
> [   74.680363][    C0] lr : regmap_unlock_spinlock+0x14/0x20
> [   74.680373][    C0] sp : ffff80008009bb40
> [   74.680375][    C0] x29: ffff80008009bb40 x28: ffff00da4587e158
> x27: ffffa38d3bc74708
> [   74.680386][    C0] x26: ffffa38d3d9c9ca8 x25: 0000000000000000
> x24: 0000000000000000
> [   74.680394][    C0] x23: 0000000000000000 x22: 0000000000000002
> x21: 0000000000000002
> [   74.680400][    C0] x20: 0000000000003a00 x19: 0000000000000000
> x18: ffffffffffffffff
> [   74.680407][    C0] x17: 0000000000000000 x16: ffffa38d3c6cf438
> x15: ffff80008009b6a0
> [   74.680414][    C0] x14: 0000000000000000 x13: ffff00dbb6980000
> x12: 00000000000007d4
> [   74.680421][    C0] x11: 000000000000029c x10: ffff00dbb6c40000 x9
> : ffff00dbb6980000
> [   74.680427][    C0] x8 : ffff80008009bc28 x7 : 0000000000000000 x6
> : 0000000000000000
> [   74.680434][    C0] x5 : ffffa38d3b83e074 x4 : 0000000000003a00 x3
> : 0000000000000000
> [   74.680440][    C0] x2 : ffffa38d3b831764 x1 : ffff00da40230000 x0
> : 0000000100000001
> [   74.680447][    C0] Kernel panic - not syncing: Asynchronous SError Interrupt
> 
> and
> 
> [   49.448590][    T1] reboot: Power down
> [   49.448747][    C0] SError Interrupt on CPU0, code
> 0x00000000be000011 -- SError
> [..]
> 
> Without the clk-gs101 critical clock change, poweroff command will
> hang, when shutting down UFS.
> 
> Without the exynos-pmu support for !atomic registers, the reboot
> command won't work when the DT is present as the register write to
> SYSTEM_CONFIGURATION register will fail.
> 
> [  114.525217][    T1] reboot: Restarting system
> [  114.525290][    T1] tensor_sec_reg_write(): SMC failed: -22
> [  115.525434][    T1] Unable to restart system
> [  116.025576][    T1] Reboot failed -- System halted
> 
> For poweroff, you are correct this has not been supported on Pixel 6
> so far upstream.
> 
> However `echo b > /proc/sysrq-trigger ` has worked in previous kernel
> releases (it would reset the system using Watchdog). With the
> syscon-reboot DT present, but without [1] this will cause a Serror and
> no longer restart the system which would be a regression.

Thanks, DTS will wait for the next cycle then.

Best regards,
Krzysztof

[PATCH 0/3] Add syscon-reboot and syscon-poweroff support for gs101/Pixel 6

[Peter Griffin]

Hi Krzysztof,

This series adds support for syscon-reboot and syscon-poweroff to gs101/Oriole.
It has been tested with reboot and poweroff commands respectively.

Note the syscon-reboot/poweroff has *runtime* dependencies on the exynos-pmu
and clk-gs101 patches in this series, as well as [1] which was just queued by
Lee. As you are the maintainer for exynos-pmu and samsung clock drivers I've
included those patches in this series.

For the moment I have left out syscon-reboot-mode dt node as whilst it uses the
same SYSIP_DAT0 register as exynosautov9, and setting this the reboot mode is
correctly reported in the Pixel bootloader. The bootloader is also checking a
scratch register in max77779 chip that isn't currently enabled upstream, and
thus the bootloader doesn't actually enter fastboot.

Kind regards,

Peter

[1] https://lore.kernel.org/linux-arm-kernel/20240621115544.1655458-1-peter.griffin@linaro.org/

Changes in v2:
 - Move PMUALIVE_MASK check into tensor_is_atomic() (William)
 - Collect up tags
 - rebase onto next-20240628

Peter Griffin (3):
  arm64: dts: exynos: gs101: add syscon-poweroff and syscon-reboot nodes
  soc: samsung: exynos-pmu: add support for PMU_ALIVE non atomic
    registers
  clk: samsung: gs101: mark gout_hsi2_ufs_embd_i_clk_unipro as critical

 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 15 +++++++++++++++
 drivers/clk/samsung/clk-gs101.c              |  2 +-
 drivers/soc/samsung/exynos-pmu.c             | 16 ++++++++++++++--
 include/linux/soc/samsung/exynos-regs-pmu.h  |  4 ++++
 4 files changed, 34 insertions(+), 3 deletions(-)

-- 
2.45.2.741.gdbec12cfda-goog