| Message ID | 20191118220432.1611-1-labbott@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | watchdog: Remove iop_wdt | expand |
On 11/18/19 2:04 PM, Laura Abbott wrote: > > Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") > removed support for some old platforms. Given this driver depends on > a now removed platform, just remove the driver. > > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > Found this while reviewing config options. Not sure if this was kept > around for other reasons or just missed. > --- > drivers/watchdog/Kconfig | 16 --- > drivers/watchdog/Makefile | 1 - > drivers/watchdog/iop_wdt.c | 249 ------------------------------------- > 3 files changed, 266 deletions(-) > delete mode 100644 drivers/watchdog/iop_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 58e7c100b6ad..fef9078a44b6 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG > > Say N if you are unsure. > > -config IOP_WATCHDOG > - tristate "IOP Watchdog" > - depends on ARCH_IOP13XX > - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) This is a bit confusing, but it suggests that the watchdog may also work with ARCH_IOP32X, which is still supported. I don't know anything about those architectures, but I hesitate to have the driver removed unless we have confirmation that it won't work with ARCH_IOP32X. Maybe the dependency needs to be updated instead ? Thanks, Guenter > - help > - Say Y here if to include support for the watchdog timer > - in the Intel IOP3XX & IOP13XX I/O Processors. This driver can > - be built as a module by choosing M. The module will > - be called iop_wdt. > - > - Note: The IOP13XX watchdog does an Internal Bus Reset which will > - affect both cores and the peripherals of the IOP. The ATU-X > - and/or ATUe configuration registers will remain intact, but if > - operating as an Root Complex and/or Central Resource, the PCI-X > - and/or PCIe busses will also be reset. THIS IS A VERY BIG HAMMER. > - > config DAVINCI_WATCHDOG > tristate "DaVinci watchdog" > depends on ARCH_DAVINCI || ARCH_KEYSTONE || COMPILE_TEST > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 2ee352bf3372..9de21f5ce909 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -55,7 +55,6 @@ obj-$(CONFIG_SAMA5D4_WATCHDOG) += sama5d4_wdt.o > obj-$(CONFIG_DW_WATCHDOG) += dw_wdt.o > obj-$(CONFIG_EP93XX_WATCHDOG) += ep93xx_wdt.o > obj-$(CONFIG_PNX4008_WATCHDOG) += pnx4008_wdt.o > -obj-$(CONFIG_IOP_WATCHDOG) += iop_wdt.o > obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o > obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o > obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o > diff --git a/drivers/watchdog/iop_wdt.c b/drivers/watchdog/iop_wdt.c > deleted file mode 100644 > index a9ccdb9a9159..000000000000 > --- a/drivers/watchdog/iop_wdt.c > +++ /dev/null > @@ -1,249 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * drivers/char/watchdog/iop_wdt.c > - * > - * WDT driver for Intel I/O Processors > - * Copyright (C) 2005, Intel Corporation. > - * > - * Based on ixp4xx driver, Copyright 2004 (c) MontaVista, Software, Inc. > - * > - * Curt E Bruns <curt.e.bruns@intel.com> > - * Peter Milne <peter.milne@d-tacq.com> > - * Dan Williams <dan.j.williams@intel.com> > - */ > - > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > - > -#include <linux/module.h> > -#include <linux/kernel.h> > -#include <linux/fs.h> > -#include <linux/init.h> > -#include <linux/device.h> > -#include <linux/miscdevice.h> > -#include <linux/watchdog.h> > -#include <linux/uaccess.h> > -#include <mach/hardware.h> > - > -static bool nowayout = WATCHDOG_NOWAYOUT; > -static unsigned long wdt_status; > -static unsigned long boot_status; > -static DEFINE_SPINLOCK(wdt_lock); > - > -#define WDT_IN_USE 0 > -#define WDT_OK_TO_CLOSE 1 > -#define WDT_ENABLED 2 > - > -static unsigned long iop_watchdog_timeout(void) > -{ > - return (0xffffffffUL / get_iop_tick_rate()); > -} > - > -/** > - * wdt_supports_disable - determine if we are accessing a iop13xx watchdog > - * or iop3xx by whether it has a disable command > - */ > -static int wdt_supports_disable(void) > -{ > - int can_disable; > - > - if (IOP_WDTCR_EN_ARM != IOP_WDTCR_DIS_ARM) > - can_disable = 1; > - else > - can_disable = 0; > - > - return can_disable; > -} > - > -static void wdt_enable(void) > -{ > - /* Arm and enable the Timer to starting counting down from 0xFFFF.FFFF > - * Takes approx. 10.7s to timeout > - */ > - spin_lock(&wdt_lock); > - write_wdtcr(IOP_WDTCR_EN_ARM); > - write_wdtcr(IOP_WDTCR_EN); > - spin_unlock(&wdt_lock); > -} > - > -/* returns 0 if the timer was successfully disabled */ > -static int wdt_disable(void) > -{ > - /* Stop Counting */ > - if (wdt_supports_disable()) { > - spin_lock(&wdt_lock); > - write_wdtcr(IOP_WDTCR_DIS_ARM); > - write_wdtcr(IOP_WDTCR_DIS); > - clear_bit(WDT_ENABLED, &wdt_status); > - spin_unlock(&wdt_lock); > - pr_info("Disabled\n"); > - return 0; > - } else > - return 1; > -} > - > -static int iop_wdt_open(struct inode *inode, struct file *file) > -{ > - if (test_and_set_bit(WDT_IN_USE, &wdt_status)) > - return -EBUSY; > - > - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); > - wdt_enable(); > - set_bit(WDT_ENABLED, &wdt_status); > - return stream_open(inode, file); > -} > - > -static ssize_t iop_wdt_write(struct file *file, const char *data, size_t len, > - loff_t *ppos) > -{ > - if (len) { > - if (!nowayout) { > - size_t i; > - > - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); > - > - for (i = 0; i != len; i++) { > - char c; > - > - if (get_user(c, data + i)) > - return -EFAULT; > - if (c == 'V') > - set_bit(WDT_OK_TO_CLOSE, &wdt_status); > - } > - } > - wdt_enable(); > - } > - return len; > -} > - > -static const struct watchdog_info ident = { > - .options = WDIOF_CARDRESET | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, > - .identity = "iop watchdog", > -}; > - > -static long iop_wdt_ioctl(struct file *file, > - unsigned int cmd, unsigned long arg) > -{ > - int options; > - int ret = -ENOTTY; > - int __user *argp = (int __user *)arg; > - > - switch (cmd) { > - case WDIOC_GETSUPPORT: > - if (copy_to_user(argp, &ident, sizeof(ident))) > - ret = -EFAULT; > - else > - ret = 0; > - break; > - > - case WDIOC_GETSTATUS: > - ret = put_user(0, argp); > - break; > - > - case WDIOC_GETBOOTSTATUS: > - ret = put_user(boot_status, argp); > - break; > - > - case WDIOC_SETOPTIONS: > - if (get_user(options, (int *)arg)) > - return -EFAULT; > - > - if (options & WDIOS_DISABLECARD) { > - if (!nowayout) { > - if (wdt_disable() == 0) { > - set_bit(WDT_OK_TO_CLOSE, &wdt_status); > - ret = 0; > - } else > - ret = -ENXIO; > - } else > - ret = 0; > - } > - if (options & WDIOS_ENABLECARD) { > - wdt_enable(); > - ret = 0; > - } > - break; > - > - case WDIOC_KEEPALIVE: > - wdt_enable(); > - ret = 0; > - break; > - > - case WDIOC_GETTIMEOUT: > - ret = put_user(iop_watchdog_timeout(), argp); > - break; > - } > - return ret; > -} > - > -static int iop_wdt_release(struct inode *inode, struct file *file) > -{ > - int state = 1; > - if (test_bit(WDT_OK_TO_CLOSE, &wdt_status)) > - if (test_bit(WDT_ENABLED, &wdt_status)) > - state = wdt_disable(); > - > - /* if the timer is not disabled reload and notify that we are still > - * going down > - */ > - if (state != 0) { > - wdt_enable(); > - pr_crit("Device closed unexpectedly - reset in %lu seconds\n", > - iop_watchdog_timeout()); > - } > - > - clear_bit(WDT_IN_USE, &wdt_status); > - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); > - > - return 0; > -} > - > -static const struct file_operations iop_wdt_fops = { > - .owner = THIS_MODULE, > - .llseek = no_llseek, > - .write = iop_wdt_write, > - .unlocked_ioctl = iop_wdt_ioctl, > - .open = iop_wdt_open, > - .release = iop_wdt_release, > -}; > - > -static struct miscdevice iop_wdt_miscdev = { > - .minor = WATCHDOG_MINOR, > - .name = "watchdog", > - .fops = &iop_wdt_fops, > -}; > - > -static int __init iop_wdt_init(void) > -{ > - int ret; > - > - /* check if the reset was caused by the watchdog timer */ > - boot_status = (read_rcsr() & IOP_RCSR_WDT) ? WDIOF_CARDRESET : 0; > - > - /* Configure Watchdog Timeout to cause an Internal Bus (IB) Reset > - * NOTE: An IB Reset will Reset both cores in the IOP342 > - */ > - write_wdtsr(IOP13XX_WDTCR_IB_RESET); > - > - /* Register after we have the device set up so we cannot race > - with an open */ > - ret = misc_register(&iop_wdt_miscdev); > - if (ret == 0) > - pr_info("timeout %lu sec\n", iop_watchdog_timeout()); > - > - return ret; > -} > - > -static void __exit iop_wdt_exit(void) > -{ > - misc_deregister(&iop_wdt_miscdev); > -} > - > -module_init(iop_wdt_init); > -module_exit(iop_wdt_exit); > - > -module_param(nowayout, bool, 0); > -MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started"); > - > -MODULE_AUTHOR("Curt E Bruns <curt.e.bruns@intel.com>"); > -MODULE_DESCRIPTION("iop watchdog timer driver"); > -MODULE_LICENSE("GPL"); >
On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") > > removed support for some old platforms. Given this driver depends on > > a now removed platform, just remove the driver. > > > > Signed-off-by: Laura Abbott <labbott@redhat.com> > > --- > > Found this while reviewing config options. Not sure if this was kept > > around for other reasons or just missed. > > --- > > drivers/watchdog/Kconfig | 16 --- > > drivers/watchdog/Makefile | 1 - > > drivers/watchdog/iop_wdt.c | 249 ------------------------------------- > > 3 files changed, 266 deletions(-) > > delete mode 100644 drivers/watchdog/iop_wdt.c > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 58e7c100b6ad..fef9078a44b6 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG > > > > Say N if you are unsure. > > > > -config IOP_WATCHDOG > > - tristate "IOP Watchdog" > > - depends on ARCH_IOP13XX > > - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) > > This is a bit confusing, but it suggests that the watchdog may also work > with ARCH_IOP32X, which is still supported. I don't know anything about > those architectures, but I hesitate to have the driver removed unless > we have confirmation that it won't work with ARCH_IOP32X. > Maybe the dependency needs to be updated instead ? See commit ec2e32ca661e ("watchdog: iop_wdt only builds for mach-iop13xx") from 2014: the watdog hardware exists on iop32x but the driver only successfully built on iop13xx, which is now gone. Adding Russell (who said he still uses iop32x hardware) and Lennert (who is still listed in the MAINTAINERS file, but previously said he does not use it any more) to Cc. If neither of them see a reason to keep the driver, I'd say we can remove it. It seems unlikely that anyone wants to revive the driver if they have not done it yet, and if they want to do it later, it is barely harder to revert the removal than to fix the compile-time problem. Arnd
On 11/19/19 1:40 AM, Arnd Bergmann wrote: > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 11/18/19 2:04 PM, Laura Abbott wrote: >>> >>> Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") >>> removed support for some old platforms. Given this driver depends on >>> a now removed platform, just remove the driver. >>> >>> Signed-off-by: Laura Abbott <labbott@redhat.com> >>> --- >>> Found this while reviewing config options. Not sure if this was kept >>> around for other reasons or just missed. >>> --- >>> drivers/watchdog/Kconfig | 16 --- >>> drivers/watchdog/Makefile | 1 - >>> drivers/watchdog/iop_wdt.c | 249 ------------------------------------- >>> 3 files changed, 266 deletions(-) >>> delete mode 100644 drivers/watchdog/iop_wdt.c >>> >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>> index 58e7c100b6ad..fef9078a44b6 100644 >>> --- a/drivers/watchdog/Kconfig >>> +++ b/drivers/watchdog/Kconfig >>> @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG >>> >>> Say N if you are unsure. >>> >>> -config IOP_WATCHDOG >>> - tristate "IOP Watchdog" >>> - depends on ARCH_IOP13XX >>> - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) >> >> This is a bit confusing, but it suggests that the watchdog may also work >> with ARCH_IOP32X, which is still supported. I don't know anything about >> those architectures, but I hesitate to have the driver removed unless >> we have confirmation that it won't work with ARCH_IOP32X. >> Maybe the dependency needs to be updated instead ? > > See commit ec2e32ca661e ("watchdog: iop_wdt only builds for > mach-iop13xx") from 2014: the watdog hardware exists on iop32x > but the driver only successfully built on iop13xx, which is now gone. > > Adding Russell (who said he still uses iop32x hardware) and Lennert > (who is still listed in the MAINTAINERS file, but previously said he > does not use it any more) to Cc. If neither of them see a reason to > keep the driver, I'd say we can remove it. > > It seems unlikely that anyone wants to revive the driver if they have > not done it yet, and if they want to do it later, it is barely harder to revert > the removal than to fix the compile-time problem. > Good point, especially since apparently no one cared for five years. Guenter
On Tue, Nov 19, 2019 at 10:40:04AM +0100, Arnd Bergmann wrote: > > > Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") > > > removed support for some old platforms. Given this driver depends on > > > a now removed platform, just remove the driver. > > > > > > Signed-off-by: Laura Abbott <labbott@redhat.com> > > > --- > > > Found this while reviewing config options. Not sure if this was kept > > > around for other reasons or just missed. > > > --- > > > drivers/watchdog/Kconfig | 16 --- > > > drivers/watchdog/Makefile | 1 - > > > drivers/watchdog/iop_wdt.c | 249 ------------------------------------- > > > 3 files changed, 266 deletions(-) > > > delete mode 100644 drivers/watchdog/iop_wdt.c > > > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > > index 58e7c100b6ad..fef9078a44b6 100644 > > > --- a/drivers/watchdog/Kconfig > > > +++ b/drivers/watchdog/Kconfig > > > @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG > > > > > > Say N if you are unsure. > > > > > > -config IOP_WATCHDOG > > > - tristate "IOP Watchdog" > > > - depends on ARCH_IOP13XX > > > - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) > > > > This is a bit confusing, but it suggests that the watchdog may also work > > with ARCH_IOP32X, which is still supported. I don't know anything about > > those architectures, but I hesitate to have the driver removed unless > > we have confirmation that it won't work with ARCH_IOP32X. > > Maybe the dependency needs to be updated instead ? > > See commit ec2e32ca661e ("watchdog: iop_wdt only builds for > mach-iop13xx") from 2014: the watdog hardware exists on iop32x > but the driver only successfully built on iop13xx, which is now gone. > > Adding Russell (who said he still uses iop32x hardware) and Lennert > (who is still listed in the MAINTAINERS file, but previously said he > does not use it any more) to Cc. I haven't worked on ARM-related things for a long time now. I'll be happy to hand over maintainership to someone else, or to orphan the platform(s) entirely. > If neither of them see a reason to keep the driver, I'd say we can > remove it. I don't see a reason (to keep it). Cheers, Lennert
On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > > > > > Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") > > > > removed support for some old platforms. Given this driver depends on > > > > a now removed platform, just remove the driver. > > > > > > > > Signed-off-by: Laura Abbott <labbott@redhat.com> > > > > --- > > > > Found this while reviewing config options. Not sure if this was kept > > > > around for other reasons or just missed. > > > > --- > > > > drivers/watchdog/Kconfig | 16 --- > > > > drivers/watchdog/Makefile | 1 - > > > > drivers/watchdog/iop_wdt.c | 249 ------------------------------------- > > > > 3 files changed, 266 deletions(-) > > > > delete mode 100644 drivers/watchdog/iop_wdt.c > > > > > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > > > index 58e7c100b6ad..fef9078a44b6 100644 > > > > --- a/drivers/watchdog/Kconfig > > > > +++ b/drivers/watchdog/Kconfig > > > > @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG > > > > > > > > Say N if you are unsure. > > > > > > > > -config IOP_WATCHDOG > > > > - tristate "IOP Watchdog" > > > > - depends on ARCH_IOP13XX > > > > - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) > > > > > > This is a bit confusing, but it suggests that the watchdog may also work > > > with ARCH_IOP32X, which is still supported. I don't know anything about > > > those architectures, but I hesitate to have the driver removed unless > > > we have confirmation that it won't work with ARCH_IOP32X. > > > Maybe the dependency needs to be updated instead ? > > > > See commit ec2e32ca661e ("watchdog: iop_wdt only builds for > > mach-iop13xx") from 2014: the watdog hardware exists on iop32x > > but the driver only successfully built on iop13xx, which is now gone. > > > > Adding Russell (who said he still uses iop32x hardware) and Lennert > > (who is still listed in the MAINTAINERS file, but previously said he > > does not use it any more) to Cc. If neither of them see a reason to > > keep the driver, I'd say we can remove it. > > > > It seems unlikely that anyone wants to revive the driver if they have > > not done it yet, and if they want to do it later, it is barely harder to revert > > the removal than to fix the compile-time problem. > > > > Good point, especially since apparently no one cared for five years. Doesn't mean that there aren't interested parties. I still have IOP32x hardware running here in the form of a N2100 (my firewall) and it seems that I never noticed this option disappearing until now...
On Wed, Nov 20, 2019 at 11:03 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > Good point, especially since apparently no one cared for five years. > > Doesn't mean that there aren't interested parties. I still have > IOP32x hardware running here in the form of a N2100 (my firewall) > and it seems that I never noticed this option disappearing until > now... It's not that it was ever there for IOP32x: the driver was introduced in 2007 and was available for IOP32x but failed to compile for it until 2014 when I sent the patch to disable the driver in all configurations that failed to build. Arnd
On Wed, Nov 20, 2019 at 11:15:01AM +0100, Arnd Bergmann wrote: > On Wed, Nov 20, 2019 at 11:03 AM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > > > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > > > Good point, especially since apparently no one cared for five years. > > > > Doesn't mean that there aren't interested parties. I still have > > IOP32x hardware running here in the form of a N2100 (my firewall) > > and it seems that I never noticed this option disappearing until > > now... > > It's not that it was ever there for IOP32x: the driver was introduced in 2007 > and was available for IOP32x but failed to compile for it until 2014 when > I sent the patch to disable the driver in all configurations that > failed to build. Well: systems/n2100/boot/config-3.11.5+:CONFIG_IOP_WATCHDOG=m systems/n2100/boot/config-3.12.6+:CONFIG_IOP_WATCHDOG=m systems/n2100/boot/config-3.9.5+:CONFIG_IOP_WATCHDOG=m -rw-rw-r-- 1 rmk rmk 5284 Dec 30 2013 systems/n2100/lib/modules/3.12.6+/kernel/drivers/watchdog/iop_wdt.ko -rw-rw-r-- 1 rmk rmk 5276 Dec 20 2013 systems/n2100/lib/modules/3.9.5+/kernel/drivers/watchdog/iop_wdt.ko It seems I've been carrying a patch to comment out the troublesome code: - write_wdtsr(IOP13XX_WDTCR_IB_RESET); +// write_wdtsr(IOP13XX_WDTCR_IB_RESET); in my stable tree since 2015.
On Wed, Nov 20, 2019 at 10:30:54AM +0000, Russell King - ARM Linux admin wrote: > On Wed, Nov 20, 2019 at 11:15:01AM +0100, Arnd Bergmann wrote: > > On Wed, Nov 20, 2019 at 11:03 AM Russell King - ARM Linux admin > > <linux@armlinux.org.uk> wrote: > > > On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > > > > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > > > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > > > > > Good point, especially since apparently no one cared for five years. > > > > > > Doesn't mean that there aren't interested parties. I still have > > > IOP32x hardware running here in the form of a N2100 (my firewall) > > > and it seems that I never noticed this option disappearing until > > > now... > > > > It's not that it was ever there for IOP32x: the driver was introduced in 2007 > > and was available for IOP32x but failed to compile for it until 2014 when > > I sent the patch to disable the driver in all configurations that > > failed to build. > > Well: > > systems/n2100/boot/config-3.11.5+:CONFIG_IOP_WATCHDOG=m > systems/n2100/boot/config-3.12.6+:CONFIG_IOP_WATCHDOG=m > systems/n2100/boot/config-3.9.5+:CONFIG_IOP_WATCHDOG=m > > -rw-rw-r-- 1 rmk rmk 5284 Dec 30 2013 systems/n2100/lib/modules/3.12.6+/kernel/drivers/watchdog/iop_wdt.ko > -rw-rw-r-- 1 rmk rmk 5276 Dec 20 2013 systems/n2100/lib/modules/3.9.5+/kernel/drivers/watchdog/iop_wdt.ko > > It seems I've been carrying a patch to comment out the troublesome code: > > - write_wdtsr(IOP13XX_WDTCR_IB_RESET); > +// write_wdtsr(IOP13XX_WDTCR_IB_RESET); > > in my stable tree since 2015. Do you have plans to update that kernel to mainline ? If yes, a patch to make the driver (and I guess everything else that broke since 3.12) work would be helpful. Thanks, Guenter
On Wed, Nov 20, 2019 at 03:05:18PM -0800, Guenter Roeck wrote: > On Wed, Nov 20, 2019 at 10:30:54AM +0000, Russell King - ARM Linux admin wrote: > > On Wed, Nov 20, 2019 at 11:15:01AM +0100, Arnd Bergmann wrote: > > > On Wed, Nov 20, 2019 at 11:03 AM Russell King - ARM Linux admin > > > <linux@armlinux.org.uk> wrote: > > > > On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > > > > > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > > > > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > > > > > > > Good point, especially since apparently no one cared for five years. > > > > > > > > Doesn't mean that there aren't interested parties. I still have > > > > IOP32x hardware running here in the form of a N2100 (my firewall) > > > > and it seems that I never noticed this option disappearing until > > > > now... > > > > > > It's not that it was ever there for IOP32x: the driver was introduced in 2007 > > > and was available for IOP32x but failed to compile for it until 2014 when > > > I sent the patch to disable the driver in all configurations that > > > failed to build. > > > > Well: > > > > systems/n2100/boot/config-3.11.5+:CONFIG_IOP_WATCHDOG=m > > systems/n2100/boot/config-3.12.6+:CONFIG_IOP_WATCHDOG=m > > systems/n2100/boot/config-3.9.5+:CONFIG_IOP_WATCHDOG=m > > > > -rw-rw-r-- 1 rmk rmk 5284 Dec 30 2013 systems/n2100/lib/modules/3.12.6+/kernel/drivers/watchdog/iop_wdt.ko > > -rw-rw-r-- 1 rmk rmk 5276 Dec 20 2013 systems/n2100/lib/modules/3.9.5+/kernel/drivers/watchdog/iop_wdt.ko > > > > It seems I've been carrying a patch to comment out the troublesome code: > > > > - write_wdtsr(IOP13XX_WDTCR_IB_RESET); > > +// write_wdtsr(IOP13XX_WDTCR_IB_RESET); > > > > in my stable tree since 2015. > > Do you have plans to update that kernel to mainline ? > If yes, a patch to make the driver (and I guess everything else that broke > since 3.12) work would be helpful. It's a currently running 4.19.xx stable kernel, but as a result of the patch to the Kconfig file, without the watchdog which I hadn't realised until I saw this thread. Is that mainline enough?
On Wed, Nov 20, 2019 at 11:56:52PM +0000, Russell King - ARM Linux admin wrote: > On Wed, Nov 20, 2019 at 03:05:18PM -0800, Guenter Roeck wrote: > > On Wed, Nov 20, 2019 at 10:30:54AM +0000, Russell King - ARM Linux admin wrote: > > > On Wed, Nov 20, 2019 at 11:15:01AM +0100, Arnd Bergmann wrote: > > > > On Wed, Nov 20, 2019 at 11:03 AM Russell King - ARM Linux admin > > > > <linux@armlinux.org.uk> wrote: > > > > > On Tue, Nov 19, 2019 at 06:29:09AM -0800, Guenter Roeck wrote: > > > > > > On 11/19/19 1:40 AM, Arnd Bergmann wrote: > > > > > > > On Tue, Nov 19, 2019 at 3:08 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > > > On 11/18/19 2:04 PM, Laura Abbott wrote: > > > > > > > > > > > > Good point, especially since apparently no one cared for five years. > > > > > > > > > > Doesn't mean that there aren't interested parties. I still have > > > > > IOP32x hardware running here in the form of a N2100 (my firewall) > > > > > and it seems that I never noticed this option disappearing until > > > > > now... > > > > > > > > It's not that it was ever there for IOP32x: the driver was introduced in 2007 > > > > and was available for IOP32x but failed to compile for it until 2014 when > > > > I sent the patch to disable the driver in all configurations that > > > > failed to build. > > > > > > Well: > > > > > > systems/n2100/boot/config-3.11.5+:CONFIG_IOP_WATCHDOG=m > > > systems/n2100/boot/config-3.12.6+:CONFIG_IOP_WATCHDOG=m > > > systems/n2100/boot/config-3.9.5+:CONFIG_IOP_WATCHDOG=m > > > > > > -rw-rw-r-- 1 rmk rmk 5284 Dec 30 2013 systems/n2100/lib/modules/3.12.6+/kernel/drivers/watchdog/iop_wdt.ko > > > -rw-rw-r-- 1 rmk rmk 5276 Dec 20 2013 systems/n2100/lib/modules/3.9.5+/kernel/drivers/watchdog/iop_wdt.ko > > > > > > It seems I've been carrying a patch to comment out the troublesome code: > > > > > > - write_wdtsr(IOP13XX_WDTCR_IB_RESET); > > > +// write_wdtsr(IOP13XX_WDTCR_IB_RESET); > > > > > > in my stable tree since 2015. > > > > Do you have plans to update that kernel to mainline ? > > If yes, a patch to make the driver (and I guess everything else that broke > > since 3.12) work would be helpful. > > It's a currently running 4.19.xx stable kernel, but as a result of the > patch to the Kconfig file, without the watchdog which I hadn't realised > until I saw this thread. Is that mainline enough? > Yes. Can you send me a patch with the above fix and the necessary Kconfig change ? Thanks, Guenter
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 58e7c100b6ad..fef9078a44b6 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -554,22 +554,6 @@ config PNX4008_WATCHDOG Say N if you are unsure. -config IOP_WATCHDOG - tristate "IOP Watchdog" - depends on ARCH_IOP13XX - select WATCHDOG_NOWAYOUT if (ARCH_IOP32X || ARCH_IOP33X) - help - Say Y here if to include support for the watchdog timer - in the Intel IOP3XX & IOP13XX I/O Processors. This driver can - be built as a module by choosing M. The module will - be called iop_wdt. - - Note: The IOP13XX watchdog does an Internal Bus Reset which will - affect both cores and the peripherals of the IOP. The ATU-X - and/or ATUe configuration registers will remain intact, but if - operating as an Root Complex and/or Central Resource, the PCI-X - and/or PCIe busses will also be reset. THIS IS A VERY BIG HAMMER. - config DAVINCI_WATCHDOG tristate "DaVinci watchdog" depends on ARCH_DAVINCI || ARCH_KEYSTONE || COMPILE_TEST diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 2ee352bf3372..9de21f5ce909 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -55,7 +55,6 @@ obj-$(CONFIG_SAMA5D4_WATCHDOG) += sama5d4_wdt.o obj-$(CONFIG_DW_WATCHDOG) += dw_wdt.o obj-$(CONFIG_EP93XX_WATCHDOG) += ep93xx_wdt.o obj-$(CONFIG_PNX4008_WATCHDOG) += pnx4008_wdt.o -obj-$(CONFIG_IOP_WATCHDOG) += iop_wdt.o obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o diff --git a/drivers/watchdog/iop_wdt.c b/drivers/watchdog/iop_wdt.c deleted file mode 100644 index a9ccdb9a9159..000000000000 --- a/drivers/watchdog/iop_wdt.c +++ /dev/null @@ -1,249 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * drivers/char/watchdog/iop_wdt.c - * - * WDT driver for Intel I/O Processors - * Copyright (C) 2005, Intel Corporation. - * - * Based on ixp4xx driver, Copyright 2004 (c) MontaVista, Software, Inc. - * - * Curt E Bruns <curt.e.bruns@intel.com> - * Peter Milne <peter.milne@d-tacq.com> - * Dan Williams <dan.j.williams@intel.com> - */ - -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - -#include <linux/module.h> -#include <linux/kernel.h> -#include <linux/fs.h> -#include <linux/init.h> -#include <linux/device.h> -#include <linux/miscdevice.h> -#include <linux/watchdog.h> -#include <linux/uaccess.h> -#include <mach/hardware.h> - -static bool nowayout = WATCHDOG_NOWAYOUT; -static unsigned long wdt_status; -static unsigned long boot_status; -static DEFINE_SPINLOCK(wdt_lock); - -#define WDT_IN_USE 0 -#define WDT_OK_TO_CLOSE 1 -#define WDT_ENABLED 2 - -static unsigned long iop_watchdog_timeout(void) -{ - return (0xffffffffUL / get_iop_tick_rate()); -} - -/** - * wdt_supports_disable - determine if we are accessing a iop13xx watchdog - * or iop3xx by whether it has a disable command - */ -static int wdt_supports_disable(void) -{ - int can_disable; - - if (IOP_WDTCR_EN_ARM != IOP_WDTCR_DIS_ARM) - can_disable = 1; - else - can_disable = 0; - - return can_disable; -} - -static void wdt_enable(void) -{ - /* Arm and enable the Timer to starting counting down from 0xFFFF.FFFF - * Takes approx. 10.7s to timeout - */ - spin_lock(&wdt_lock); - write_wdtcr(IOP_WDTCR_EN_ARM); - write_wdtcr(IOP_WDTCR_EN); - spin_unlock(&wdt_lock); -} - -/* returns 0 if the timer was successfully disabled */ -static int wdt_disable(void) -{ - /* Stop Counting */ - if (wdt_supports_disable()) { - spin_lock(&wdt_lock); - write_wdtcr(IOP_WDTCR_DIS_ARM); - write_wdtcr(IOP_WDTCR_DIS); - clear_bit(WDT_ENABLED, &wdt_status); - spin_unlock(&wdt_lock); - pr_info("Disabled\n"); - return 0; - } else - return 1; -} - -static int iop_wdt_open(struct inode *inode, struct file *file) -{ - if (test_and_set_bit(WDT_IN_USE, &wdt_status)) - return -EBUSY; - - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); - wdt_enable(); - set_bit(WDT_ENABLED, &wdt_status); - return stream_open(inode, file); -} - -static ssize_t iop_wdt_write(struct file *file, const char *data, size_t len, - loff_t *ppos) -{ - if (len) { - if (!nowayout) { - size_t i; - - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); - - for (i = 0; i != len; i++) { - char c; - - if (get_user(c, data + i)) - return -EFAULT; - if (c == 'V') - set_bit(WDT_OK_TO_CLOSE, &wdt_status); - } - } - wdt_enable(); - } - return len; -} - -static const struct watchdog_info ident = { - .options = WDIOF_CARDRESET | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, - .identity = "iop watchdog", -}; - -static long iop_wdt_ioctl(struct file *file, - unsigned int cmd, unsigned long arg) -{ - int options; - int ret = -ENOTTY; - int __user *argp = (int __user *)arg; - - switch (cmd) { - case WDIOC_GETSUPPORT: - if (copy_to_user(argp, &ident, sizeof(ident))) - ret = -EFAULT; - else - ret = 0; - break; - - case WDIOC_GETSTATUS: - ret = put_user(0, argp); - break; - - case WDIOC_GETBOOTSTATUS: - ret = put_user(boot_status, argp); - break; - - case WDIOC_SETOPTIONS: - if (get_user(options, (int *)arg)) - return -EFAULT; - - if (options & WDIOS_DISABLECARD) { - if (!nowayout) { - if (wdt_disable() == 0) { - set_bit(WDT_OK_TO_CLOSE, &wdt_status); - ret = 0; - } else - ret = -ENXIO; - } else - ret = 0; - } - if (options & WDIOS_ENABLECARD) { - wdt_enable(); - ret = 0; - } - break; - - case WDIOC_KEEPALIVE: - wdt_enable(); - ret = 0; - break; - - case WDIOC_GETTIMEOUT: - ret = put_user(iop_watchdog_timeout(), argp); - break; - } - return ret; -} - -static int iop_wdt_release(struct inode *inode, struct file *file) -{ - int state = 1; - if (test_bit(WDT_OK_TO_CLOSE, &wdt_status)) - if (test_bit(WDT_ENABLED, &wdt_status)) - state = wdt_disable(); - - /* if the timer is not disabled reload and notify that we are still - * going down - */ - if (state != 0) { - wdt_enable(); - pr_crit("Device closed unexpectedly - reset in %lu seconds\n", - iop_watchdog_timeout()); - } - - clear_bit(WDT_IN_USE, &wdt_status); - clear_bit(WDT_OK_TO_CLOSE, &wdt_status); - - return 0; -} - -static const struct file_operations iop_wdt_fops = { - .owner = THIS_MODULE, - .llseek = no_llseek, - .write = iop_wdt_write, - .unlocked_ioctl = iop_wdt_ioctl, - .open = iop_wdt_open, - .release = iop_wdt_release, -}; - -static struct miscdevice iop_wdt_miscdev = { - .minor = WATCHDOG_MINOR, - .name = "watchdog", - .fops = &iop_wdt_fops, -}; - -static int __init iop_wdt_init(void) -{ - int ret; - - /* check if the reset was caused by the watchdog timer */ - boot_status = (read_rcsr() & IOP_RCSR_WDT) ? WDIOF_CARDRESET : 0; - - /* Configure Watchdog Timeout to cause an Internal Bus (IB) Reset - * NOTE: An IB Reset will Reset both cores in the IOP342 - */ - write_wdtsr(IOP13XX_WDTCR_IB_RESET); - - /* Register after we have the device set up so we cannot race - with an open */ - ret = misc_register(&iop_wdt_miscdev); - if (ret == 0) - pr_info("timeout %lu sec\n", iop_watchdog_timeout()); - - return ret; -} - -static void __exit iop_wdt_exit(void) -{ - misc_deregister(&iop_wdt_miscdev); -} - -module_init(iop_wdt_init); -module_exit(iop_wdt_exit); - -module_param(nowayout, bool, 0); -MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started"); - -MODULE_AUTHOR("Curt E Bruns <curt.e.bruns@intel.com>"); -MODULE_DESCRIPTION("iop watchdog timer driver"); -MODULE_LICENSE("GPL");
Commit 59d3ae9a5bf6 ("ARM: remove Intel iop33x and iop13xx support") removed support for some old platforms. Given this driver depends on a now removed platform, just remove the driver. Signed-off-by: Laura Abbott <labbott@redhat.com> --- Found this while reviewing config options. Not sure if this was kept around for other reasons or just missed. --- drivers/watchdog/Kconfig | 16 --- drivers/watchdog/Makefile | 1 - drivers/watchdog/iop_wdt.c | 249 ------------------------------------- 3 files changed, 266 deletions(-) delete mode 100644 drivers/watchdog/iop_wdt.c