2024-03-20 02:06:03

by Alexey Klimov

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: power: reset: add gs101 poweroff bindings

Signed-off-by: Alexey Klimov <[email protected]>
---
.../power/reset/google,gs101-poweroff.yaml | 54 +++++++++++++++++++
1 file changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml b/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml
new file mode 100644
index 000000000000..d704bf28294a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/google,gs101-poweroff.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google GS101 poweroff driver
+
+maintainers:
+ - Alexey Klimov <[email protected]>
+
+description: |+
+ This is a Google Tensor gs101 poweroff driver using custom regmap
+ to map the poweroff register. The poweroff itself is performed with
+ a write to the poweroff register from a privileged mode. Since generic
+ syscon does not support this, the specific one is required.
+ The write to the poweroff register is defined by the register map pointed
+ by syscon reference plus the offset with the value and mask defined
+ in the poweroff node.
+ Default will be little endian mode, 32 bit access only.
+
+properties:
+ compatible:
+ const: google,gs101-poweroff
+
+ mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Update only the register bits defined by the mask (32 bit).
+
+ offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Offset in the register map for the poweroff register (in bytes).
+
+ samsung,syscon-phandle:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the PMU system controller node.
+
+required:
+ - compatible
+ - offset
+ - mask
+ - samsung,syscon-phandle
+
+additionalProperties: false
+
+examples:
+ - |
+ poweroff {
+ compatible = "google,gs101-poweroff";
+ samsung,syscon-phandle = <&pmu_syscon>;
+ offset = <0x10>;
+ mask = <0x42>;
+ };
--
2.43.0



2024-03-20 02:06:16

by Alexey Klimov

[permalink] [raw]
Subject: [PATCH 2/3] arm64: dts: exynos: gs101: add poweroff node

