2019-07-30 03:12:22

by Guenter Roeck

[permalink] [raw]
Subject: Marking legacy watchdog drivers as deprecated / obsolete

Hi,

we have recently seen a number of changes to legacy watchdog drivers,
mostly surrounding the coding style used some 10+ years ago, but also
fixing minor formatting or coding problems found by static analyzers.
This slowly rises above the level of background noise.

Would it be acceptable to mark all those drivers as deprecated/obsolete,
warn users that the driver should be converted to use the watchdog
subsystem, and that it will otherwise be removed in a later Linux kernel
version ? This would give us an idea which drivers are still in use,
and it would enable us to remove the remaining drivers maybe 5 or 6
releases for now.

Thoughts ?

Guenter


2019-07-30 11:05:11

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Marking legacy watchdog drivers as deprecated / obsolete

On Tue, Jul 30, 2019 at 12:07 AM Guenter Roeck <[email protected]> wrote:
>
> Hi,
>
> we have recently seen a number of changes to legacy watchdog drivers,
> mostly surrounding the coding style used some 10+ years ago, but also
> fixing minor formatting or coding problems found by static analyzers.
> This slowly rises above the level of background noise.
>
> Would it be acceptable to mark all those drivers as deprecated/obsolete,
> warn users that the driver should be converted to use the watchdog
> subsystem, and that it will otherwise be removed in a later Linux kernel
> version ? This would give us an idea which drivers are still in use,
> and it would enable us to remove the remaining drivers maybe 5 or 6
> releases for now.
>
> Thoughts ?

I don't think an automated approach across 61 drivers is likely to work
well. About half of the drivers appear to be for specific SoCs, and
removing the watchdog driver while keeping the rest of the SoC support
would not be helpful, it just means we break one of the drivers for the
last remaining users of an old SoC the next time they try to upgrade to
a new kernel.

It would probably be helpful to go through the list and see if any of
the drivers
are for platforms that are already gone. FWIW, here is the list of drivers that
have their own .ioctl() method, taken from a patch I'm sending soon
to add a .compat_ioctl handler:

arch/powerpc/platforms/52xx/mpc52xx_gpt.c
arch/um/drivers/harddog_kern.c
drivers/char/ipmi/ipmi_watchdog.c
drivers/hwmon/fschmd.c
drivers/rtc/rtc-ds1374.c
drivers/watchdog/acquirewdt.c
drivers/watchdog/advantechwdt.c
drivers/watchdog/alim1535_wdt.c
drivers/watchdog/alim7101_wdt.c
drivers/watchdog/ar7_wdt.c
drivers/watchdog/at91rm9200_wdt.c
drivers/watchdog/ath79_wdt.c
drivers/watchdog/bcm63xx_wdt.c
drivers/watchdog/cpu5wdt.c
drivers/watchdog/eurotechwdt.c
drivers/watchdog/f71808e_wdt.c
drivers/watchdog/gef_wdt.c
drivers/watchdog/geodewdt.c
drivers/watchdog/ib700wdt.c
drivers/watchdog/ibmasr.c
drivers/watchdog/indydog.c
drivers/watchdog/intel_scu_watchdog.c
drivers/watchdog/iop_wdt.c
drivers/watchdog/it8712f_wdt.c
drivers/watchdog/ixp4xx_wdt.c
drivers/watchdog/ks8695_wdt.c
drivers/watchdog/m54xx_wdt.c
drivers/watchdog/machzwd.c
drivers/watchdog/mixcomwd.c
drivers/watchdog/mtx-1_wdt.c
drivers/watchdog/mv64x60_wdt.c
drivers/watchdog/nuc900_wdt.c
drivers/watchdog/nv_tco.c
drivers/watchdog/pc87413_wdt.c
drivers/watchdog/pcwd.c
drivers/watchdog/pcwd_pci.c
drivers/watchdog/pcwd_usb.c
drivers/watchdog/pika_wdt.c
drivers/watchdog/pnx833x_wdt.c
drivers/watchdog/rc32434_wdt.c
drivers/watchdog/rdc321x_wdt.c
drivers/watchdog/riowd.c
drivers/watchdog/sa1100_wdt.c
drivers/watchdog/sb_wdog.c
drivers/watchdog/sbc60xxwdt.c
drivers/watchdog/sbc7240_wdt.c
drivers/watchdog/sbc_epx_c3.c
drivers/watchdog/sbc_fitpc2_wdt.c
drivers/watchdog/sc1200wdt.c
drivers/watchdog/sc520_wdt.c
drivers/watchdog/sch311x_wdt.c
drivers/watchdog/scx200_wdt.c
drivers/watchdog/smsc37b787_wdt.c
drivers/watchdog/w83877f_wdt.c
drivers/watchdog/w83977f_wdt.c
drivers/watchdog/wafer5823wdt.c
drivers/watchdog/wdrtas.c
drivers/watchdog/wdt.c
drivers/watchdog/wdt285.c
drivers/watchdog/wdt977.c
drivers/watchdog/wdt_pci.c

Arnd

2019-07-30 18:05:16

by Guenter Roeck

