Re: [PATCH 4/5] watchdog: Add Hexagon EFI watchdog driver

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 2/5/26 4:45 PM, Marco Felsch wrote:
> The EFI system co-processor implements a watchdog device which must be
> pinged to inform the MCU that the system is up and running.
> 
> Normaly the ping is done by Linux but sometimes it can become necessary
> to do it within barebox too, e.g. to allow barebox debugging and
> development.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/watchdog/Kconfig       |  9 +++++
>  drivers/watchdog/Makefile      |  1 +
>  drivers/watchdog/hgs_efi_wdt.c | 88 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bf18782bdb58b20240d7762ce32f98c78c4cd12d..c962e8f22e5a7cb21ee93b3f82189cc8536781b9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -195,4 +195,13 @@ config K3_RTI_WDT
>  	  Say Y here if you want to include support for the K3 watchdog
>  	  timer (RTI module) available in the K3 generation of processors.
>  
> +config HGS_EFI_WATCHDOG
> +	bool "Hexagon Geosystems EFI watchdog"
> +	depends on MFD_HGS_EFI || COMPILE_TEST

Thanks for making sure it compiles under COMPILE_TEST. :)


> +#include <mfd/hgs-efi.h>
> +
> +struct hgs_efi_wdt_data {
> +	unsigned int msg_id;
> +};
> +
> +struct hgs_efi_wdt {
> +	struct watchdog wdd;
> +	struct hgs_efi *efi;
> +	const struct hgs_efi_wdt_data *data;
> +	bool is_running;

This is only ever read, but set no where.

> +};
> +
> +static struct hgs_efi_wdt *to_hgs_efi_wdt(struct watchdog *wdd)
> +{
> +	return container_of(wdd, struct hgs_efi_wdt, wdd);
> +}
> +
> +static int hgs_efi_wdt_set_timeout(struct watchdog *wdd, unsigned int timeout)
> +{
> +	struct hgs_efi_wdt *efi_wd = to_hgs_efi_wdt(wdd);
> +	struct device *dev = &wdd->dev;
> +	struct hgs_sep_cmd cmd = {
> +		.type = HGS_SEP_MSG_TYPE_EVENT,
> +		.msg_id = efi_wd->data->msg_id,
> +	};
> +	int ret;
> +
> +	/* The watchdog can't be turned of */
> +	if (efi_wd->is_running && !timeout)
> +		return -ENOSYS;
> +
> +	/* The EFI watchdog doesn't have a timeout, once pinged */
> +	if (efi_wd->is_running)
> +		return 0;

A watchdog can also implement a ping callback, which may be suitable here.

> +static int hgs_efi_wdt_drv_probe(struct device *dev)
> +{
> +	struct hgs_efi_wdt *efi_wd;
> +	struct watchdog *wdd;
> +	int ret;
> +
> +	efi_wd = xzalloc(sizeof(*efi_wd));
> +	efi_wd->efi = dev_get_priv(dev->parent);
> +	efi_wd->data = of_device_get_match_data(dev);
> +
> +	wdd = &efi_wd->wdd;
> +	wdd->hwdev = dev;
> +	wdd->set_timeout = hgs_efi_wdt_set_timeout;

FYI, if you can query the EFI whether it's enabled, you can set
wdd->running to either WDOG_HW_RUNNING or WDOG_HW_NOT_RUNNING

> +
> +	ret = watchdog_register(wdd);
> +	if (ret)
> +		dev_err(dev, "Failed to register watchdog device\n");
> +
> +	return ret;
> +}
> +
> +const struct hgs_efi_wdt_data hgs_efi_wdt_gs05 = {
> +	.msg_id = 0,
> +};
> +
> +static struct of_device_id hgs_efi_wdt_of_match[] = {
> +	{ .compatible = "hgs,efi-gs05-wdt", .data = &hgs_efi_wdt_gs05 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, hgs_efi_wdt_of_match);
> +
> +static struct driver hgs_efi_wdt_driver = {
> +	.name		= "hgs-efi-wdt",
> +	.probe		= hgs_efi_wdt_drv_probe,
> +	.of_compatible	= hgs_efi_wdt_of_match,
> +};
> +device_platform_driver(hgs_efi_wdt_driver);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |