Thanks for the feedback, I'll try to circle back to it in a few days, perhaps 
on the weekend.


-------- Original Message --------
On 4/14/25 12:24 AM, Neil Armstrong <neil.armstr...@linaro.org> wrote:

>  Hi,
>  
>  On 13/04/2025 12:12, Paul Sajna wrote:
>  > From: Caleb Connolly <caleb.conno...@linaro.org>
>  >
>  > Some Qualcomm device vendors decide to turn the watchdog on in the
>  > bootloader, resulting in the device being reset if it isn't petted every
>  > ~30 seconds. Introduce a driver to disable the watchdog and prevent this
>  > annoying behaviour.
>  >
>  > Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
>  > Signed-off-by: Paul Sajna <he...@paulsajna.com>
>  > Tested-by: Paul Sajna <he...@paulsajna.com>
>  > ---
>  > This patch introduces a simple watchdog driver for Qualcomm devices.
>  >
>  > Some devices, including one I have been spending a lot of time with,
>  > the LG G7 ThinQ (codename judyln, arch sdm845) will reset within 30
>  > seconds if the watchdog is not serviced.
>  >
>  > I have tested this patch from Caleb Connolly that is over a year old and 
> has
>  > not yet been upstreamed, and it is very useful to me. I can hardly use
>  > u-boot on this device without it. I have received Caleb's permission to
>  > work on upstreaming this patch.
>  >
>  > This is my first contribution to U-Boot, and I am unfamiliar with
>  > mailing list workflows, so please be patient :)
>  > ---
>  >   configs/qcom_defconfig      |   2 +
>  >   drivers/watchdog/Kconfig    |   7 ++++
>  >   drivers/watchdog/Makefile   |   1 +
>  >   drivers/watchdog/qcom-wdt.c | 100 
> ++++++++++++++++++++++++++++++++++++++++++++
>  >   4 files changed, 110 insertions(+)
>  >
>  > diff --git a/configs/qcom_defconfig b/configs/qcom_defconfig
>  > index 
> 537806450dc4a61d3c617cdd2b0cfb8eab1c343c..0232f6d51c552bd06c8ba72b71edb5cb72fe0706
>  100644
>  > --- a/configs/qcom_defconfig
>  > +++ b/configs/qcom_defconfig
>  > @@ -144,3 +144,5 @@ CONFIG_VIDEO_FONT_16X32=y
>  >   CONFIG_SYS_WHITE_ON_BLACK=y
>  >   CONFIG_NO_FB_CLEAR=y
>  >   CONFIG_VIDEO_SIMPLE=y
>  > +CONFIG_WDT_QCOM=y
>  > +CONFIG_WDT=y
>  > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>  > index 
> 1bb67f5035231df9f6ce01adb08d074855393143..808f4e578e28e7812f93b393a7f7d95d9501cdd4
>  100644
>  > --- a/drivers/watchdog/Kconfig
>  > +++ b/drivers/watchdog/Kconfig
>  > @@ -335,6 +335,13 @@ config WDT_K3_RTI_FW_FILE
>  >
>  >   endif
>  >
>  > +config WDT_QCOM
>  > +  bool "Qualcomm watchdog timer support"
>  > +  depends on WDT && ARCH_SNAPDRAGON
>  > +  help
>  > +    Select this to enable Qualcomm watchdog timer, which can be found on
>  > +    some Qualcomm chips.
>  > +
>  >   config WDT_SANDBOX
>  >    bool "Enable Watchdog Timer support for Sandbox"
>  >    depends on SANDBOX && WDT
>  > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>  > index 
> e6bd4c587af6133c405dde6dbada8050debc781c..5eaabd4aba58c815de4165f901aecfba0e0da255
>  100644
>  > --- a/drivers/watchdog/Makefile
>  > +++ b/drivers/watchdog/Makefile
>  > @@ -54,3 +54,4 @@ obj-$(CONFIG_WDT_SUNXI) += sunxi_wdt.o
>  >   obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
>  >   obj-$(CONFIG_WDT_XILINX) += xilinx_wwdt.o
>  >   obj-$(CONFIG_WDT_ADI) += adi_wdt.o
>  > +obj-$(CONFIG_WDT_QCOM) += qcom-wdt.o
>  > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
>  > new file mode 100644
>  > index 
> 0000000000000000000000000000000000000000..a7ce31ef811c717fd16700aa7d984a000763a93e
>  > --- /dev/null
>  > +++ b/drivers/watchdog/qcom-wdt.c
>  > @@ -0,0 +1,100 @@
>  > +// SPDX-License-Identifier: GPL-2.0+
>  > +/*
>  > + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
>  > + * Copyright (c) Linaro Ltd. 2024
>  > + *
>  > + * Authors:
>  > + *   Caleb Connolly <caleb.conno...@linaro.org>
>  > + *
>  > + * Derived from linux/drivers/watchdog/qcom-wdt.c
>  > + */
>  > +
>  > +#include <dm.h>
>  > +#include <wdt.h>
>  > +
>  > +#include <asm/io.h>
>  > +
>  > +enum wdt_reg {
>  > +  WDT_RST,
>  > +  WDT_EN,
>  > +  WDT_STS,
>  > +  WDT_BARK_TIME,
>  > +  WDT_BITE_TIME,
>  > +};
>  > +
>  > +struct qcom_wdt_match_data {
>  > +  const u32 *offset;
>  > +};
>  > +
>  > +struct qcom_wdt {
>  > +  void __iomem *base;
>  > +  const u32 *layout;
>  > +};
>  > +
>  > +static const u32 reg_offset_data_kpss[] = {
>  > +  [WDT_RST] = 0x4,
>  > +  [WDT_EN] = 0x8,
>  > +  [WDT_STS] = 0xC,
>  > +  [WDT_BARK_TIME] = 0x10,
>  > +  [WDT_BITE_TIME] = 0x14,
>  > +};
>  > +
>  > +static const struct qcom_wdt_match_data match_data_kpss = {
>  > +  .offset = reg_offset_data_kpss,
>  > +};
>  > +
>  > +static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
>  > +{
>  > +  return wdt->base + wdt->layout[reg];
>  > +}
>  > +
>  > +int qcom_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>  > +{
>  > +  /* unimplemented */
>  > +  return 0;
>  > +}
>  > +
>  > +int qcom_wdt_stop(struct udevice *dev)
>  > +{
>  > +  struct qcom_wdt *wdt = dev_get_priv(dev);
>  > +
>  > +  writel(0, wdt_addr(wdt, WDT_EN));
>  > +  if (readl(wdt_addr(wdt, WDT_EN))) {
>  > +          printf("Failed to disable Qualcomm watchdog!\n");
>  > +          return -EIO;
>  > +  }
>  > +
>  > +  return 0;
>  > +}
>  > +
>  > +static int qcom_wdt_probe(struct udevice *dev)
>  > +{
>  > +  struct qcom_wdt *wdt = dev_get_priv(dev);
>  > +  struct qcom_wdt_match_data *data = (void *)dev_get_driver_data(dev);
>  > +  int ret;
>  > +
>  > +  wdt->base = dev_read_addr_ptr(dev);
>  > +  wdt->layout = data->offset;
>  > +
>  > +  ret = qcom_wdt_stop(dev);
>  
>  Fine but perhaps just add a simple code into qcom_wdt_start() and add
>  a reset callback to allow people to actually use the wdog from u-boot
>  with CONFIG_WATCHDOG_AUTOSTART and/or u-boot,autostart.
>  
>  No need to implement set_timeout, we can use the bootloader value.
>  
>  > +
>  > +  return ret;
>  > +}
>  > +
>  > +static const struct wdt_ops qcom_wdt_ops = {
>  > +  .stop = qcom_wdt_stop,
>  
>  You forgot qcom_wdt_start()
>  
>  > +};
>  > +
>  > +static const struct udevice_id qcom_wdt_ids[] = {
>  > +  { .compatible = "qcom,kpss-wdt", .data = (ulong)&match_data_kpss },
>  > +  {}
>  > +};
>  > +
>  > +U_BOOT_DRIVER(qcom_wdt) = {
>  > +  .name = "qcom_wdt",
>  > +  .id = UCLASS_WDT,
>  > +  .of_match = qcom_wdt_ids,
>  > +  .ops = &qcom_wdt_ops,
>  > +  .probe = qcom_wdt_probe,
>  > +  .priv_auto = sizeof(struct qcom_wdt),
>  > +};
>  >
>  > ---
>  > base-commit: f3563fc30356c8079527b90e532407050dff7c7b
>  > change-id: 20250413-b4-qcom-wdt-e7b913fc375e
>  >
>  > Best regards,
>  
>  
>

Reply via email to