[permalink] [raw]
Subject: Re: Marking legacy watchdog drivers as deprecated / obsolete

On Tue, Jul 30, 2019 at 10:00:36AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 30, 2019 at 12:07 AM Guenter Roeck <[email protected]> wrote:
> >
> > Hi,
> >
> > we have recently seen a number of changes to legacy watchdog drivers,
> > mostly surrounding the coding style used some 10+ years ago, but also
> > fixing minor formatting or coding problems found by static analyzers.
> > This slowly rises above the level of background noise.
> >
> > Would it be acceptable to mark all those drivers as deprecated/obsolete,
> > warn users that the driver should be converted to use the watchdog
> > subsystem, and that it will otherwise be removed in a later Linux kernel
> > version ? This would give us an idea which drivers are still in use,
> > and it would enable us to remove the remaining drivers maybe 5 or 6
> > releases for now.
> >
> > Thoughts ?
>
> I don't think an automated approach across 61 drivers is likely to work
> well. About half of the drivers appear to be for specific SoCs, and
> removing the watchdog driver while keeping the rest of the SoC support
> would not be helpful, it just means we break one of the drivers for the
> last remaining users of an old SoC the next time they try to upgrade to
> a new kernel.
>

The primary goal would be to identify drivers still in use, and to trigger
efforts to convert those drivers to the new infrastructure. Removal of
obsolete / unused drivers would be a separate decision, to be made at some
point in the future, and individually for each driver. I specifically wasn't
trying to suggest auto-removal.

Guenter

> It would probably be helpful to go through the list and see if any of
> the drivers
> are for platforms that are already gone. FWIW, here is the list of drivers that
> have their own .ioctl() method, taken from a patch I'm sending soon
> to add a .compat_ioctl handler:
>
> arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> arch/um/drivers/harddog_kern.c
> drivers/char/ipmi/ipmi_watchdog.c
> drivers/hwmon/fschmd.c
> drivers/rtc/rtc-ds1374.c
> drivers/watchdog/acquirewdt.c
> drivers/watchdog/advantechwdt.c
> drivers/watchdog/alim1535_wdt.c
> drivers/watchdog/alim7101_wdt.c
> drivers/watchdog/ar7_wdt.c
> drivers/watchdog/at91rm9200_wdt.c
> drivers/watchdog/ath79_wdt.c
> drivers/watchdog/bcm63xx_wdt.c
> drivers/watchdog/cpu5wdt.c
> drivers/watchdog/eurotechwdt.c
> drivers/watchdog/f71808e_wdt.c
> drivers/watchdog/gef_wdt.c
> drivers/watchdog/geodewdt.c
> drivers/watchdog/ib700wdt.c
> drivers/watchdog/ibmasr.c
> drivers/watchdog/indydog.c
> drivers/watchdog/intel_scu_watchdog.c
> drivers/watchdog/iop_wdt.c
> drivers/watchdog/it8712f_wdt.c
> drivers/watchdog/ixp4xx_wdt.c
> drivers/watchdog/ks8695_wdt.c
> drivers/watchdog/m54xx_wdt.c
> drivers/watchdog/machzwd.c
> drivers/watchdog/mixcomwd.c
> drivers/watchdog/mtx-1_wdt.c
> drivers/watchdog/mv64x60_wdt.c
> drivers/watchdog/nuc900_wdt.c
> drivers/watchdog/nv_tco.c
> drivers/watchdog/pc87413_wdt.c
> drivers/watchdog/pcwd.c
> drivers/watchdog/pcwd_pci.c
> drivers/watchdog/pcwd_usb.c
> drivers/watchdog/pika_wdt.c
> drivers/watchdog/pnx833x_wdt.c
> drivers/watchdog/rc32434_wdt.c
> drivers/watchdog/rdc321x_wdt.c
> drivers/watchdog/riowd.c
> drivers/watchdog/sa1100_wdt.c
> drivers/watchdog/sb_wdog.c
> drivers/watchdog/sbc60xxwdt.c
> drivers/watchdog/sbc7240_wdt.c
> drivers/watchdog/sbc_epx_c3.c
> drivers/watchdog/sbc_fitpc2_wdt.c
> drivers/watchdog/sc1200wdt.c
> drivers/watchdog/sc520_wdt.c
> drivers/watchdog/sch311x_wdt.c
> drivers/watchdog/scx200_wdt.c
> drivers/watchdog/smsc37b787_wdt.c
> drivers/watchdog/w83877f_wdt.c
> drivers/watchdog/w83977f_wdt.c
> drivers/watchdog/wafer5823wdt.c
> drivers/watchdog/wdrtas.c
> drivers/watchdog/wdt.c
> drivers/watchdog/wdt285.c
> drivers/watchdog/wdt977.c
> drivers/watchdog/wdt_pci.c
>
> Arnd

2019-08-02 02:22:30

by Ondrej Zary

[permalink] [raw]
Subject: Re: Marking legacy watchdog drivers as deprecated / obsolete

