On Fri, Mar 18, 2016 at 08:18:30PM +0100, Mark Kettenis wrote:
> > Date: Thu, 17 Mar 2016 14:28:04 +0900
> > From: Masao Uebayashi <uebay...@tombiinc.com>
> >
> > This tiny driver is only meant for a backup watchdog configuration on
> > QEMU, whose ICH watchdog, supported by ichwdt(4), seems broken.
> >
> > ib700wdt(4) supports only 0-30 second timeouts but in practice it should
> > be good enough.
>
Wait. So we're saying "let's write an entirely new driver because qemu
has broken emulation somewhere else"? Why not just ask (nicely) the qemu
folks to fix the root cause, upstream?
I had a pretty good exchange with some of their devs in the past - can't
speak for all of them but the team I was working with before seemed very
receptive to accepting changes.
Writing a new device for this (nevermind the concerns kettenis and theo
are also mentioning) seems like the wrong approach.
-ml
> The problem is that isa-style probes like this are dangerous. That is
> probably why you didn't enable this driver by default. But if you
> don't enable the code, it will rot.
>
> Is there a better way to detect this "device"? For example in the
> acpi device tree?
>
> > diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
> > index fee3c2a..82e7de0 100644
> > --- a/sys/arch/amd64/conf/GENERIC
> > +++ b/sys/arch/amd64/conf/GENERIC
> > @@ -608,6 +608,7 @@ bktr0 at pci?
> > radio* at bktr?
> >
> > #wdt0 at pci? # Ind Computer Source PCI-WDT50x driver
> > +#ib700wdt* at isa? port 0x441 # iBase 700 (IB700) Watchdog Timer
> >
> > # crypto support
> > hifn* at pci? # Hi/fn 7751 crypto card
> > diff --git a/sys/arch/i386/conf/GENERIC b/sys/arch/i386/conf/GENERIC
> > index 0984e5b..dbb7df6 100644
> > --- a/sys/arch/i386/conf/GENERIC
> > +++ b/sys/arch/i386/conf/GENERIC
> > @@ -108,6 +108,7 @@ geodesc* at pci? # Geode SC1100/SCx200
> > IAOC
> > wdt0 at pci? # Ind Computer Source PCI-WDT50x driver
> > berkwdt0 at pci? # Berkshire PCI-PC Watchdog driver
> > pwdog0 at pci? # Quancom PWDOG1 watchdog timer
> > +#ib700wdt* at isa? port 0x441 # iBase 700 (IB700) Watchdog Timer
> >
> > # National Semiconductor LM7[89] and compatible hardware monitors
> > lm0 at isa? port 0x290
> > diff --git a/sys/dev/isa/files.isa b/sys/dev/isa/files.isa
> > index 863d11c..92a9ecc 100644
> > --- a/sys/dev/isa/files.isa
> > +++ b/sys/dev/isa/files.isa
> > @@ -380,3 +380,8 @@ file dev/isa/isagpio.c isagpio
> > #file dev/isa/pcmcia_pcic.c pcic | pcicmaster
> >
> > #file dev/isa/pcmcia_isa.c pcmcia
> > +
> > +# iBase 700 (IB700) Watchdog Timer
> > +device ib700wdt
> > +attach ib700wdt at isa
> > +file dev/isa/ib700wdt.c ib700wdt
> > diff --git a/sys/dev/isa/ib700wdt.c b/sys/dev/isa/ib700wdt.c
> > new file mode 100644
> > index 0000000..cdad79f
> > --- /dev/null
> > +++ b/sys/dev/isa/ib700wdt.c
> > @@ -0,0 +1,142 @@
> > +/*
> > + * Copyright (c) 2016 Masao Uebayashi <uebay...@tombiinc.com>
> > + *
> > + * Permission to use, copy, modify, and distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#include <sys/param.h>
> > +#include <sys/systm.h>
> > +#include <sys/device.h>
> > +#include <sys/kernel.h>
> > +#include <sys/timeout.h>
> > +#include <machine/bus.h>
> > +#include <sys/event.h>
> > +
> > +#include <dev/isa/isavar.h>
> > +
> > +/*
> > + * IB700 USER'S MANUAL
> > + * Version 1.0B
> > + * p28
> > + * Watchdog Timer Configuration
> > + */
> > +
> > +#define IB700WDT_BASE 0x441
> > +#define IB700WDT_SIZE 3
> > +#define IB700WDT_DIS 0 /* 0x441 */
> > +#define IB700WDT_ENA 2 /* 0x443 */
> > +#define IB700WDT_SEC_MIN 0
> > +#define IB700WDT_SEC_MAX 30
> > +
> > +static inline uint8_t
> > +ib700wdt_sec2val(int sec)
> > +{
> > + /* 0/1sec = 0xf, 2/3sec = 0xe, ..., 28/29 = 0x1, 30sec = 0x0 */
> > + return (0xf - (sec * 0xf / 30));
> > +}
> > +
> > +/*
> > + * Driver for iBase 700 Watchdog Timer
> > + * Mainly meant for QEMU's "ib700" watchdog device
> > + * Can't handle immediate reset (sec=0)
> > + */
> > +
> > +struct ib700wdt_softc {
> > + struct device sc_dev;
> > + bus_space_tag_t sc_iot;
> > + bus_space_handle_t sc_ioh;
> > + int sc_period;
> > +};
> > +
> > +int ib700wdt_match(struct device *, void *, void *);
> > +void ib700wdt_attach(struct device *, struct device *, void *);
> > +int ib700wdt_activate(struct device *, int);
> > +
> > +int ib700wdt_cb(void *, int);
> > +
> > +struct cfattach ib700wdt_ca = {
> > + sizeof(struct ib700wdt_softc),
> > + ib700wdt_match, ib700wdt_attach, NULL, ib700wdt_activate
> > +};
> > +
> > +struct cfdriver ib700wdt_cd = {
> > + NULL, "ib700wdt", DV_DULL
> > +};
> > +
> > +int
> > +ib700wdt_match(struct device *parent, void *match, void *aux)
> > +{
> > + struct isa_attach_args *ia = aux;
> > +
> > + ia->ia_iosize = IB700WDT_SIZE;
> > + ia->ia_msize = 0;
> > +
> > + return (1);
> > +}
> > +
> > +void
> > +ib700wdt_attach(struct device *parent, struct device *self, void *aux)
> > +{
> > + struct ib700wdt_softc *sc = (struct ib700wdt_softc *)self;
> > + struct isa_attach_args *ia = aux;
> > +
> > + sc->sc_iot = ia->ia_iot;
> > +
> > + if (bus_space_map(sc->sc_iot, ia->ia_iobase, IB700WDT_SIZE, 0,
> > &sc->sc_ioh))
> > + return;
> > +
> > + printf("\n");
> > +
> > + wdog_register(ib700wdt_cb, sc);
> > +}
> > +
> > +int
> > +ib700wdt_activate(struct device *self, int act)
> > +{
> > + switch (act) {
> > + case DVACT_POWERDOWN:
> > + wdog_shutdown(self);
> > + break;
> > + }
> > +
> > + return (0);
> > +}
> > +
> > +int
> > +ib700wdt_cb(void *arg, int period)
> > +{
> > + struct ib700wdt_softc *sc = arg;
> > + uint8_t val;
> > +
> > + if (period == 0) {
> > + if (sc->sc_period != 0)
> > + bus_space_write_1(sc->sc_iot, sc->sc_ioh, IB700WDT_DIS,
> > 0);
> > + } else {
> > + if (period < IB700WDT_SEC_MIN)
> > + period = IB700WDT_SEC_MIN;
> > + else if (period > IB700WDT_SEC_MAX)
> > + period = IB700WDT_SEC_MAX;
> > + if (sc->sc_period != period) {
> > + val = ib700wdt_sec2val(period);
> > + bus_space_write_1(sc->sc_iot, sc->sc_ioh, IB700WDT_ENA,
> > val);
> > + }
> > + if (sc->sc_period == 0)
> > + val = ib700wdt_sec2val(period);
> > + else
> > + val = ib700wdt_sec2val(sc->sc_period);
> > + bus_space_write_1(sc->sc_iot, sc->sc_ioh, IB700WDT_ENA, val);
> > + }
> > + sc->sc_period = period;
> > +
> > + return (period);
> > +}
> >
> >
>
