On 26-02-06, Ahmad Fatoum wrote:
> 
> 
> 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 <[email protected]>
> > ---
> >  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.

You're right, I forgot to set it since pinging the watchdog within
barebox is a dev-feature :/ Fixed it with the new version.

> > +};
> > +
> > +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.

You're right, I adapted the code. Since the ping() is optional and the
set_timeout() isn't I went the way of supplying only the set_timeout().

However, I convered it to ping() and always return -ENOSYS in case of
set_timeout().

> > +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

Right! I added the WDOG_HW_RUNNING, since the watchdog is always
running.

Thanks for the review!

Regards,
  Marco


> 
> > +
> > +   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 |
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

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

Reply via email to