On Tuesday 30 July 2019 17:57:37 Guenter Roeck wrote:
> On Tue, Jul 30, 2019 at 10:00:36AM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 30, 2019 at 12:07 AM Guenter Roeck <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > we have recently seen a number of changes to legacy watchdog drivers,
> > > mostly surrounding the coding style used some 10+ years ago, but also
> > > fixing minor formatting or coding problems found by static analyzers.
> > > This slowly rises above the level of background noise.
> > >
> > > Would it be acceptable to mark all those drivers as deprecated/obsolete,
> > > warn users that the driver should be converted to use the watchdog
> > > subsystem, and that it will otherwise be removed in a later Linux kernel
> > > version ? This would give us an idea which drivers are still in use,
> > > and it would enable us to remove the remaining drivers maybe 5 or 6
> > > releases for now.
> > >
> > > Thoughts ?
> >
> > I don't think an automated approach across 61 drivers is likely to work
> > well. About half of the drivers appear to be for specific SoCs, and
> > removing the watchdog driver while keeping the rest of the SoC support
> > would not be helpful, it just means we break one of the drivers for the
> > last remaining users of an old SoC the next time they try to upgrade to
> > a new kernel.
> >
>
> The primary goal would be to identify drivers still in use, and to trigger
> efforts to convert those drivers to the new infrastructure. Removal of
> obsolete / unused drivers would be a separate decision, to be made at some
> point in the future, and individually for each driver. I specifically wasn't
> trying to suggest auto-removal.
>
> Guenter
>
> > It would probably be helpful to go through the list and see if any of
> > the drivers
> > are for platforms that are already gone. FWIW, here is the list of drivers that
> > have their own .ioctl() method, taken from a patch I'm sending soon
> > to add a .compat_ioctl handler:
> >
> > arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> > arch/um/drivers/harddog_kern.c
> > drivers/char/ipmi/ipmi_watchdog.c
> > drivers/hwmon/fschmd.c
> > drivers/rtc/rtc-ds1374.c
> > drivers/watchdog/acquirewdt.c
> > drivers/watchdog/advantechwdt.c
> > drivers/watchdog/alim1535_wdt.c
> > drivers/watchdog/alim7101_wdt.c
> > drivers/watchdog/ar7_wdt.c
> > drivers/watchdog/at91rm9200_wdt.c
> > drivers/watchdog/ath79_wdt.c
> > drivers/watchdog/bcm63xx_wdt.c
> > drivers/watchdog/cpu5wdt.c
> > drivers/watchdog/eurotechwdt.c
> > drivers/watchdog/f71808e_wdt.c
> > drivers/watchdog/gef_wdt.c
> > drivers/watchdog/geodewdt.c
> > drivers/watchdog/ib700wdt.c
> > drivers/watchdog/ibmasr.c
> > drivers/watchdog/indydog.c
> > drivers/watchdog/intel_scu_watchdog.c
> > drivers/watchdog/iop_wdt.c
> > drivers/watchdog/it8712f_wdt.c
> > drivers/watchdog/ixp4xx_wdt.c
> > drivers/watchdog/ks8695_wdt.c
> > drivers/watchdog/m54xx_wdt.c
> > drivers/watchdog/machzwd.c
> > drivers/watchdog/mixcomwd.c
> > drivers/watchdog/mtx-1_wdt.c
> > drivers/watchdog/mv64x60_wdt.c
> > drivers/watchdog/nuc900_wdt.c
> > drivers/watchdog/nv_tco.c
> > drivers/watchdog/pc87413_wdt.c
> > drivers/watchdog/pcwd.c
> > drivers/watchdog/pcwd_pci.c
> > drivers/watchdog/pcwd_usb.c
> > drivers/watchdog/pika_wdt.c
> > drivers/watchdog/pnx833x_wdt.c
> > drivers/watchdog/rc32434_wdt.c
> > drivers/watchdog/rdc321x_wdt.c
> > drivers/watchdog/riowd.c
> > drivers/watchdog/sa1100_wdt.c
> > drivers/watchdog/sb_wdog.c
> > drivers/watchdog/sbc60xxwdt.c
> > drivers/watchdog/sbc7240_wdt.c
> > drivers/watchdog/sbc_epx_c3.c
> > drivers/watchdog/sbc_fitpc2_wdt.c
> > drivers/watchdog/sc1200wdt.c
> > drivers/watchdog/sc520_wdt.c
> > drivers/watchdog/sch311x_wdt.c
> > drivers/watchdog/scx200_wdt.c
> > drivers/watchdog/smsc37b787_wdt.c
> > drivers/watchdog/w83877f_wdt.c
> > drivers/watchdog/w83977f_wdt.c
> > drivers/watchdog/wafer5823wdt.c
> > drivers/watchdog/wdrtas.c
> > drivers/watchdog/wdt.c
> > drivers/watchdog/wdt285.c
> > drivers/watchdog/wdt977.c
> > drivers/watchdog/wdt_pci.c
> >
> > Arnd
>

I have some older x86 boards so I probably could test some of them. Likely ALi chipset (alim1535_wdt.c and/or alim7101_wdt.c) and some super I/Os (it8712f_wdt.c, w83877f_wdt.c, w83977f_wdt.c).

--
Ondrej Zary