Page MenuHomeFreeBSD
Differential D5327 Diff 13447 sys/arm/allwinner/aw_wdog.c

Changeset View

Standalone View

View Options

sys/arm/allwinner/aw_wdog.c

  • This file was moved from sys/arm/allwinner/a10_wdog.c.
/*- /*-
* Copyright (c) 2013 Oleksandr Tymoshenko <gonzo@freebsd.org> * Copyright (c) 2013 Oleksandr Tymoshenko <gonzo@freebsd.org>
* Copyright (c) 2016 Emmanuel Vadot <manu@bidouilliste.com>
* All rights reserved. * All rights reserved.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
* are met: * are met:
* 1. Redistributions of source code must retain the above copyright * 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer. * notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright * 2. Redistributions in binary form must reproduce the above copyright
Show All 29 Lines
#include <dev/ofw/openfirm.h> #include <dev/ofw/openfirm.h>
#include <dev/ofw/ofw_bus.h> #include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h> #include <dev/ofw/ofw_bus_subr.h>
#include <machine/bus.h> #include <machine/bus.h>
#include <machine/cpufunc.h> #include <machine/cpufunc.h>
#include <machine/machdep.h> #include <machine/machdep.h>
#include <arm/allwinner/a10_wdog.h> #include <arm/allwinner/aw_wdog.h>
#define READ(_sc, _r) bus_read_4((_sc)->res, (_r)) #define READ(_sc, _r) bus_read_4((_sc)->res, (_r))
#define WRITE(_sc, _r, _v) bus_write_4((_sc)->res, (_r), (_v)) #define WRITE(_sc, _r, _v) bus_write_4((_sc)->res, (_r), (_v))
#define WDOG_CTRL 0x00 #define A10_WDOG_CTRL 0x00
#define A31_WDOG_CTRL 0x10
jmcneill_invisible.caUnsubmitted
Not Done
Inline Actions

Bits 1:0 of A31_WDOG_CONFIG controls the target for the reset signal. 0x1 is "to whole system" and 0x2 is "only interrupt". Maybe A31_WDOG_CONFIG_RST_EN could have a name to better reflect this?

jmcneill_invisible.ca: Bits 1:0 of A31_WDOG_CONFIG controls the target for the reset signal. 0x1 is "to whole system"…
#define WDOG_CTRL_RESTART (1 << 0) #define WDOG_CTRL_RESTART (1 << 0)
#define WDOG_MODE 0x04 #define A10_WDOG_MODE 0x04
#define WDOG_MODE_INTVL_SHIFT 3 #define A31_WDOG_MODE 0x18
#define WDOG_MODE_RST_EN (1 << 1) #define A10_WDOG_MODE_INTVL_SHIFT 3
#define A31_WDOG_MODE_INTVL_SHIFT 4
andrewUnsubmitted
Not Done
Inline Actions

There is some odd indentation here.

andrew: There is some odd indentation here.
manu_bidouilliste.comAuthorUnsubmitted
Not Done
Inline Actions

This is the original indentation but I can fix that if you want.

manu_bidouilliste.com: This is the original indentation but I can fix that if you want.
andrewUnsubmitted
Not Done
Inline Actions

I normally use #define<tab>FOO for the register and #define<tab> FOO_BAR for the contents of the register.

andrew: I normally use `#define<tab>FOO` for the register and `#define<tab> FOO_BAR` for the contents…
#define A10_WDOG_MODE_RST_EN (1 << 1)
#define WDOG_MODE_EN (1 << 0) #define WDOG_MODE_EN (1 << 0)
#define A31_WDOG_CONFIG 0x14
#define A31_WDOG_CONFIG_RST_EN_SYSTEM (1 << 0)
#define A31_WDOG_CONFIG_RST_EN_INT (2 << 0)
struct a10wd_interval { struct aw_wdog_interval {
uint64_t milliseconds; uint64_t milliseconds;
unsigned int value; unsigned int value;
}; };
struct a10wd_interval wd_intervals[] = { struct aw_wdog_interval wd_intervals[] = {
{ 500, 0 }, { 500, 0 },
{ 1000, 1 }, { 1000, 1 },
{ 2000, 2 }, { 2000, 2 },
{ 3000, 3 }, { 3000, 3 },
{ 4000, 4 }, { 4000, 4 },
{ 5000, 5 }, { 5000, 5 },
{ 6000, 6 }, { 6000, 6 },
{ 8000, 7 }, { 8000, 7 },
{ 10000, 8 }, { 10000, 8 },
{ 12000, 9 }, { 12000, 9 },
{ 14000, 10 }, { 14000, 10 },
{ 16000, 11 }, { 16000, 11 },
{ 0, 0 } /* sentinel */ { 0, 0 } /* sentinel */
}; };
static struct a10wd_softc *a10wd_sc = NULL; static struct aw_wdog_softc *aw_wdog_sc = NULL;
struct a10wd_softc { struct aw_wdog_softc {
device_t dev; device_t dev;
struct resource * res; struct resource * res;
struct mtx mtx; struct mtx mtx;
uint8_t wdog_ctrl;
uint8_t wdog_mode;
uint8_t wdog_mode_intvl_shift;
uint8_t wdog_mode_en;
uint8_t wdog_config;
uint8_t wdog_config_value;
andrewUnsubmitted
Not Done
Inline Actions

At least wdog_mode_en should be a 32-bit value as it stores the contents of the register. It could be argued the register offsets should be bus_size_t. Given there are only a limited number of copies of this struct the size shouldn't matter too much.

andrew: At least `wdog_mode_en` should be a 32-bit value as it stores the contents of the register. It…
}; };
static void a10wd_watchdog_fn(void *private, u_int cmd, int *error); #define A10_WATCHDOG 1
#define A31_WATCHDOG 2
static struct ofw_compat_data compat_data[] = {
{"allwinner,sun4i-a10-wdt", A10_WATCHDOG},
{"allwinner,sun6i-a31-wdt", A31_WATCHDOG},
{NULL, 0}
};
static void aw_wdog_watchdog_fn(void *private, u_int cmd, int *error);
static int static int
a10wd_probe(device_t dev) aw_wdog_probe(device_t dev)
{ {
struct aw_wdog_softc *sc;
sc = device_get_softc(dev);
if (!ofw_bus_status_okay(dev)) if (!ofw_bus_status_okay(dev))
return (ENXIO); return (ENXIO);
switch (ofw_bus_search_compatible(dev, compat_data)->ocd_data) {
if (ofw_bus_is_compatible(dev, "allwinner,sun4i-a10-wdt")) { case A10_WATCHDOG:
device_set_desc(dev, "Allwinner A10 Watchdog"); device_set_desc(dev, "Allwinner A10 Watchdog");
return (BUS_PROBE_DEFAULT); return (BUS_PROBE_DEFAULT);
case A31_WATCHDOG:
device_set_desc(dev, "Allwinner A31 Watchdog");
return (BUS_PROBE_DEFAULT);
} }
return (ENXIO); return (ENXIO);
andrewUnsubmitted
Not Done
Inline Actions

Extra return.

andrew: Extra return.
} }
andrewUnsubmitted
Not Done
Inline Actions

