The watchdog controller on the Meson-A/C series SoCs is moved to secure world,
We have to call SMC instruction to trap the ATF for watchdog operation. These
operations are different from previous SoCs, so we introduce a new watchdog
driver to support this kind of SoCs.
Changes since v4 at [3]:
- add watchdog node in dts, and introduce a new optional property "timeout-sec"
- add dt-binding for meson secure watchdog
- instantiate wdt device through dts node instead of platform_device_register_simple()
Changes since v3 at [2]:
- add SM_A1_ prefix for WATCHDOG_OPS
- remove phandle to secure-monitor node
- remove watchdog node from dts, and register wdt device by platform_device_register_simple()
- remove dt-binding for meson secure watchdog
- use the msec as unit of timeout parameter which is passed to fw side
Changes since v2 at [1]:
- remove useless dependency in Kconfig
- return zero when getting left time value fails
Changes since v1 at [0]:
- add a new dependency in Kconfig
- simplify/add the return operation
- remove useless ping operation when setting the timeout
- fix some return values
- fix the license statement
[0]:https://lore.kernel.org/linux-amlogic/[email protected]
[1]:https://lore.kernel.org/linux-amlogic/[email protected]
[2]:https://lore.kernel.org/linux-amlogic/[email protected]
[3]:https://lore.kernel.org/linux-amlogic/[email protected]
Xingyu Chen (4):
firmware: meson_sm: add new SMC ID support for accessing secure
watchdog
dt-bindings: watchdog: add new binding for meson secure watchdog
watchdog: add meson secure watchdog driver
arm64: dts: a1: add secure watchdog controller
.../bindings/watchdog/amlogic,meson-sec-wdt.yaml | 35 ++++
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 +
drivers/firmware/meson/meson_sm.c | 1 +
drivers/watchdog/Kconfig | 16 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/meson_sec_wdt.c | 188 +++++++++++++++++++++
include/linux/firmware/meson/meson_sm.h | 1 +
7 files changed, 248 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
create mode 100644 drivers/watchdog/meson_sec_wdt.c
--
2.7.4
The new SMC ID is used to access secure registers by meson secure
watchdog driver.
Signed-off-by: Xingyu Chen <[email protected]>
---
drivers/firmware/meson/meson_sm.c | 1 +
include/linux/firmware/meson/meson_sm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 1d5b4d7..8cdd405 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -44,6 +44,7 @@ static const struct meson_sm_chip gxbb_chip = {
CMD(SM_EFUSE_WRITE, 0x82000031),
CMD(SM_EFUSE_USER_MAX, 0x82000033),
CMD(SM_GET_CHIP_ID, 0x82000044),
+ CMD(SM_A1_WATCHDOG_OPS, 0x82000086),
{ /* sentinel */ },
},
};
diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
index 6669e2a..9ef1524 100644
--- a/include/linux/firmware/meson/meson_sm.h
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -12,6 +12,7 @@ enum {
SM_EFUSE_WRITE,
SM_EFUSE_USER_MAX,
SM_GET_CHIP_ID,
+ SM_A1_WATCHDOG_OPS,
};
struct meson_sm_firmware;
--
2.7.4
The binding targets the Meson-A/C series compatible SoCs, in which the
watchdog registers are in secure world.
Signed-off-by: Xingyu Chen <[email protected]>
---
.../bindings/watchdog/amlogic,meson-sec-wdt.yaml | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
new file mode 100644
index 00000000..c0ff2ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+# Copyright (c) 2019 Amlogic, Inc
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson Secure Watchdog Timer
+
+maintainers:
+ - Xingyu Chen <[email protected]>
+
+description: |+
+ Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
+
+properties:
+ compatible:
+ enum:
+ - amlogic,meson-sec-wdt
+
+ timeout-sec:
+ minimum: 1
+ maximum: 65
+ description:
+ Watchdog timeout values (in seconds)
+
+required:
+ - compatible
+
+examples:
+ - |
+ watchdog {
+ compatible = "amlogic,meson-sec-wdt";
+ timeout-sec = <60>;
+ };
--
2.7.4
The watchdog controller on the Meson-A/C series SoCs is moved to secure
world, watchdog operation needs to be done in secure EL3 mode via ATF,
Non-secure world can call SMC instruction to trap to AFT for watchdog
operation.
Signed-off-by: Xingyu Chen <[email protected]>
---
drivers/watchdog/Kconfig | 16 ++++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/meson_sec_wdt.c | 188 +++++++++++++++++++++++++++++++++++++++
3 files changed, 205 insertions(+)
create mode 100644 drivers/watchdog/meson_sec_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1679e0d..5f88325 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -826,6 +826,22 @@ config MESON_GXBB_WATCHDOG
To compile this driver as a module, choose M here: the
module will be called meson_gxbb_wdt.
+config MESON_SEC_WATCHDOG
+ tristate "Amlogic Meson Secure watchdog support"
+ depends on MESON_SM
+ select WATCHDOG_CORE
+ help
+ The watchdog controller on the Meson-A/C series SoCs is moved to
+ secure world, watchdog operation needs to be done in secure EL3
+ mode via ATF, non-secure world can call SMC instruction to trap
+ to ATF for the watchdog operation.
+
+ Say Y here if watchdog controller on Meson SoCs is located in
+ secure world.
+
+ To compile this driver as a module, choose M here: the
+ module will be called meson_sec_wdt.
+
config MESON_WATCHDOG
tristate "Amlogic Meson SoCs watchdog support"
depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2ee352b..5e6b73d 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o
obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
obj-$(CONFIG_MESON_GXBB_WATCHDOG) += meson_gxbb_wdt.o
+obj-$(CONFIG_MESON_SEC_WATCHDOG) += meson_sec_wdt.o
obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o
obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
diff --git a/drivers/watchdog/meson_sec_wdt.c b/drivers/watchdog/meson_sec_wdt.c
new file mode 100644
index 00000000..7975522
--- /dev/null
+++ b/drivers/watchdog/meson_sec_wdt.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ * Author: Xingyu Chen <[email protected]>
+ *
+ */
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+#include <linux/firmware/meson/meson_sm.h>
+
+#define MESON_SIP_WDT_DISABLE 0x1
+#define MESON_SIP_WDT_ENABLE 0x2
+#define MESON_SIP_WDT_PING 0x3
+#define MESON_SIP_WDT_INIT 0x4
+#define MESON_SIP_WDT_RESETNOW 0x5
+#define MESON_SIP_WDT_SETTIMEOUT 0x6
+#define MESON_SIP_WDT_GETTIMELEFT 0x7
+
+#define DEFAULT_TIMEOUT 30 /* seconds */
+
+/*
+ * Watchdog timer tick is set to 1ms in secfw side, and tick count is
+ * stored in the bit[16-31] of WATCHDOG_CNT register, so the maximum
+ * timeout value is 0xffff ms.
+ */
+#define MAX_TIMEOUT_MS 0xFFFF
+
+struct meson_sec_wdt {
+ struct watchdog_device wdt_dev;
+ struct meson_sm_firmware *fw;
+};
+
+static int meson_sec_wdt_start(struct watchdog_device *wdt_dev)
+{
+ struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+ return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
+ MESON_SIP_WDT_ENABLE, 0, 0, 0, 0);
+}
+
+static int meson_sec_wdt_stop(struct watchdog_device *wdt_dev)
+{
+ struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+ return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
+ MESON_SIP_WDT_DISABLE, 0, 0, 0, 0);
+}
+
+static int meson_sec_wdt_ping(struct watchdog_device *wdt_dev)
+{
+ struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+ return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
+ MESON_SIP_WDT_PING, 0, 0, 0, 0);
+}
+
+static int meson_sec_wdt_set_timeout(struct watchdog_device *wdt_dev,
+ unsigned int timeout)
+{
+ struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+ wdt_dev->timeout = timeout;
+
+ return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
+ MESON_SIP_WDT_SETTIMEOUT,
+ wdt_dev->timeout * 1000, 0, 0, 0);
+}
+
+static unsigned int meson_sec_wdt_get_timeleft(struct watchdog_device *wdt_dev)
+{
+ int ret;
+ unsigned int timeleft;
+ struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+ ret = meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, &timeleft,
+ MESON_SIP_WDT_GETTIMELEFT, 0, 0, 0, 0);
+
+ if (ret)
+ return 0;
+
+ return timeleft;
+}
+
+static const struct watchdog_ops meson_sec_wdt_ops = {
+ .start = meson_sec_wdt_start,
+ .stop = meson_sec_wdt_stop,
+ .ping = meson_sec_wdt_ping,
+ .set_timeout = meson_sec_wdt_set_timeout,
+ .get_timeleft = meson_sec_wdt_get_timeleft,
+};
+
+static const struct watchdog_info meson_sec_wdt_info = {
+ .identity = "Meson Secure Watchdog Timer",
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+};
+
+static int __maybe_unused meson_sec_wdt_resume(struct device *dev)
+{
+ struct meson_sec_wdt *data = dev_get_drvdata(dev);
+
+ if (watchdog_active(&data->wdt_dev))
+ return meson_sec_wdt_start(&data->wdt_dev);
+
+ return 0;
+}
+
+static int __maybe_unused meson_sec_wdt_suspend(struct device *dev)
+{
+ struct meson_sec_wdt *data = dev_get_drvdata(dev);
+
+ if (watchdog_active(&data->wdt_dev))
+ return meson_sec_wdt_stop(&data->wdt_dev);
+
+ return 0;
+}
+
+static const struct dev_pm_ops meson_sec_wdt_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(meson_sec_wdt_suspend, meson_sec_wdt_resume)
+};
+
+static const struct of_device_id meson_sec_wdt_dt_ids[] = {
+ { .compatible = "amlogic,meson-sec-wdt", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_sec_wdt_dt_ids);
+
+static int meson_sec_wdt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct meson_sec_wdt *data;
+ struct device_node *sm_np;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ sm_np = of_find_compatible_node(NULL, NULL, "amlogic,meson-gxbb-sm");
+ if (!sm_np) {
+ dev_err(&pdev->dev, "no secure-monitor node\n");
+ return -EINVAL;
+ }
+
+ data->fw = meson_sm_get(sm_np);
+ of_node_put(sm_np);
+ if (!data->fw)
+ return -EPROBE_DEFER;
+
+ platform_set_drvdata(pdev, data);
+
+ data->wdt_dev.parent = dev;
+ data->wdt_dev.info = &meson_sec_wdt_info;
+ data->wdt_dev.ops = &meson_sec_wdt_ops;
+ data->wdt_dev.max_hw_heartbeat_ms = MAX_TIMEOUT_MS;
+ data->wdt_dev.min_timeout = 1;
+ data->wdt_dev.timeout = DEFAULT_TIMEOUT;
+ watchdog_set_drvdata(&data->wdt_dev, data);
+ watchdog_init_timeout(&data->wdt_dev, 0, dev);
+
+ ret = meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
+ MESON_SIP_WDT_INIT,
+ data->wdt_dev.timeout * 1000, 0, 0, 0);
+ if (ret)
+ return ret;
+
+ watchdog_stop_on_reboot(&data->wdt_dev);
+
+ return devm_watchdog_register_device(dev, &data->wdt_dev);
+}
+
+static struct platform_driver meson_sec_wdt_driver = {
+ .probe = meson_sec_wdt_probe,
+ .driver = {
+ .name = "meson-sec-wdt",
+ .pm = &meson_sec_wdt_pm_ops,
+ .of_match_table = meson_sec_wdt_dt_ids,
+ },
+};
+
+module_platform_driver(meson_sec_wdt_driver);
+
+MODULE_AUTHOR("Xingyu Chen <[email protected]>");
+MODULE_DESCRIPTION("Amlogic Secure Watchdog Timer Driver");
+MODULE_LICENSE("Dual MIT/GPL");
--
2.7.4
Enable secure watchdog controller for Meson-A1 SoC
Signed-off-by: Xingyu Chen <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 4dec518..1e354b7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -118,6 +118,12 @@
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
+
+ watchdog {
+ compatible = "amlogic,meson-sec-wdt";
+ timeout-sec = <60>;
+ status = "okay";
+ };
};
gic: interrupt-controller@ff901000 {
--
2.7.4
Hi Xingyu and Rob,
On Thu, Dec 12, 2019 at 1:20 PM Xingyu Chen <[email protected]> wrote:
[...]
> +examples:
> + - |
> + watchdog {
> + compatible = "amlogic,meson-sec-wdt";
> + timeout-sec = <60>;
> + };
in v3 of this patch Rob commented that there shouldn't be an OF node
if there are no additional properties
with timeout-sec there's now an additional property so my
understanding is that it's fine to have an OF node
what I don't understand yet is where this node should be placed.
is it supposed to be a child node of the secure monitor node (for
which we already have a binding here:
Documentation/devicetree/bindings/firmware/meson/meson_sm.txt) or
where else would we place it inside the .dts?
Martin
[0] https://patchwork.kernel.org/patch/11211399/
On 12/12/19 4:19 AM, Xingyu Chen wrote:
> The watchdog controller on the Meson-A/C series SoCs is moved to secure
> world, watchdog operation needs to be done in secure EL3 mode via ATF,
> Non-secure world can call SMC instruction to trap to AFT for watchdog
> operation.
>
> Signed-off-by: Xingyu Chen <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/Kconfig | 16 ++++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/meson_sec_wdt.c | 188 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 205 insertions(+)
> create mode 100644 drivers/watchdog/meson_sec_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 1679e0d..5f88325 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -826,6 +826,22 @@ config MESON_GXBB_WATCHDOG
> To compile this driver as a module, choose M here: the
> module will be called meson_gxbb_wdt.
>
> +config MESON_SEC_WATCHDOG
> + tristate "Amlogic Meson Secure watchdog support"
> + depends on MESON_SM
> + select WATCHDOG_CORE
> + help
> + The watchdog controller on the Meson-A/C series SoCs is moved to
> + secure world, watchdog operation needs to be done in secure EL3
> + mode via ATF, non-secure world can call SMC instruction to trap
> + to ATF for the watchdog operation.
> +
> + Say Y here if watchdog controller on Meson SoCs is located in
> + secure world.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called meson_sec_wdt.
> +
> config MESON_WATCHDOG
> tristate "Amlogic Meson SoCs watchdog support"
> depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2ee352b..5e6b73d 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -78,6 +78,7 @@ obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o
> obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
> obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
> obj-$(CONFIG_MESON_GXBB_WATCHDOG) += meson_gxbb_wdt.o
> +obj-$(CONFIG_MESON_SEC_WATCHDOG) += meson_sec_wdt.o
> obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o
> obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
> obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
> diff --git a/drivers/watchdog/meson_sec_wdt.c b/drivers/watchdog/meson_sec_wdt.c
> new file mode 100644
> index 00000000..7975522
> --- /dev/null
> +++ b/drivers/watchdog/meson_sec_wdt.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + * Author: Xingyu Chen <[email protected]>
> + *
> + */
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +#include <linux/firmware/meson/meson_sm.h>
> +
> +#define MESON_SIP_WDT_DISABLE 0x1
> +#define MESON_SIP_WDT_ENABLE 0x2
> +#define MESON_SIP_WDT_PING 0x3
> +#define MESON_SIP_WDT_INIT 0x4
> +#define MESON_SIP_WDT_RESETNOW 0x5
> +#define MESON_SIP_WDT_SETTIMEOUT 0x6
> +#define MESON_SIP_WDT_GETTIMELEFT 0x7
> +
> +#define DEFAULT_TIMEOUT 30 /* seconds */
> +
> +/*
> + * Watchdog timer tick is set to 1ms in secfw side, and tick count is
> + * stored in the bit[16-31] of WATCHDOG_CNT register, so the maximum
> + * timeout value is 0xffff ms.
> + */
> +#define MAX_TIMEOUT_MS 0xFFFF
> +
> +struct meson_sec_wdt {
> + struct watchdog_device wdt_dev;
> + struct meson_sm_firmware *fw;
> +};
> +
> +static int meson_sec_wdt_start(struct watchdog_device *wdt_dev)
> +{
> + struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> + return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
> + MESON_SIP_WDT_ENABLE, 0, 0, 0, 0);
> +}
> +
> +static int meson_sec_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> + struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> + return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
> + MESON_SIP_WDT_DISABLE, 0, 0, 0, 0);
> +}
> +
> +static int meson_sec_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> + struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> + return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
> + MESON_SIP_WDT_PING, 0, 0, 0, 0);
> +}
> +
> +static int meson_sec_wdt_set_timeout(struct watchdog_device *wdt_dev,
> + unsigned int timeout)
> +{
> + struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> + wdt_dev->timeout = timeout;
> +
> + return meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
> + MESON_SIP_WDT_SETTIMEOUT,
> + wdt_dev->timeout * 1000, 0, 0, 0);
> +}
> +
> +static unsigned int meson_sec_wdt_get_timeleft(struct watchdog_device *wdt_dev)
> +{
> + int ret;
> + unsigned int timeleft;
> + struct meson_sec_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> + ret = meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, &timeleft,
> + MESON_SIP_WDT_GETTIMELEFT, 0, 0, 0, 0);
> +
> + if (ret)
> + return 0;
> +
> + return timeleft;
> +}
> +
> +static const struct watchdog_ops meson_sec_wdt_ops = {
> + .start = meson_sec_wdt_start,
> + .stop = meson_sec_wdt_stop,
> + .ping = meson_sec_wdt_ping,
> + .set_timeout = meson_sec_wdt_set_timeout,
> + .get_timeleft = meson_sec_wdt_get_timeleft,
> +};
> +
> +static const struct watchdog_info meson_sec_wdt_info = {
> + .identity = "Meson Secure Watchdog Timer",
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +};
> +
> +static int __maybe_unused meson_sec_wdt_resume(struct device *dev)
> +{
> + struct meson_sec_wdt *data = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&data->wdt_dev))
> + return meson_sec_wdt_start(&data->wdt_dev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused meson_sec_wdt_suspend(struct device *dev)
> +{
> + struct meson_sec_wdt *data = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&data->wdt_dev))
> + return meson_sec_wdt_stop(&data->wdt_dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops meson_sec_wdt_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(meson_sec_wdt_suspend, meson_sec_wdt_resume)
> +};
> +
> +static const struct of_device_id meson_sec_wdt_dt_ids[] = {
> + { .compatible = "amlogic,meson-sec-wdt", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, meson_sec_wdt_dt_ids);
> +
> +static int meson_sec_wdt_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct meson_sec_wdt *data;
> + struct device_node *sm_np;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + sm_np = of_find_compatible_node(NULL, NULL, "amlogic,meson-gxbb-sm");
> + if (!sm_np) {
> + dev_err(&pdev->dev, "no secure-monitor node\n");
> + return -EINVAL;
> + }
> +
> + data->fw = meson_sm_get(sm_np);
> + of_node_put(sm_np);
> + if (!data->fw)
> + return -EPROBE_DEFER;
> +
> + platform_set_drvdata(pdev, data);
> +
> + data->wdt_dev.parent = dev;
> + data->wdt_dev.info = &meson_sec_wdt_info;
> + data->wdt_dev.ops = &meson_sec_wdt_ops;
> + data->wdt_dev.max_hw_heartbeat_ms = MAX_TIMEOUT_MS;
> + data->wdt_dev.min_timeout = 1;
> + data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> + watchdog_set_drvdata(&data->wdt_dev, data);
> + watchdog_init_timeout(&data->wdt_dev, 0, dev);
> +
> + ret = meson_sm_call(data->fw, SM_A1_WATCHDOG_OPS, NULL,
> + MESON_SIP_WDT_INIT,
> + data->wdt_dev.timeout * 1000, 0, 0, 0);
> + if (ret)
> + return ret;
> +
> + watchdog_stop_on_reboot(&data->wdt_dev);
> +
> + return devm_watchdog_register_device(dev, &data->wdt_dev);
> +}
> +
> +static struct platform_driver meson_sec_wdt_driver = {
> + .probe = meson_sec_wdt_probe,
> + .driver = {
> + .name = "meson-sec-wdt",
> + .pm = &meson_sec_wdt_pm_ops,
> + .of_match_table = meson_sec_wdt_dt_ids,
> + },
> +};
> +
> +module_platform_driver(meson_sec_wdt_driver);
> +
> +MODULE_AUTHOR("Xingyu Chen <[email protected]>");
> +MODULE_DESCRIPTION("Amlogic Secure Watchdog Timer Driver");
> +MODULE_LICENSE("Dual MIT/GPL");
>
Hi, Rob
On 2019/12/19 4:34, Rob Herring wrote:
> On Wed, Dec 18, 2019 at 03:37:49PM +0800, Xingyu Chen wrote:
>> Hi, Guenter Martin
>>
>> On 2019/12/16 21:30, Guenter Roeck wrote:
>>> On 12/15/19 10:03 PM, Xingyu Chen wrote:
>>>> Hi, Martin
>>>>
>>>> Sorry for the late reply.
>>>>
>>>> On 2019/12/13 4:05, Martin Blumenstingl wrote:
>>>>> Hi Xingyu and Rob,
>>>>>
>>>>> On Thu, Dec 12, 2019 at 1:20 PM Xingyu Chen
>>>>> <[email protected]> wrote:
>>>>> [...]
>>>>>> +examples:
>>>>>> + - |
>>>>>> + watchdog {
>>>>>> + compatible = "amlogic,meson-sec-wdt";
>>>>>> + timeout-sec = <60>;
>>>>>> + };
>>>>> in v3 of this patch Rob commented that there shouldn't be an OF node
>>>>> if there are no additional properties
>>>>> with timeout-sec there's now an additional property so my
>>>>> understanding is that it's fine to have an OF node
>>>> Your understanding is correct.
>>>>> what I don't understand yet is where this node should be placed.
>>>>> is it supposed to be a child node of the secure monitor node (for
>>>>> which we already have a binding here:
>>>>> Documentation/devicetree/bindings/firmware/meson/meson_sm.txt) or
>>>>> where else would we place it inside the .dts?
>>>> IMO, Although the watchdog node need to reference the meson_sm
>>>> node, there is no
>>>> bus-like dependencies between the devices which the two nodes
>>>> corresponding to.
>>>> so i think that the watchdog node as child node of meson_sm maybe
>>>> not appropriate.
>>> The watchdog driver needs the meson SM's dt node, and it depends on the
>>> existence
>>> of that node. That seems enough of a relationship to warrant having it
>>> as child note.
>> Thanks for your reply, if i take the wdt node as child of secure monitor
>> (sm), how should
>> i register or find the wdt device ?
>>
>> I only think of the following three methods :
>> 1). update the sm driver，and scan®ister wdt device when the sm driver
>> probes(It is like i2c), but there
>> are too many changes involved.
> Just add of_platform_default_populate() call and clean-up calls. That's
> not what I'd call 'too many changes'.
Thanks for your guidance.
>
>
>> 2). add "simple-bus" key string to compatible of sm node, and it will make
>> the child node is registered as
>> platform device, but it seems that the key string is not match current
>> scene.
> You previously said it's not a bus...
>
>> secure-monitor {
>> compatible = "amlogic,meson-gxbb-sm", "simple-bus";
>>
>> watchdog {
>> compatible = "amlogic,meson-sec-wdt";
>> timeout-sec = <60>;
>> }
>> }
>>
>> 3). don't register device, and find directly the watchdog node by using the
>> of_* API in watchdog
>> driver (Eg: linux-4.x/drivers/tee/optee/core.c)
>>
>> secure-monitor {
>> compatible = "amlogic,meson-gxbb-sm";
>>
>> watchdog {
>> compatible = "amlogic,meson-sec-wdt";
>> timeout-sec = <60>;
>> }
>> }
>>
>> The method 3 looks better for me, do you have a better suggestion ? Thanks
>>
>> BR
>>> Guenter
>>>
>>> .
>>>
> .
>