Signed-off-by: Alexey Klimov <[email protected]>
---
arch/arm64/boot/dts/exynos/google/gs101.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index 55e6bcb3689e..9def28393274 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -509,6 +509,13 @@ sysreg_apm: syscon@174204e0 {
pmu_system_controller: system-controller@17460000 {
compatible = "google,gs101-pmu", "syscon";
reg = <0x17460000 0x10000>;
+
+ poweroff {
+ compatible = "google,gs101-poweroff";
+ samsung,syscon-phandle = <&pmu_system_controller>;
+ offset = <0x3e9c>;
+ mask = <0x100>;
+ };
};

pinctrl_gpio_alive: pinctrl@174d0000 {
--
2.43.0


2024-03-20 02:06:34

by Alexey Klimov

[permalink] [raw]
Subject: [PATCH 3/3] power: reset: add new gs101-poweroff driver

The driver allows switching off the Google gs101 SoC (Pixel6 family of
mobile phones). The syscon-poweroff cannot be used since gs101 requires
smc-based regmap i.e. a write to PMU register done from EL3 is required.
Additionally the power off write should be performed when power button
is not pressed.

When USB charging cable is connected then this leads to a reboot of
a device initiated by bootloader/firmware.

Signed-off-by: Alexey Klimov <[email protected]>
---
drivers/power/reset/Kconfig | 7 ++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/gs101-poweroff.c | 157 +++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
create mode 100644 drivers/power/reset/gs101-poweroff.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index fece990af4a7..e7323b3b4a61 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -100,6 +100,13 @@ config POWER_RESET_GPIO_RESTART
If your board needs a GPIO high/low to restart, say Y and
create a binding in your devicetree.

+config POWER_RESET_GS101_POWEROFF
+ tristate "GS101 power-off driver"
+ depends on ARCH_EXYNOS || COMPILE_TEST
+ help
+ This driver supports turning off the Google Tensor Pixel6 GS101 phones.
+ Select this if you're building a kernel with Google Tensor SoC support.
+
config POWER_RESET_HISI
bool "Hisilicon power-off driver"
depends on ARCH_HISI
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index a95d1bd275d1..7065b7e4ce77 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_POWER_RESET_BRCMSTB) += brcmstb-reboot.o
obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
+obj-$(CONFIG_POWER_RESET_GS101_POWEROFF) += gs101-poweroff.o
obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-poweroff.o
obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
diff --git a/drivers/power/reset/gs101-poweroff.c b/drivers/power/reset/gs101-poweroff.c
new file mode 100644
index 000000000000..2be903de16a1
--- /dev/null
+++ b/drivers/power/reset/gs101-poweroff.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GS101 Poweroff Driver
+ *
+ * Copyright (c) 2024, Linaro Ltd.
+ * Author: Alexey Klimov <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/devm-helpers.h>
+#include <linux/input.h>
+#include <linux/io.h>
+#include <linux/keyboard.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/soc/samsung/exynos-pmu.h>
+
+#define shwork_to_poweroff(x) \
+ container_of(x, struct gs101_poweroff, shutdown_work)
+
+#define keyboard_nb_to_poweroff(x) \
+ container_of(x, struct gs101_poweroff, keyboard_nb)
+
+struct gs101_poweroff {
+ struct notifier_block keyboard_nb;
+ bool power_key_pressed;
+ struct work_struct shutdown_work;
+ struct regmap *map;
+ u32 offset;
+ u32 mask;
+};
+
+static struct gs101_poweroff *gs101_poweroff_ctx;
+
+static void gs101_shutdown_work_fn(struct work_struct *work)
+{
+ struct gs101_poweroff *gs101 = shwork_to_poweroff(work);
+
+ while (1) {
+ /* wait for power button release */
+ if (!gs101->power_key_pressed) {
+ /* Issue the poweroff */
+ regmap_update_bits(gs101->map,
+ gs101->offset,
+ gs101->mask, 0);
+ } else {
+ /*
+ * if power button is not released,
+ * wait and check TA again
+ */
+ pr_info("power key is not released.\n");
+ }
+ mdelay(1000);
+ }
+}
+
+static int gs101_keyboard_notifier_call(struct notifier_block *nb,
+ unsigned long code, void *_param)
+{
+ struct keyboard_notifier_param *param = _param;
+
+ if (param->value == KEY_POWER) {
+ struct gs101_poweroff *gs101 = keyboard_nb_to_poweroff(nb);
+
+ gs101->power_key_pressed = param->down;
+ }
+
+ return NOTIFY_OK;
+}
+
+static void gs101_poweroff(void)
+{
+ schedule_work(&gs101_poweroff_ctx->shutdown_work);
+}
+
+static int gs101_poweroff_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct gs101_poweroff *gs101;
+ int ret;
+
+ gs101 = devm_kzalloc(dev, sizeof(*gs101), GFP_KERNEL);
+ if (!gs101)
+ return -ENOMEM;
+
+ gs101->map = exynos_get_pmu_regmap_by_phandle(dev->of_node,
+ "samsung,syscon-phandle");
+ if (IS_ERR(gs101->map))
+ return PTR_ERR(gs101->map);
+
+ if (of_property_read_u32(dev->of_node, "offset", &gs101->offset)) {
+ dev_err(dev, "unable to read 'offset' from DT\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_u32(dev->of_node, "mask", &gs101->mask)) {
+ dev_err(dev, "unable to read 'mask' from DT\n");
+ return -EINVAL;
+ }
+
+ gs101->keyboard_nb.notifier_call = gs101_keyboard_notifier_call;
+ ret = register_keyboard_notifier(&gs101->keyboard_nb);
+ if (ret) {
+ dev_err(dev, "failed to register keyboard notifier: %i\n", ret);
+ return ret;
+ }
+
+ ret = devm_work_autocancel(dev, &gs101->shutdown_work,
+ gs101_shutdown_work_fn);
+ if (ret) {
+ dev_err(dev, "failed to register gs101 shutdown_work: %i\n", ret);
+ unregister_keyboard_notifier(&gs101->keyboard_nb);
+ return ret;
+ }
+
+ gs101_poweroff_ctx = gs101;
+ platform_set_drvdata(pdev, gs101);
+
+ /*
+ * At this point there is a chance that psci_sys_poweroff already
+ * registered as pm_power_off hook but unfortunately it cannot power
+ * off the gs101 SoC hence we are rewriting it here just as is.
+ */
+ pm_power_off = gs101_poweroff;
+
+ return 0;
+}
+
+static void gs101_poweroff_remove(struct platform_device *pdev)
+{
+ struct gs101_poweroff *gs101 = platform_get_drvdata(pdev);
+
+ if (pm_power_off == gs101_poweroff)
+ pm_power_off = NULL;
+
+ unregister_keyboard_notifier(&gs101->keyboard_nb);
+}
+
+static const struct of_device_id gs101_poweroff_of_match[] = {
+ { .compatible = "google,gs101-poweroff" },
+ {}
+};
+
+static struct platform_driver gs101_poweroff_driver = {
+ .probe = gs101_poweroff_probe,
+ .remove_new = gs101_poweroff_remove,
+ .driver = {
+ .name = "gs101-poweroff",
+ .of_match_table = gs101_poweroff_of_match,
+ },
+};
+
+module_platform_driver(gs101_poweroff_driver);
+MODULE_AUTHOR("Alexey Klimov <[email protected]>");
+MODULE_DESCRIPTION("Google GS101 poweroff driver");
+MODULE_LICENSE("GPL v2");
--
2.43.0


2024-03-20 07:14:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: power: reset: add gs101 poweroff bindings

On 20/03/2024 03:05, Alexey Klimov wrote:
> Signed-off-by: Alexey Klimov <[email protected]>

Missing commit msg.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
> .../power/reset/google,gs101-poweroff.yaml | 54 +++++++++++++++++++
> 1 file changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml
>
> diff --git a/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml b/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml
> new file mode 100644
> index 000000000000..d704bf28294a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/google,gs101-poweroff.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/google,gs101-poweroff.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google GS101 poweroff driver

Bindings are not for drivers, but hardware.

> +
> +maintainers:
> + - Alexey Klimov <[email protected]>
> +
> +description: |+
> + This is a Google Tensor gs101 poweroff driver using custom regmap

Bindings are not for drivers, but hardware.

> + to map the poweroff register. The poweroff itself is performed with
> + a write to the poweroff register from a privileged mode. Since generic
> + syscon does not support this, the specific one is required.
> + The write to the poweroff register is defined by the register map pointed
> + by syscon reference plus the offset with the value and mask defined
> + in the poweroff node.
> + Default will be little endian mode, 32 bit access only.
> +
> +properties:
> + compatible:
> + const: google,gs101-poweroff
> +
> + mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Update only the register bits defined by the mask (32 bit).
> +
> + offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Offset in the register map for the poweroff register (in bytes).
> +
> + samsung,syscon-phandle:
> + $ref: /schemas/types.yaml#/definitions/phandle

This does not look right. The poweroff handling is within PMU, not
somewhere else. Don't use syscons as a replacement of regular MMIO
addresses.



Best regards,
Krzysztof


2024-03-20 07:15:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm64: dts: exynos: gs101: add poweroff node

On 20/03/2024 03:05, Alexey Klimov wrote:
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> arch/arm64/boot/dts/exynos/google/gs101.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index 55e6bcb3689e..9def28393274 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -509,6 +509,13 @@ sysreg_apm: syscon@174204e0 {
> pmu_system_controller: system-controller@17460000 {
> compatible = "google,gs101-pmu", "syscon";
> reg = <0x17460000 0x10000>;
> +
> + poweroff {
> + compatible = "google,gs101-poweroff";
> + samsung,syscon-phandle = <&pmu_system_controller>;

This is just senseless... you are the child of pmu, as seen in this DTS.
You do not need to reference yourself (so the PMU)!



Best regards,
Krzysztof


2024-03-20 07:21:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/3] power: reset: add new gs101-poweroff driver

On 20/03/2024 03:05, Alexey Klimov wrote:
> +
> + ret = devm_work_autocancel(dev, &gs101->shutdown_work,
> + gs101_shutdown_work_fn);
> + if (ret) {
> + dev_err(dev, "failed to register gs101 shutdown_work: %i\n", ret);
> + unregister_keyboard_notifier(&gs101->keyboard_nb);
> + return ret;
> + }
> +
> + gs101_poweroff_ctx = gs101;
> + platform_set_drvdata(pdev, gs101);
> +
> + /*
> + * At this point there is a chance that psci_sys_poweroff already
> + * registered as pm_power_off hook but unfortunately it cannot power
> + * off the gs101 SoC hence we are rewriting it here just as is.
> + */
> + pm_power_off = gs101_poweroff;

So that's a duplicated syscon power off driver. Why syscon does not
work? syscon_node_to_regmap() does not return correct regmap? If so,
this should be fixed instead of copying the driver with basically only
one difference.

Best regards,
Krzysztof


2024-03-22 12:25:33

by Peter Griffin

[permalink] [raw]
Subject: Re: [PATCH 3/3] power: reset: add new gs101-poweroff driver

Hi Krzysztof,

Thanks for your review feedback!

On Wed, 20 Mar 2024 at 07:20, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 20/03/2024 03:05, Alexey Klimov wrote:
> > +
> > + ret = devm_work_autocancel(dev, &gs101->shutdown_work,
> > + gs101_shutdown_work_fn);
> > + if (ret) {
> > + dev_err(dev, "failed to register gs101 shutdown_work: %i\n", ret);
> > + unregister_keyboard_notifier(&gs101->keyboard_nb);
> > + return ret;
> > + }
> > +
> > + gs101_poweroff_ctx = gs101;
> > + platform_set_drvdata(pdev, gs101);
> > +
> > + /*
> > + * At this point there is a chance that psci_sys_poweroff already
> > + * registered as pm_power_off hook but unfortunately it cannot power
> > + * off the gs101 SoC hence we are rewriting it here just as is.
> > + */
> > + pm_power_off = gs101_poweroff;
>
> So that's a duplicated syscon power off driver. Why syscon does not
> work? syscon_node_to_regmap() does not return correct regmap?

Yes, for gs101 the regmap handling PMU registers is now created by
exynos-pmu driver and is obtained using
exynos_get_pmu_regmap_by_phandle() API. That was required due to the
SMC call required to write to these registers from Linux.

> If so,
> this should be fixed instead of copying the driver with basically only
> one difference.

Are you suggesting we should add some API to syscon.c that allows
regmaps created in other drivers like exynos-pmu.c or altera-sysmgr.c
to be registered in the syscon_list?

Thanks,

Peter

2024-03-25 19:23:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/3] power: reset: add new gs101-poweroff driver

On 22/03/2024 13:25, Peter Griffin wrote:
> Hi Krzysztof,
>
> Thanks for your review feedback!
>
> On Wed, 20 Mar 2024 at 07:20, Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 20/03/2024 03:05, Alexey Klimov wrote:
>>> +
>>> + ret = devm_work_autocancel(dev, &gs101->shutdown_work,
>>> + gs101_shutdown_work_fn);
>>> + if (ret) {
>>> + dev_err(dev, "failed to register gs101 shutdown_work: %i\n", ret);
>>> + unregister_keyboard_notifier(&gs101->keyboard_nb);
>>> + return ret;
>>> + }
>>> +
>>> + gs101_poweroff_ctx = gs101;
>>> + platform_set_drvdata(pdev, gs101);
>>> +
>>> + /*
>>> + * At this point there is a chance that psci_sys_poweroff already
>>> + * registered as pm_power_off hook but unfortunately it cannot power
>>> + * off the gs101 SoC hence we are rewriting it here just as is.
>>> + */
>>> + pm_power_off = gs101_poweroff;
>>
>> So that's a duplicated syscon power off driver. Why syscon does not
>> work? syscon_node_to_regmap() does not return correct regmap?
>
> Yes, for gs101 the regmap handling PMU registers is now created by
> exynos-pmu driver and is obtained using
> exynos_get_pmu_regmap_by_phandle() API. That was required due to the
> SMC call required to write to these registers from Linux.
>
>> If so,
>> this should be fixed instead of copying the driver with basically only
>> one difference.
>
> Are you suggesting we should add some API to syscon.c that allows
> regmaps created in other drivers like exynos-pmu.c or altera-sysmgr.c
> to be registered in the syscon_list?

Yes, I think this could work.

Best regards,
Krzysztof