You should use ofw_bus_search_compatible, and set the a10 flag in the attach function.

andrew: You should use `ofw_bus_search_compatible`, and set the a10 flag in the attach function.
andrewUnsubmitted
Not Done
Inline Actions

Missing device_set_desc(dev, "Allwinner A10 Watchdog"); (or similar).

andrew: Missing `device_set_desc(dev, "Allwinner A10 Watchdog");` (or similar).
static int static int
a10wd_attach(device_t dev) aw_wdog_attach(device_t dev)
{ {
struct a10wd_softc *sc; struct aw_wdog_softc *sc;
int rid; int rid;
if (a10wd_sc != NULL) if (aw_wdog_sc != NULL)
return (ENXIO); return (ENXIO);
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->dev = dev; sc->dev = dev;
rid = 0; rid = 0;
sc->res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE); sc->res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE);
if (sc->res == NULL) { if (sc->res == NULL) {
device_printf(dev, "could not allocate memory resource\n"); device_printf(dev, "could not allocate memory resource\n");
return (ENXIO); return (ENXIO);
} }
a10wd_sc = sc; aw_wdog_sc = sc;
mtx_init(&sc->mtx, "A10 Watchdog", "a10wd", MTX_DEF);
EVENTHANDLER_REGISTER(watchdog_list, a10wd_watchdog_fn, sc, 0);
switch (ofw_bus_search_compatible(dev, compat_data)->ocd_data) {
andrewUnsubmitted
Not Done
Inline Actions

I would suggest putting this last, after all the softc data is set.

andrew: I would suggest putting this last, after all the softc data is set.
case A10_WATCHDOG:
andrewUnsubmitted
Not Done
Inline Actions

You could put a unique value in the compat_data map, then have something like:

switch (ofw_bus_search_compatible(dev, compat_data)->ocd_data) {
case A10:
   ...
   break;
case A31:
   ...
   break;
 default:
   ...
   return (ENXIO);
}
andrew: You could put a unique value in the `compat_data` map, then have something like: switch…
sc->wdog_ctrl = A10_WDOG_CTRL;
sc->wdog_mode = A10_WDOG_MODE;
sc->wdog_mode_intvl_shift = A10_WDOG_MODE_INTVL_SHIFT;
sc->wdog_mode_en = A10_WDOG_MODE_RST_EN | WDOG_MODE_EN;
break;
case A31_WATCHDOG:
sc->wdog_ctrl = A31_WDOG_CTRL;
sc->wdog_mode = A31_WDOG_MODE;
sc->wdog_mode_intvl_shift = A31_WDOG_MODE_INTVL_SHIFT;
sc->wdog_mode_en = WDOG_MODE_EN;
sc->wdog_config = A31_WDOG_CONFIG;
sc->wdog_config_value = A31_WDOG_CONFIG_RST_EN_SYSTEM;
break;
default:
andrewUnsubmitted
Not Done
Inline Actions

If you move the mtx_init call below this you can remove this line.

andrew: If you move the `mtx_init` call below this you can remove this line.
bus_release_resource(dev, SYS_RES_MEMORY, rid, sc->res);
return (ENXIO);
}
mtx_init(&sc->mtx, "AW Watchdog", "aw_wdog", MTX_DEF);
EVENTHANDLER_REGISTER(watchdog_list, aw_wdog_watchdog_fn, sc, 0);
return (0); return (0);
} }
static void static void
a10wd_watchdog_fn(void *private, u_int cmd, int *error) aw_wdog_watchdog_fn(void *private, u_int cmd, int *error)
{ {
struct a10wd_softc *sc; struct aw_wdog_softc *sc;
uint64_t ms; uint64_t ms;
int i; int i;
sc = private; sc = private;
mtx_lock(&sc->mtx); mtx_lock(&sc->mtx);
cmd &= WD_INTERVAL; cmd &= WD_INTERVAL;
if (cmd > 0) { if (cmd > 0) {
ms = ((uint64_t)1 << (cmd & WD_INTERVAL)) / 1000000; ms = ((uint64_t)1 << (cmd & WD_INTERVAL)) / 1000000;
i = 0; i = 0;
while (wd_intervals[i].milliseconds && while (wd_intervals[i].milliseconds &&
(ms > wd_intervals[i].milliseconds)) (ms > wd_intervals[i].milliseconds))
i++; i++;
if (wd_intervals[i].milliseconds) { if (wd_intervals[i].milliseconds) {
WRITE(sc, WDOG_MODE, WRITE(sc, sc->wdog_mode,
(wd_intervals[i].value << WDOG_MODE_INTVL_SHIFT) | (wd_intervals[i].value << sc->wdog_mode_intvl_shift) |
WDOG_MODE_EN | WDOG_MODE_RST_EN); sc->wdog_mode_en);
WRITE(sc, WDOG_CTRL, WDOG_CTRL_RESTART); WRITE(sc, sc->wdog_ctrl, WDOG_CTRL_RESTART);
if (sc->wdog_config)
WRITE(sc, sc->wdog_config, sc->wdog_config_value);
*error = 0; *error = 0;
} }
else { else {
/* /*
* Can't arm * Can't arm
* disable watchdog as watchdog(9) requires * disable watchdog as watchdog(9) requires
*/ */
device_printf(sc->dev, device_printf(sc->dev,
"Can't arm, timeout is more than 16 sec\n"); "Can't arm, timeout is more than 16 sec\n");
mtx_unlock(&sc->mtx); mtx_unlock(&sc->mtx);
WRITE(sc, WDOG_MODE, 0); WRITE(sc, sc->wdog_mode, 0);
return; return;
} }
} }
else else
WRITE(sc, WDOG_MODE, 0); WRITE(sc, sc->wdog_mode, 0);
mtx_unlock(&sc->mtx); mtx_unlock(&sc->mtx);
} }
void void
a10wd_watchdog_reset() aw_wdog_watchdog_reset()
{ {
if (a10wd_sc == NULL) { if (aw_wdog_sc == NULL) {
printf("Reset: watchdog device has not been initialized\n"); printf("Reset: watchdog device has not been initialized\n");
return; return;
} }
WRITE(a10wd_sc, WDOG_MODE, WRITE(aw_wdog_sc, aw_wdog_sc->wdog_mode,
(wd_intervals[0].value << WDOG_MODE_INTVL_SHIFT) | (wd_intervals[0].value << aw_wdog_sc->wdog_mode_intvl_shift) |
WDOG_MODE_EN | WDOG_MODE_RST_EN); aw_wdog_sc->wdog_mode_en);
if (aw_wdog_sc->wdog_config)
WRITE(aw_wdog_sc, aw_wdog_sc->wdog_config,
aw_wdog_sc->wdog_config_value);
while(1) while(1)
; ;
} }
static device_method_t a10wd_methods[] = { static device_method_t aw_wdog_methods[] = {
DEVMETHOD(device_probe, a10wd_probe), DEVMETHOD(device_probe, aw_wdog_probe),
DEVMETHOD(device_attach, a10wd_attach), DEVMETHOD(device_attach, aw_wdog_attach),
DEVMETHOD_END DEVMETHOD_END
}; };
static driver_t a10wd_driver = { static driver_t aw_wdog_driver = {
"a10wd", "aw_wdog",
a10wd_methods, aw_wdog_methods,
sizeof(struct a10wd_softc), sizeof(struct aw_wdog_softc),
}; };
static devclass_t a10wd_devclass; static devclass_t aw_wdog_devclass;
DRIVER_MODULE(a10wd, simplebus, a10wd_driver, a10wd_devclass, 0, 0); DRIVER_MODULE(aw_wdog, simplebus, aw_wdog_driver, aw_wdog_devclass, 0, 0);