* [PATCH 1/2 v4] dt-bindings: watchdog: marvell GTI system watchdog driver @ 2023-04-27 7:14 Bharat Bhushan 2023-04-27 7:14 ` [PATCH 2/2 v4] Watchdog: Add marvell GTI " Bharat Bhushan 0 siblings, 1 reply; 6+ messages in thread From: Bharat Bhushan @ 2023-04-27 7:14 UTC (permalink / raw) To: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree, linux-kernel, sgoutham Cc: Bharat Bhushan Add binding documentation for the Marvell GTI system watchdog driver. Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> Reviewed-by: Rob Herring <robh@kernel.org> --- v4: - Use generic name "GTI (Global Timer)" and removed OcteonTX2 as this driver is applicable for other marvell devices as well. There are no logical changes - Added Reviewed-by: Rob Herring <robh@kernel.org> .../bindings/watchdog/marvell,gti-wdt.yaml | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml diff --git a/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file mode 100644 index 000000000000..4d34e9635ac1 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml @@ -0,0 +1,46 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/watchdog/marvell,gti-wdt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell GTI system watchdog + +allOf: + - $ref: watchdog.yaml# + +maintainers: + - Bharat Bhushan <bbhushan2@marvell.com> + +properties: + compatible: + const: marvell,gti-wdt + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + soc { + #address-cells = <2>; + #size-cells = <2>; + + watchdog@802000040000 { + compatible = "marvell,gti-wdt"; + reg = <0x00008020 0x00040000 0x00000000 0x00020000>; + interrupts = <0 38 IRQ_TYPE_EDGE_RISING>; + }; + }; + +... -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver 2023-04-27 7:14 [PATCH 1/2 v4] dt-bindings: watchdog: marvell GTI system watchdog driver Bharat Bhushan @ 2023-04-27 7:14 ` Bharat Bhushan 2023-04-27 21:29 ` kernel test robot 2023-04-29 15:14 ` Guenter Roeck 0 siblings, 2 replies; 6+ messages in thread From: Bharat Bhushan @ 2023-04-27 7:14 UTC (permalink / raw) To: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree, linux-kernel, sgoutham Cc: Bharat Bhushan This patch add support for Marvell GTI watchdog. Global timer unit (GTI) support hardware watchdog timer. Software programs watchdog timer to generate interrupt on first timeout, second timeout is configured to be ignored and system reboots on third timeout. Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> --- v4: - Use generic name "GTI (Global Timer)" and removed OcteonTX2 as this driver is applicable for other marvell devices as well. drivers/watchdog/Kconfig | 11 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/marvell_gti_wdt.c | 271 +++++++++++++++++++++++++++++ 3 files changed, 283 insertions(+) create mode 100644 drivers/watchdog/marvell_gti_wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f0872970daf9..a96e7d7c0ad2 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1779,6 +1779,17 @@ config OCTEON_WDT from the first interrupt, it is then only poked when the device is written. +config MARVELL_GTI_WDT + tristate "Marvell GTI Watchdog driver" + depends on ARCH_THUNDER || COMPILE_TEST + help + Marvell GTI hardware supports watchdog timer. First timeout + works as watchdog pretimeout and installed interrupt handler + will be called on first timeout. Hardware can generate interrupt + to SCP on second timeout but it is not enabled, So second + timeout is ignored. If device poke does not happen then system + will reboot on third timeout. + config BCM2835_WDT tristate "Broadcom BCM2835 hardware watchdog" depends on ARCH_BCM2835 || (OF && COMPILE_TEST) diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 9cbf6580f16c..bd425408fcaa 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o # X86 (i386 + ia64 + x86_64) Architecture obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o diff --git a/drivers/watchdog/marvell_gti_wdt.c b/drivers/watchdog/marvell_gti_wdt.c new file mode 100644 index 000000000000..a3f0740dedca --- /dev/null +++ b/drivers/watchdog/marvell_gti_wdt.c @@ -0,0 +1,271 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Marvell GTI Watchdog driver + * + * Copyright (C) 2023 Marvell. + */ + +#include <linux/module.h> +#include <linux/interrupt.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/watchdog.h> + +#include <asm/arch_timer.h> + +/* + * Hardware supports following mode of operation: + * 1) Interrupt Only: + * This will generate the interrupt to arm core whenever timeout happens. + * + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). + * This will generate interrupt to arm core on 1st timeout happens + * This will generate interrupt to SCP processor on 2nd timeout happens + * + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. + * This will generate interrupt to arm core on 1st timeout happens + * Will generate interrupt to SCP processor on 2nd timeout happens, + * if interrupt is configured. + * Reboot on 3rd timeout. + * + * Driver will use hardware in mode-3 above so that system can reboot in case + * a hardware hang. Also h/w is configured not to generate SCP interrupt, so + * effectively 2nd timeout is ignored within hardware. + */ + + +/* GTI CWD Watchdog Registers */ +#define GTI_CWD_GLOBAL_WDOG_IDX 63 +#define GTI_CWD_WDOG (0x8 * GTI_CWD_GLOBAL_WDOG_IDX) +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 +#define GTI_CWD_WDOG_MODE_MASK 0x3 +#define GTI_CWD_WDOG_LEN_SHIFT 4 +#define GTI_CWD_WDOG_CNT_SHIFT 20 + +/* GTI Per-core Watchdog Interrupt Register */ +#define GTI_CWD_INT 0x200 +#define GTI_CWD_INT_PENDING_STATUS (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) + +/* GTI Per-core Watchdog Interrupt Enable Clear Register */ +#define GTI_CWD_INT_ENA_CLR 0x210 +#define GTI_CWD_INT_ENA_CLR_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) + +/* GTI Per-core Watchdog Interrupt Enable Set Register */ +#define GTI_CWD_INT_ENA_SET 0x218 +#define GTI_CWD_INT_ENA_SET_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) + +/* GTI Per-core Watchdog Poke Registers */ +#define GTI_CWD_POKE (0x10000 + 0x8 * GTI_CWD_GLOBAL_WDOG_IDX) +#define GTI_CWD_POKE_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) + +struct gti_wdt_priv { + struct watchdog_device wdev; + void __iomem *base; + u64 clock_freq; + int irq; +}; + +static irqreturn_t gti_wdt_interrupt(int irq, void *data) +{ + struct watchdog_device *wdev = data; + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + + /* Clear Interrupt Pending Status */ + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); + + watchdog_notify_pretimeout(wdev); + + return IRQ_HANDLED; +} + +static int gti_wdt_ping(struct watchdog_device *wdev) +{ + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + + writeq(GTI_CWD_POKE_VAL, priv->base + GTI_CWD_POKE); + + return 0; +} + +static int gti_wdt_start(struct watchdog_device *wdev) +{ + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + u64 regval; + + if (!wdev->pretimeout) + return -EINVAL; + + set_bit(WDOG_HW_RUNNING, &wdev->status); + + /* Clear any pending interrupt */ + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); + + /* Enable Interrupt */ + writeq(GTI_CWD_INT_ENA_SET_VAL, priv->base + GTI_CWD_INT_ENA_SET); + + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ + regval = readq(priv->base + GTI_CWD_WDOG); + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; + writeq(regval, priv->base + GTI_CWD_WDOG); + + return 0; +} + +static int gti_wdt_stop(struct watchdog_device *wdev) +{ + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + u64 regval; + + /* Disable Interrupt */ + writeq(GTI_CWD_INT_ENA_CLR_VAL, priv->base + GTI_CWD_INT_ENA_CLR); + + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ + regval = readq(priv->base + GTI_CWD_WDOG); + regval &= ~GTI_CWD_WDOG_MODE_MASK; + writeq(regval, priv->base + GTI_CWD_WDOG); + + return 0; +} + +static int gti_wdt_settimeout(struct watchdog_device *wdev, + unsigned int timeout) +{ + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + u64 timeout_wdog, regval; + + /* Update new timeout */ + wdev->timeout = timeout; + + /* Pretimeout is 1/3 of timeout */ + wdev->pretimeout = timeout / 3; + if (!wdev->pretimeout) + return -EINVAL; + + /* Get clock cycles from timeout in second */ + timeout_wdog = wdev->pretimeout * priv->clock_freq; + + /* Watchdog counts in 1024 cycle steps */ + timeout_wdog = timeout_wdog >> 10; + + timeout_wdog = (timeout_wdog + 0xff) >> 8; + if (timeout_wdog >= 0x10000) + timeout_wdog = 0xffff; + + /* + * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits + * GTI_CWD_WDOG.CNT, need addition shift of 8. + */ + regval = readq(priv->base + GTI_CWD_WDOG); + regval &= GTI_CWD_WDOG_MODE_MASK; + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); + writeq(regval, priv->base + GTI_CWD_WDOG); + + return 0; +} + +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, + unsigned int timeout) +{ + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); + struct watchdog_device *wdog_dev = &priv->wdev; + + /* pretimeout should 1/3 of max_timeout */ + if ((timeout * 3) <= wdog_dev->max_timeout) + return gti_wdt_settimeout(wdev, timeout * 3); + + return -EINVAL; +} + +static const struct watchdog_info gti_wdt_ident = { + .identity = "Marvell GTI watchdog", + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, +}; + +static const struct watchdog_ops gti_wdt_ops = { + .owner = THIS_MODULE, + .start = gti_wdt_start, + .stop = gti_wdt_stop, + .ping = gti_wdt_ping, + .set_timeout = gti_wdt_settimeout, + .set_pretimeout = gti_wdt_set_pretimeout, +}; + +static int gti_wdt_probe(struct platform_device *pdev) +{ + struct gti_wdt_priv *priv; + struct device *dev = &pdev->dev; + struct watchdog_device *wdog_dev; + int irq; + int err; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(priv->base)) + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), + "reg property not valid/found\n"); + + priv->clock_freq = arch_timer_get_cntfrq(); + + wdog_dev = &priv->wdev; + wdog_dev->info = >i_wdt_ident, + wdog_dev->ops = >i_wdt_ops, + wdog_dev->parent = dev; + /* + * Watchdog counter is 24 bit where lower 8 bits are zeros, so maximum + * value that can be programmed in hardware is 0xffff00. This counter + * decrements every one microsec. + * Hardware will generate interrupt on first timeout (pretimeout) + * reset the system on 3rd timeout. + * Max watchdog pretimeout can be 16 sec, so max timeout is 48 sec. + * Let's have min pretimeout to 1sec, so min timeout is 3sec. + */ + wdog_dev->max_timeout = 48; + wdog_dev->min_timeout = 3; + wdog_dev->timeout = 30; + wdog_dev->pretimeout = wdog_dev->timeout / 3; + + priv->irq = irq; + watchdog_set_drvdata(wdog_dev, priv); + platform_set_drvdata(pdev, priv); + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); + watchdog_stop_on_reboot(wdog_dev); + watchdog_stop_on_unregister(wdog_dev); + + err = devm_watchdog_register_device(dev, wdog_dev); + if (err) + return err; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return dev_err_probe(&pdev->dev, irq, "IRQ resource not found\n"); + + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, + pdev->name, &priv->wdev); + if (err) + return dev_err_probe(dev, err, "Failed to register interrupt handler\n"); + + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev->timeout); + return 0; +} + +static const struct of_device_id gti_wdt_of_match[] = { + { .compatible = "marvell,gti-wdt", }, + { }, +}; +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); + +static struct platform_driver gti_wdt_driver = { + .driver = { + .name = "gti-wdt", + .of_match_table = gti_wdt_of_match, + }, + .probe = gti_wdt_probe, +}; +module_platform_driver(gti_wdt_driver); + +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); +MODULE_DESCRIPTION("Marvell GTI watchdog driver"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver 2023-04-27 7:14 ` [PATCH 2/2 v4] Watchdog: Add marvell GTI " Bharat Bhushan @ 2023-04-27 21:29 ` kernel test robot 2023-04-29 15:14 ` Guenter Roeck 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2023-04-27 21:29 UTC (permalink / raw) To: Bharat Bhushan, wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree, linux-kernel, sgoutham Cc: oe-kbuild-all, Bharat Bhushan Hi Bharat, kernel test robot noticed the following build errors: [auto build test ERROR on robh/for-next] [also build test ERROR on groeck-staging/hwmon-next linus/master v6.3 next-20230427] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bharat-Bhushan/Watchdog-Add-marvell-GTI-watchdog-driver/20230427-154706 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20230427071408.8493-2-bbhushan2%40marvell.com patch subject: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20230428/202304280541.b8a8NzCY-lkp@intel.com/config) compiler: powerpc-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/86559345de78e2e9f634ba9835a63764b6f88fd5 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Bharat-Bhushan/Watchdog-Add-marvell-GTI-watchdog-driver/20230427-154706 git checkout 86559345de78e2e9f634ba9835a63764b6f88fd5 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/watchdog/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202304280541.b8a8NzCY-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/watchdog/marvell_gti_wdt.c:13:10: fatal error: asm/arch_timer.h: No such file or directory 13 | #include <asm/arch_timer.h> | ^~~~~~~~~~~~~~~~~~ compilation terminated. vim +13 drivers/watchdog/marvell_gti_wdt.c 12 > 13 #include <asm/arch_timer.h> 14 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver 2023-04-27 7:14 ` [PATCH 2/2 v4] Watchdog: Add marvell GTI " Bharat Bhushan 2023-04-27 21:29 ` kernel test robot @ 2023-04-29 15:14 ` Guenter Roeck 2023-05-02 17:05 ` [EXT] " Bharat Bhushan 1 sibling, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2023-04-29 15:14 UTC (permalink / raw) To: Bharat Bhushan, wim, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree, linux-kernel, sgoutham On 4/27/23 00:14, Bharat Bhushan wrote: > This patch add support for Marvell GTI watchdog. Global timer > unit (GTI) support hardware watchdog timer. Software programs > watchdog timer to generate interrupt on first timeout, second > timeout is configured to be ignored and system reboots on > third timeout. > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > --- > v4: > - Use generic name "GTI (Global Timer)" and removed OcteonTX2 as this driver > is applicable for other marvell devices as well. > > drivers/watchdog/Kconfig | 11 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/marvell_gti_wdt.c | 271 +++++++++++++++++++++++++++++ > 3 files changed, 283 insertions(+) > create mode 100644 drivers/watchdog/marvell_gti_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index f0872970daf9..a96e7d7c0ad2 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -1779,6 +1779,17 @@ config OCTEON_WDT > from the first interrupt, it is then only poked when the > device is written. > > +config MARVELL_GTI_WDT > + tristate "Marvell GTI Watchdog driver" > + depends on ARCH_THUNDER || COMPILE_TEST As 0-day reported, COMPILE_TEST does not work if there are asm includes in the code. > + help > + Marvell GTI hardware supports watchdog timer. First timeout > + works as watchdog pretimeout and installed interrupt handler > + will be called on first timeout. Hardware can generate interrupt > + to SCP on second timeout but it is not enabled, So second > + timeout is ignored. If device poke does not happen then system > + will reboot on third timeout. > + > config BCM2835_WDT > tristate "Broadcom BCM2835 hardware watchdog" > depends on ARCH_BCM2835 || (OF && COMPILE_TEST) > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 9cbf6580f16c..bd425408fcaa 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o > +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o > > # X86 (i386 + ia64 + x86_64) Architecture > obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o > diff --git a/drivers/watchdog/marvell_gti_wdt.c b/drivers/watchdog/marvell_gti_wdt.c > new file mode 100644 > index 000000000000..a3f0740dedca > --- /dev/null > +++ b/drivers/watchdog/marvell_gti_wdt.c > @@ -0,0 +1,271 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Marvell GTI Watchdog driver > + * > + * Copyright (C) 2023 Marvell. > + */ > + > +#include <linux/module.h> > +#include <linux/interrupt.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/watchdog.h> > + > +#include <asm/arch_timer.h> > + > +/* > + * Hardware supports following mode of operation: > + * 1) Interrupt Only: > + * This will generate the interrupt to arm core whenever timeout happens. > + * > + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). > + * This will generate interrupt to arm core on 1st timeout happens > + * This will generate interrupt to SCP processor on 2nd timeout happens > + * > + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. > + * This will generate interrupt to arm core on 1st timeout happens > + * Will generate interrupt to SCP processor on 2nd timeout happens, > + * if interrupt is configured. > + * Reboot on 3rd timeout. > + * > + * Driver will use hardware in mode-3 above so that system can reboot in case > + * a hardware hang. Also h/w is configured not to generate SCP interrupt, so > + * effectively 2nd timeout is ignored within hardware. > + */ > + > + > +/* GTI CWD Watchdog Registers */ > +#define GTI_CWD_GLOBAL_WDOG_IDX 63 It might make sense to explain "63". > +#define GTI_CWD_WDOG (0x8 * GTI_CWD_GLOBAL_WDOG_IDX) > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 > +#define GTI_CWD_WDOG_MODE_MASK 0x3 > +#define GTI_CWD_WDOG_LEN_SHIFT 4 > +#define GTI_CWD_WDOG_CNT_SHIFT 20 > + > +/* GTI Per-core Watchdog Interrupt Register */ > +#define GTI_CWD_INT 0x200 > +#define GTI_CWD_INT_PENDING_STATUS (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) Please use BIT_ULL(). > + > +/* GTI Per-core Watchdog Interrupt Enable Clear Register */ > +#define GTI_CWD_INT_ENA_CLR 0x210 > +#define GTI_CWD_INT_ENA_CLR_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) > + > +/* GTI Per-core Watchdog Interrupt Enable Set Register */ > +#define GTI_CWD_INT_ENA_SET 0x218 > +#define GTI_CWD_INT_ENA_SET_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) > + > +/* GTI Per-core Watchdog Poke Registers */ > +#define GTI_CWD_POKE (0x10000 + 0x8 * GTI_CWD_GLOBAL_WDOG_IDX) There should be a comment explaining the "8". Also, there is a define for "0x8 * GTI_CWD_GLOBAL_WDOG_IDX" above. Why not use it here ? > +#define GTI_CWD_POKE_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) I don't think it makes sense to have three defines with the same value. > + > +struct gti_wdt_priv { > + struct watchdog_device wdev; > + void __iomem *base; > + u64 clock_freq; > + int irq; > +}; > + > +static irqreturn_t gti_wdt_interrupt(int irq, void *data) > +{ > + struct watchdog_device *wdev = data; > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + > + /* Clear Interrupt Pending Status */ > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > + > + watchdog_notify_pretimeout(wdev); > + > + return IRQ_HANDLED; > +} > + > +static int gti_wdt_ping(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + > + writeq(GTI_CWD_POKE_VAL, priv->base + GTI_CWD_POKE); > + > + return 0; > +} > + > +static int gti_wdt_start(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 regval; > + > + if (!wdev->pretimeout) > + return -EINVAL; > + > + set_bit(WDOG_HW_RUNNING, &wdev->status); > + > + /* Clear any pending interrupt */ > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > + > + /* Enable Interrupt */ > + writeq(GTI_CWD_INT_ENA_SET_VAL, priv->base + GTI_CWD_INT_ENA_SET); > + > + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ > + regval = readq(priv->base + GTI_CWD_WDOG); > + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; > + writeq(regval, priv->base + GTI_CWD_WDOG); > + > + return 0; > +} > + > +static int gti_wdt_stop(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 regval; > + > + /* Disable Interrupt */ > + writeq(GTI_CWD_INT_ENA_CLR_VAL, priv->base + GTI_CWD_INT_ENA_CLR); > + > + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ > + regval = readq(priv->base + GTI_CWD_WDOG); > + regval &= ~GTI_CWD_WDOG_MODE_MASK; > + writeq(regval, priv->base + GTI_CWD_WDOG); > + > + return 0; > +} > + > +static int gti_wdt_settimeout(struct watchdog_device *wdev, > + unsigned int timeout) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 timeout_wdog, regval; > + > + /* Update new timeout */ > + wdev->timeout = timeout; > + > + /* Pretimeout is 1/3 of timeout */ > + wdev->pretimeout = timeout / 3; > + if (!wdev->pretimeout) > + return -EINVAL; Since min_timeout is set to 3 seconds, this can not happen, and the check is unnecessary. > + > + /* Get clock cycles from timeout in second */ from pretimeout ? > + timeout_wdog = wdev->pretimeout * priv->clock_freq; > + > + /* Watchdog counts in 1024 cycle steps */ > + timeout_wdog = timeout_wdog >> 10; > + > + timeout_wdog = (timeout_wdog + 0xff) >> 8; > + if (timeout_wdog >= 0x10000) > + timeout_wdog = 0xffff; > + > + /* > + * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits > + * GTI_CWD_WDOG.CNT, need addition shift of 8. > + */ > + regval = readq(priv->base + GTI_CWD_WDOG); > + regval &= GTI_CWD_WDOG_MODE_MASK; > + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | > + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); > + writeq(regval, priv->base + GTI_CWD_WDOG); > + > + return 0; > +} > + > +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, > + unsigned int timeout) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + struct watchdog_device *wdog_dev = &priv->wdev; > + > + /* pretimeout should 1/3 of max_timeout */ > + if ((timeout * 3) <= wdog_dev->max_timeout) Unnecessary () > + return gti_wdt_settimeout(wdev, timeout * 3); > + > + return -EINVAL; > +} > + > +static const struct watchdog_info gti_wdt_ident = { > + .identity = "Marvell GTI watchdog", > + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | WDIOF_KEEPALIVEPING | > + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, > +}; > + > +static const struct watchdog_ops gti_wdt_ops = { > + .owner = THIS_MODULE, > + .start = gti_wdt_start, > + .stop = gti_wdt_stop, > + .ping = gti_wdt_ping, > + .set_timeout = gti_wdt_settimeout, > + .set_pretimeout = gti_wdt_set_pretimeout, > +}; > + > +static int gti_wdt_probe(struct platform_device *pdev) > +{ > + struct gti_wdt_priv *priv; > + struct device *dev = &pdev->dev; > + struct watchdog_device *wdog_dev; > + int irq; > + int err; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->base)) > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), > + "reg property not valid/found\n"); > + > + priv->clock_freq = arch_timer_get_cntfrq(); arch_timer_get_cntfrq() returns u32. Storing it as u64 to save a couple of typecasts really doesn't add value. Note that this limits compilability to arm and arm64. > + > + wdog_dev = &priv->wdev; > + wdog_dev->info = >i_wdt_ident, > + wdog_dev->ops = >i_wdt_ops, > + wdog_dev->parent = dev; > + /* > + * Watchdog counter is 24 bit where lower 8 bits are zeros, so maximum > + * value that can be programmed in hardware is 0xffff00. This counter > + * decrements every one microsec. > + * Hardware will generate interrupt on first timeout (pretimeout) > + * reset the system on 3rd timeout. > + * Max watchdog pretimeout can be 16 sec, so max timeout is 48 sec. > + * Let's have min pretimeout to 1sec, so min timeout is 3sec. > + */ > + wdog_dev->max_timeout = 48; The code setting the actual timeout checks for overflow, suggesting that the actual maximum timeout depends on the clock speed and may be less than 48 seconds. If that ever happens, the actual timeout would be set to less than 48 seconds, which could cause random reboots. > + wdog_dev->min_timeout = 3; > + wdog_dev->timeout = 30; > + wdog_dev->pretimeout = wdog_dev->timeout / 3; > + > + priv->irq = irq; > + watchdog_set_drvdata(wdog_dev, priv); > + platform_set_drvdata(pdev, priv); > + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); > + watchdog_stop_on_reboot(wdog_dev); > + watchdog_stop_on_unregister(wdog_dev); > + > + err = devm_watchdog_register_device(dev, wdog_dev); > + if (err) > + return err; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return dev_err_probe(&pdev->dev, irq, "IRQ resource not found\n"); > + > + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, > + pdev->name, &priv->wdev); > + if (err) > + return dev_err_probe(dev, err, "Failed to register interrupt handler\n"); > + > + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev->timeout); > + return 0; > +} > + > +static const struct of_device_id gti_wdt_of_match[] = { > + { .compatible = "marvell,gti-wdt", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); > + > +static struct platform_driver gti_wdt_driver = { > + .driver = { > + .name = "gti-wdt", > + .of_match_table = gti_wdt_of_match, > + }, > + .probe = gti_wdt_probe, > +}; > +module_platform_driver(gti_wdt_driver); > + > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); > +MODULE_DESCRIPTION("Marvell GTI watchdog driver"); ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver 2023-04-29 15:14 ` Guenter Roeck @ 2023-05-02 17:05 ` Bharat Bhushan 2023-05-03 12:13 ` Bharat Bhushan 0 siblings, 1 reply; 6+ messages in thread From: Bharat Bhushan @ 2023-05-02 17:05 UTC (permalink / raw) To: Guenter Roeck, wim@linux-watchdog.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Sunil Kovvuri Goutham Please see inline > -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Saturday, April 29, 2023 8:44 PM > To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux- > watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com> > Subject: [EXT] Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver > > External Email > > ---------------------------------------------------------------------- > On 4/27/23 00:14, Bharat Bhushan wrote: > > This patch add support for Marvell GTI watchdog. Global timer unit > > (GTI) support hardware watchdog timer. Software programs watchdog > > timer to generate interrupt on first timeout, second timeout is > > configured to be ignored and system reboots on third timeout. > > > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > > --- > > v4: > > - Use generic name "GTI (Global Timer)" and removed OcteonTX2 as this driver > > is applicable for other marvell devices as well. > > > > drivers/watchdog/Kconfig | 11 ++ > > drivers/watchdog/Makefile | 1 + > > drivers/watchdog/marvell_gti_wdt.c | 271 > +++++++++++++++++++++++++++++ > > 3 files changed, 283 insertions(+) > > create mode 100644 drivers/watchdog/marvell_gti_wdt.c > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index > > f0872970daf9..a96e7d7c0ad2 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -1779,6 +1779,17 @@ config OCTEON_WDT > > from the first interrupt, it is then only poked when the > > device is written. > > > > +config MARVELL_GTI_WDT > > + tristate "Marvell GTI Watchdog driver" > > + depends on ARCH_THUNDER || COMPILE_TEST > > As 0-day reported, COMPILE_TEST does not work if there are asm includes in the > code. Will remove asm include dependency in next version > > > + help > > + Marvell GTI hardware supports watchdog timer. First timeout > > + works as watchdog pretimeout and installed interrupt handler > > + will be called on first timeout. Hardware can generate interrupt > > + to SCP on second timeout but it is not enabled, So second > > + timeout is ignored. If device poke does not happen then system > > + will reboot on third timeout. > > + > > config BCM2835_WDT > > tristate "Broadcom BCM2835 hardware watchdog" > > depends on ARCH_BCM2835 || (OF && COMPILE_TEST) diff --git > > a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index > > 9cbf6580f16c..bd425408fcaa 100644 > > --- a/drivers/watchdog/Makefile > > +++ b/drivers/watchdog/Makefile > > @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o > > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > > obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o > > +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o > > > > # X86 (i386 + ia64 + x86_64) Architecture > > obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o diff --git > > a/drivers/watchdog/marvell_gti_wdt.c > > b/drivers/watchdog/marvell_gti_wdt.c > > new file mode 100644 > > index 000000000000..a3f0740dedca > > --- /dev/null > > +++ b/drivers/watchdog/marvell_gti_wdt.c > > @@ -0,0 +1,271 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Marvell GTI Watchdog driver > > + * > > + * Copyright (C) 2023 Marvell. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/interrupt.h> > > +#include <linux/of_platform.h> > > +#include <linux/platform_device.h> > > +#include <linux/watchdog.h> > > + > > +#include <asm/arch_timer.h> > > + > > +/* > > + * Hardware supports following mode of operation: > > + * 1) Interrupt Only: > > + * This will generate the interrupt to arm core whenever timeout happens. > > + * > > + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). > > + * This will generate interrupt to arm core on 1st timeout happens > > + * This will generate interrupt to SCP processor on 2nd timeout happens > > + * > > + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. > > + * This will generate interrupt to arm core on 1st timeout happens > > + * Will generate interrupt to SCP processor on 2nd timeout happens, > > + * if interrupt is configured. > > + * Reboot on 3rd timeout. > > + * > > + * Driver will use hardware in mode-3 above so that system can reboot > > +in case > > + * a hardware hang. Also h/w is configured not to generate SCP > > +interrupt, so > > + * effectively 2nd timeout is ignored within hardware. > > + */ > > + > > + > > +/* GTI CWD Watchdog Registers */ > > +#define GTI_CWD_GLOBAL_WDOG_IDX 63 > > It might make sense to explain "63". Okay, will add below comment /* GTI hardware block supports total 64 timers and timer 63 is used for system watchdog */ "IDX" seems confusing, will rename to "GTI_CWD_GLOBAL_WDOG_TIMER" > > > +#define GTI_CWD_WDOG (0x8 * > GTI_CWD_GLOBAL_WDOG_IDX) > > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 > > +#define GTI_CWD_WDOG_MODE_MASK 0x3 > > +#define GTI_CWD_WDOG_LEN_SHIFT 4 > > +#define GTI_CWD_WDOG_CNT_SHIFT 20 > > + > > +/* GTI Per-core Watchdog Interrupt Register */ > > +#define GTI_CWD_INT 0x200 > > +#define GTI_CWD_INT_PENDING_STATUS (1ULL << > GTI_CWD_GLOBAL_WDOG_IDX) > > Please use BIT_ULL(). Ok > > > + > > +/* GTI Per-core Watchdog Interrupt Enable Clear Register */ > > +#define GTI_CWD_INT_ENA_CLR 0x210 > > +#define GTI_CWD_INT_ENA_CLR_VAL (1ULL << > GTI_CWD_GLOBAL_WDOG_IDX) > > + > > +/* GTI Per-core Watchdog Interrupt Enable Set Register */ > > +#define GTI_CWD_INT_ENA_SET 0x218 > > +#define GTI_CWD_INT_ENA_SET_VAL (1ULL << > GTI_CWD_GLOBAL_WDOG_IDX) > > + > > +/* GTI Per-core Watchdog Poke Registers */ > > +#define GTI_CWD_POKE (0x10000 + 0x8 * > GTI_CWD_GLOBAL_WDOG_IDX) > > There should be a comment explaining the "8". GTI_CWD_POKE is poke register for watchdog counter GTI_CWD_GLOBAL_WDOG_TIMER . Above calculation is for getting the register address for watchdog timer GTI_CWD_GLOBAL_WDOG_TIMER Can add comment /* Each POKE register is 64bit and so GTI_CWD_POKE is register address for watchdog counter GTI_CWD_GLOBAL_WDOG_TIMER */ > Also, there is a define for "0x8 * > GTI_CWD_GLOBAL_WDOG_IDX" above. > Why not use it here ? GTI_CWD_GLOBAL_WDOG_TIMER is used for register address calculation, so will keep then separate to avoid any confusion. > > > +#define GTI_CWD_POKE_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) > > I don't think it makes sense to have three defines with the same value. Yes they are same value, but they are bit field in different h/w register. I want to keep a separate name to avoid any confusion. > > > + > > +struct gti_wdt_priv { > > + struct watchdog_device wdev; > > + void __iomem *base; > > + u64 clock_freq; > > + int irq; > > +}; > > + > > +static irqreturn_t gti_wdt_interrupt(int irq, void *data) { > > + struct watchdog_device *wdev = data; > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + > > + /* Clear Interrupt Pending Status */ > > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > > + > > + watchdog_notify_pretimeout(wdev); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int gti_wdt_ping(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + > > + writeq(GTI_CWD_POKE_VAL, priv->base + GTI_CWD_POKE); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_start(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 regval; > > + > > + if (!wdev->pretimeout) > > + return -EINVAL; > > + > > + set_bit(WDOG_HW_RUNNING, &wdev->status); > > + > > + /* Clear any pending interrupt */ > > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > > + > > + /* Enable Interrupt */ > > + writeq(GTI_CWD_INT_ENA_SET_VAL, priv->base + > GTI_CWD_INT_ENA_SET); > > + > > + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ > > + regval = readq(priv->base + GTI_CWD_WDOG); > > + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_stop(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 regval; > > + > > + /* Disable Interrupt */ > > + writeq(GTI_CWD_INT_ENA_CLR_VAL, priv->base + > GTI_CWD_INT_ENA_CLR); > > + > > + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ > > + regval = readq(priv->base + GTI_CWD_WDOG); > > + regval &= ~GTI_CWD_WDOG_MODE_MASK; > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_settimeout(struct watchdog_device *wdev, > > + unsigned int timeout) > > +{ > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 timeout_wdog, regval; > > + > > + /* Update new timeout */ > > + wdev->timeout = timeout; > > + > > + /* Pretimeout is 1/3 of timeout */ > > + wdev->pretimeout = timeout / 3; > > + if (!wdev->pretimeout) > > + return -EINVAL; > > Since min_timeout is set to 3 seconds, this can not happen, and the check is > unnecessary. > > > + > > + /* Get clock cycles from timeout in second */ > > from pretimeout ? Yes, will change comment. > > > + timeout_wdog = wdev->pretimeout * priv->clock_freq; > > + > > + /* Watchdog counts in 1024 cycle steps */ > > + timeout_wdog = timeout_wdog >> 10; > > + > > + timeout_wdog = (timeout_wdog + 0xff) >> 8; > > + if (timeout_wdog >= 0x10000) > > + timeout_wdog = 0xffff; > > + > > + /* > > + * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits > > + * GTI_CWD_WDOG.CNT, need addition shift of 8. > > + */ > > + regval = readq(priv->base + GTI_CWD_WDOG); > > + regval &= GTI_CWD_WDOG_MODE_MASK; > > + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | > > + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, > > + unsigned int timeout) > > +{ > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + struct watchdog_device *wdog_dev = &priv->wdev; > > + > > + /* pretimeout should 1/3 of max_timeout */ > > + if ((timeout * 3) <= wdog_dev->max_timeout) > > Unnecessary () > > > + return gti_wdt_settimeout(wdev, timeout * 3); > > + > > + return -EINVAL; > > +} > > + > > +static const struct watchdog_info gti_wdt_ident = { > > + .identity = "Marvell GTI watchdog", > > + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | > WDIOF_KEEPALIVEPING | > > + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, }; > > + > > +static const struct watchdog_ops gti_wdt_ops = { > > + .owner = THIS_MODULE, > > + .start = gti_wdt_start, > > + .stop = gti_wdt_stop, > > + .ping = gti_wdt_ping, > > + .set_timeout = gti_wdt_settimeout, > > + .set_pretimeout = gti_wdt_set_pretimeout, }; > > + > > +static int gti_wdt_probe(struct platform_device *pdev) { > > + struct gti_wdt_priv *priv; > > + struct device *dev = &pdev->dev; > > + struct watchdog_device *wdog_dev; > > + int irq; > > + int err; > > + > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(priv->base)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), > > + "reg property not valid/found\n"); > > + > > + priv->clock_freq = arch_timer_get_cntfrq(); > > arch_timer_get_cntfrq() returns u32. Storing it as u64 to save a couple of > typecasts really doesn't add value. > > Note that this limits compilability to arm and arm64. Will remove using arch_timer_get_cntfrq() and clock reference in device tree. > > > + > > + wdog_dev = &priv->wdev; > > + wdog_dev->info = >i_wdt_ident, > > + wdog_dev->ops = >i_wdt_ops, > > + wdog_dev->parent = dev; > > + /* > > + * Watchdog counter is 24 bit where lower 8 bits are zeros, so maximum > > + * value that can be programmed in hardware is 0xffff00. This counter > > + * decrements every one microsec. > > + * Hardware will generate interrupt on first timeout (pretimeout) > > + * reset the system on 3rd timeout. > > + * Max watchdog pretimeout can be 16 sec, so max timeout is 48 sec. > > + * Let's have min pretimeout to 1sec, so min timeout is 3sec. > > + */ > > + wdog_dev->max_timeout = 48; > > The code setting the actual timeout checks for overflow, suggesting that the > actual maximum timeout depends on the clock speed and may be less than 48 > seconds. If that ever happens, the actual timeout would be set to less than 48 > seconds, which could cause random reboots. Will use clock for these calculation. Thanks -Bharat > > > + wdog_dev->min_timeout = 3; > > + wdog_dev->timeout = 30; > > + wdog_dev->pretimeout = wdog_dev->timeout / 3; > > + > > + priv->irq = irq; > > + watchdog_set_drvdata(wdog_dev, priv); > > + platform_set_drvdata(pdev, priv); > > + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); > > + watchdog_stop_on_reboot(wdog_dev); > > + watchdog_stop_on_unregister(wdog_dev); > > + > > + err = devm_watchdog_register_device(dev, wdog_dev); > > + if (err) > > + return err; > > + > > + irq = platform_get_irq(pdev, 0); > > + if (irq < 0) > > + return dev_err_probe(&pdev->dev, irq, "IRQ resource not > found\n"); > > + > > + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, > > + pdev->name, &priv->wdev); > > + if (err) > > + return dev_err_probe(dev, err, "Failed to register interrupt > > +handler\n"); > > + > > + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev- > >timeout); > > + return 0; > > +} > > + > > +static const struct of_device_id gti_wdt_of_match[] = { > > + { .compatible = "marvell,gti-wdt", }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); > > + > > +static struct platform_driver gti_wdt_driver = { > > + .driver = { > > + .name = "gti-wdt", > > + .of_match_table = gti_wdt_of_match, > > + }, > > + .probe = gti_wdt_probe, > > +}; > > +module_platform_driver(gti_wdt_driver); > > + > > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); > > +MODULE_DESCRIPTION("Marvell GTI watchdog driver"); ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver 2023-05-02 17:05 ` [EXT] " Bharat Bhushan @ 2023-05-03 12:13 ` Bharat Bhushan 0 siblings, 0 replies; 6+ messages in thread From: Bharat Bhushan @ 2023-05-03 12:13 UTC (permalink / raw) To: Guenter Roeck, wim@linux-watchdog.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Sunil Kovvuri Goutham > -----Original Message----- > From: Bharat Bhushan > Sent: Tuesday, May 2, 2023 10:35 PM > To: Guenter Roeck <linux@roeck-us.net>; wim@linux-watchdog.org; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux- > watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com> > Subject: RE: [EXT] Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog driver > > Please see inline > > > -----Original Message----- > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > > Sent: Saturday, April 29, 2023 8:44 PM > > To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org; > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux- > > watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux- > > kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com> > > Subject: [EXT] Re: [PATCH 2/2 v4] Watchdog: Add marvell GTI watchdog > > driver > > > > External Email > > > > ---------------------------------------------------------------------- > > On 4/27/23 00:14, Bharat Bhushan wrote: > > > This patch add support for Marvell GTI watchdog. Global timer unit > > > (GTI) support hardware watchdog timer. Software programs watchdog > > > timer to generate interrupt on first timeout, second timeout is > > > configured to be ignored and system reboots on third timeout. > > > > > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > > > --- > > > v4: > > > - Use generic name "GTI (Global Timer)" and removed OcteonTX2 as this > driver > > > is applicable for other marvell devices as well. > > > > > > drivers/watchdog/Kconfig | 11 ++ > > > drivers/watchdog/Makefile | 1 + > > > drivers/watchdog/marvell_gti_wdt.c | 271 > > +++++++++++++++++++++++++++++ > > > 3 files changed, 283 insertions(+) > > > create mode 100644 drivers/watchdog/marvell_gti_wdt.c > > > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > > index > > > f0872970daf9..a96e7d7c0ad2 100644 > > > --- a/drivers/watchdog/Kconfig > > > +++ b/drivers/watchdog/Kconfig > > > @@ -1779,6 +1779,17 @@ config OCTEON_WDT > > > from the first interrupt, it is then only poked when the > > > device is written. > > > > > > +config MARVELL_GTI_WDT > > > + tristate "Marvell GTI Watchdog driver" > > > + depends on ARCH_THUNDER || COMPILE_TEST > > > > As 0-day reported, COMPILE_TEST does not work if there are asm > > includes in the code. > > Will remove asm include dependency in next version > > > > > > + help > > > + Marvell GTI hardware supports watchdog timer. First timeout > > > + works as watchdog pretimeout and installed interrupt handler > > > + will be called on first timeout. Hardware can generate interrupt > > > + to SCP on second timeout but it is not enabled, So second > > > + timeout is ignored. If device poke does not happen then system > > > + will reboot on third timeout. > > > + > > > config BCM2835_WDT > > > tristate "Broadcom BCM2835 hardware watchdog" > > > depends on ARCH_BCM2835 || (OF && COMPILE_TEST) diff --git > > > a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index > > > 9cbf6580f16c..bd425408fcaa 100644 > > > --- a/drivers/watchdog/Makefile > > > +++ b/drivers/watchdog/Makefile > > > @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += > visconti_wdt.o > > > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > > > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > > > obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o > > > +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o > > > > > > # X86 (i386 + ia64 + x86_64) Architecture > > > obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o diff --git > > > a/drivers/watchdog/marvell_gti_wdt.c > > > b/drivers/watchdog/marvell_gti_wdt.c > > > new file mode 100644 > > > index 000000000000..a3f0740dedca > > > --- /dev/null > > > +++ b/drivers/watchdog/marvell_gti_wdt.c > > > @@ -0,0 +1,271 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* Marvell GTI Watchdog driver > > > + * > > > + * Copyright (C) 2023 Marvell. > > > + */ > > > + > > > +#include <linux/module.h> > > > +#include <linux/interrupt.h> > > > +#include <linux/of_platform.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/watchdog.h> > > > + > > > +#include <asm/arch_timer.h> > > > + > > > +/* > > > + * Hardware supports following mode of operation: > > > + * 1) Interrupt Only: > > > + * This will generate the interrupt to arm core whenever timeout happens. > > > + * > > > + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). > > > + * This will generate interrupt to arm core on 1st timeout happens > > > + * This will generate interrupt to SCP processor on 2nd timeout happens > > > + * > > > + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. > > > + * This will generate interrupt to arm core on 1st timeout happens > > > + * Will generate interrupt to SCP processor on 2nd timeout happens, > > > + * if interrupt is configured. > > > + * Reboot on 3rd timeout. > > > + * > > > + * Driver will use hardware in mode-3 above so that system can > > > +reboot in case > > > + * a hardware hang. Also h/w is configured not to generate SCP > > > +interrupt, so > > > + * effectively 2nd timeout is ignored within hardware. > > > + */ > > > + > > > + > > > +/* GTI CWD Watchdog Registers */ > > > +#define GTI_CWD_GLOBAL_WDOG_IDX 63 > > > > It might make sense to explain "63". > > Okay, will add below comment > /* GTI hardware block supports total 64 timers and timer 63 is used for system > watchdog */ > > "IDX" seems confusing, will rename to "GTI_CWD_GLOBAL_WDOG_TIMER" After some more thought removed this hardcoding from code and added device tree property for which counter to be used. Sent next version with this change. Thanks -Bharat > > > > > > +#define GTI_CWD_WDOG (0x8 * > > GTI_CWD_GLOBAL_WDOG_IDX) > > > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 > > > +#define GTI_CWD_WDOG_MODE_MASK 0x3 > > > +#define GTI_CWD_WDOG_LEN_SHIFT 4 > > > +#define GTI_CWD_WDOG_CNT_SHIFT 20 > > > + > > > +/* GTI Per-core Watchdog Interrupt Register */ > > > +#define GTI_CWD_INT 0x200 > > > +#define GTI_CWD_INT_PENDING_STATUS (1ULL << > > GTI_CWD_GLOBAL_WDOG_IDX) > > > > Please use BIT_ULL(). > > Ok > > > > > > + > > > +/* GTI Per-core Watchdog Interrupt Enable Clear Register */ > > > +#define GTI_CWD_INT_ENA_CLR 0x210 > > > +#define GTI_CWD_INT_ENA_CLR_VAL (1ULL << > > GTI_CWD_GLOBAL_WDOG_IDX) > > > + > > > +/* GTI Per-core Watchdog Interrupt Enable Set Register */ > > > +#define GTI_CWD_INT_ENA_SET 0x218 > > > +#define GTI_CWD_INT_ENA_SET_VAL (1ULL << > > GTI_CWD_GLOBAL_WDOG_IDX) > > > + > > > +/* GTI Per-core Watchdog Poke Registers */ > > > +#define GTI_CWD_POKE (0x10000 + 0x8 * > > GTI_CWD_GLOBAL_WDOG_IDX) > > > > There should be a comment explaining the "8". > > GTI_CWD_POKE is poke register for watchdog counter > GTI_CWD_GLOBAL_WDOG_TIMER . > Above calculation is for getting the register address for watchdog timer > GTI_CWD_GLOBAL_WDOG_TIMER Can add comment > /* Each POKE register is 64bit and so GTI_CWD_POKE is register address for > watchdog counter GTI_CWD_GLOBAL_WDOG_TIMER */ > > > Also, there is a define for "0x8 * > > GTI_CWD_GLOBAL_WDOG_IDX" above. > > Why not use it here ? > > GTI_CWD_GLOBAL_WDOG_TIMER is used for register address calculation, so will > keep then separate to avoid any confusion. > > > > > > +#define GTI_CWD_POKE_VAL (1ULL << GTI_CWD_GLOBAL_WDOG_IDX) > > > > I don't think it makes sense to have three defines with the same value. > > Yes they are same value, but they are bit field in different h/w register. I want to > keep a separate name to avoid any confusion. > > > > > > + > > > +struct gti_wdt_priv { > > > + struct watchdog_device wdev; > > > + void __iomem *base; > > > + u64 clock_freq; > > > + int irq; > > > +}; > > > + > > > +static irqreturn_t gti_wdt_interrupt(int irq, void *data) { > > > + struct watchdog_device *wdev = data; > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + > > > + /* Clear Interrupt Pending Status */ > > > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > > > + > > > + watchdog_notify_pretimeout(wdev); > > > + > > > + return IRQ_HANDLED; > > > +} > > > + > > > +static int gti_wdt_ping(struct watchdog_device *wdev) { > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + > > > + writeq(GTI_CWD_POKE_VAL, priv->base + GTI_CWD_POKE); > > > + > > > + return 0; > > > +} > > > + > > > +static int gti_wdt_start(struct watchdog_device *wdev) { > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + u64 regval; > > > + > > > + if (!wdev->pretimeout) > > > + return -EINVAL; > > > + > > > + set_bit(WDOG_HW_RUNNING, &wdev->status); > > > + > > > + /* Clear any pending interrupt */ > > > + writeq(GTI_CWD_INT_PENDING_STATUS, priv->base + GTI_CWD_INT); > > > + > > > + /* Enable Interrupt */ > > > + writeq(GTI_CWD_INT_ENA_SET_VAL, priv->base + > > GTI_CWD_INT_ENA_SET); > > > + > > > + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ > > > + regval = readq(priv->base + GTI_CWD_WDOG); > > > + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; > > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > > + > > > + return 0; > > > +} > > > + > > > +static int gti_wdt_stop(struct watchdog_device *wdev) { > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + u64 regval; > > > + > > > + /* Disable Interrupt */ > > > + writeq(GTI_CWD_INT_ENA_CLR_VAL, priv->base + > > GTI_CWD_INT_ENA_CLR); > > > + > > > + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ > > > + regval = readq(priv->base + GTI_CWD_WDOG); > > > + regval &= ~GTI_CWD_WDOG_MODE_MASK; > > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > > + > > > + return 0; > > > +} > > > + > > > +static int gti_wdt_settimeout(struct watchdog_device *wdev, > > > + unsigned int timeout) > > > +{ > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + u64 timeout_wdog, regval; > > > + > > > + /* Update new timeout */ > > > + wdev->timeout = timeout; > > > + > > > + /* Pretimeout is 1/3 of timeout */ > > > + wdev->pretimeout = timeout / 3; > > > + if (!wdev->pretimeout) > > > + return -EINVAL; > > > > Since min_timeout is set to 3 seconds, this can not happen, and the > > check is unnecessary. > > > > > + > > > + /* Get clock cycles from timeout in second */ > > > > from pretimeout ? > > Yes, will change comment. > > > > > > + timeout_wdog = wdev->pretimeout * priv->clock_freq; > > > + > > > + /* Watchdog counts in 1024 cycle steps */ > > > + timeout_wdog = timeout_wdog >> 10; > > > + > > > + timeout_wdog = (timeout_wdog + 0xff) >> 8; > > > + if (timeout_wdog >= 0x10000) > > > + timeout_wdog = 0xffff; > > > + > > > + /* > > > + * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits > > > + * GTI_CWD_WDOG.CNT, need addition shift of 8. > > > + */ > > > + regval = readq(priv->base + GTI_CWD_WDOG); > > > + regval &= GTI_CWD_WDOG_MODE_MASK; > > > + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | > > > + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); > > > + writeq(regval, priv->base + GTI_CWD_WDOG); > > > + > > > + return 0; > > > +} > > > + > > > +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, > > > + unsigned int timeout) > > > +{ > > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > > + struct watchdog_device *wdog_dev = &priv->wdev; > > > + > > > + /* pretimeout should 1/3 of max_timeout */ > > > + if ((timeout * 3) <= wdog_dev->max_timeout) > > > > Unnecessary () > > > > > + return gti_wdt_settimeout(wdev, timeout * 3); > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static const struct watchdog_info gti_wdt_ident = { > > > + .identity = "Marvell GTI watchdog", > > > + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | > > WDIOF_KEEPALIVEPING | > > > + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, }; > > > + > > > +static const struct watchdog_ops gti_wdt_ops = { > > > + .owner = THIS_MODULE, > > > + .start = gti_wdt_start, > > > + .stop = gti_wdt_stop, > > > + .ping = gti_wdt_ping, > > > + .set_timeout = gti_wdt_settimeout, > > > + .set_pretimeout = gti_wdt_set_pretimeout, }; > > > + > > > +static int gti_wdt_probe(struct platform_device *pdev) { > > > + struct gti_wdt_priv *priv; > > > + struct device *dev = &pdev->dev; > > > + struct watchdog_device *wdog_dev; > > > + int irq; > > > + int err; > > > + > > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > > + if (!priv) > > > + return -ENOMEM; > > > + > > > + priv->base = devm_platform_ioremap_resource(pdev, 0); > > > + if (IS_ERR(priv->base)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), > > > + "reg property not valid/found\n"); > > > + > > > + priv->clock_freq = arch_timer_get_cntfrq(); > > > > arch_timer_get_cntfrq() returns u32. Storing it as u64 to save a > > couple of typecasts really doesn't add value. > > > > Note that this limits compilability to arm and arm64. > > Will remove using arch_timer_get_cntfrq() and clock reference in device tree. > > > > > > + > > > + wdog_dev = &priv->wdev; > > > + wdog_dev->info = >i_wdt_ident, > > > + wdog_dev->ops = >i_wdt_ops, > > > + wdog_dev->parent = dev; > > > + /* > > > + * Watchdog counter is 24 bit where lower 8 bits are zeros, so maximum > > > + * value that can be programmed in hardware is 0xffff00. This counter > > > + * decrements every one microsec. > > > + * Hardware will generate interrupt on first timeout (pretimeout) > > > + * reset the system on 3rd timeout. > > > + * Max watchdog pretimeout can be 16 sec, so max timeout is 48 sec. > > > + * Let's have min pretimeout to 1sec, so min timeout is 3sec. > > > + */ > > > + wdog_dev->max_timeout = 48; > > > > The code setting the actual timeout checks for overflow, suggesting > > that the actual maximum timeout depends on the clock speed and may be > > less than 48 seconds. If that ever happens, the actual timeout would > > be set to less than 48 seconds, which could cause random reboots. > > Will use clock for these calculation. > > Thanks > -Bharat > > > > > > + wdog_dev->min_timeout = 3; > > > + wdog_dev->timeout = 30; > > > + wdog_dev->pretimeout = wdog_dev->timeout / 3; > > > + > > > + priv->irq = irq; > > > + watchdog_set_drvdata(wdog_dev, priv); > > > + platform_set_drvdata(pdev, priv); > > > + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); > > > + watchdog_stop_on_reboot(wdog_dev); > > > + watchdog_stop_on_unregister(wdog_dev); > > > + > > > + err = devm_watchdog_register_device(dev, wdog_dev); > > > + if (err) > > > + return err; > > > + > > > + irq = platform_get_irq(pdev, 0); > > > + if (irq < 0) > > > + return dev_err_probe(&pdev->dev, irq, "IRQ resource not > > found\n"); > > > + > > > + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, > > > + pdev->name, &priv->wdev); > > > + if (err) > > > + return dev_err_probe(dev, err, "Failed to register interrupt > > > +handler\n"); > > > + > > > + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev- > > >timeout); > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id gti_wdt_of_match[] = { > > > + { .compatible = "marvell,gti-wdt", }, > > > + { }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); > > > + > > > +static struct platform_driver gti_wdt_driver = { > > > + .driver = { > > > + .name = "gti-wdt", > > > + .of_match_table = gti_wdt_of_match, > > > + }, > > > + .probe = gti_wdt_probe, > > > +}; > > > +module_platform_driver(gti_wdt_driver); > > > + > > > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); > > > +MODULE_DESCRIPTION("Marvell GTI watchdog driver"); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-03 12:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-27 7:14 [PATCH 1/2 v4] dt-bindings: watchdog: marvell GTI system watchdog driver Bharat Bhushan 2023-04-27 7:14 ` [PATCH 2/2 v4] Watchdog: Add marvell GTI " Bharat Bhushan 2023-04-27 21:29 ` kernel test robot 2023-04-29 15:14 ` Guenter Roeck 2023-05-02 17:05 ` [EXT] " Bharat Bhushan 2023-05-03 12:13 ` Bharat Bhushan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).