piixpm(4) support for AMD FCH watchdog

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

piixpm(4) support for AMD FCH watchdog

Nathanael Rensen-3
The diff below adds support for the watchdog as found in the embedded
AMD FCH (fusion controller hub) as found on APU2.

Index: sys/dev/pci/piixpm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/piixpm.c,v
retrieving revision 1.42
diff -u -p -r1.42 piixpm.c
--- sys/dev/pci/piixpm.c 21 Jan 2020 06:37:24 -0000 1.42
+++ sys/dev/pci/piixpm.c 8 Feb 2020 04:44:42 -0000
@@ -54,8 +54,10 @@ struct piixpm_softc {
  struct device sc_dev;
 
  bus_space_tag_t sc_iot;
+ bus_space_tag_t sc_memt;
  bus_space_handle_t sc_ioh;
  bus_space_handle_t sc_sb800_ioh;
+ bus_space_handle_t sc_wdt_mh;
  void * sc_ih;
  int sc_poll;
  int sc_is_sb800;
@@ -83,6 +85,8 @@ int piixpm_i2c_exec(void *, i2c_op_t, i2
 
 int piixpm_intr(void *);
 
+int piixpm_wdt_cb(void *arg, int period);
+
 struct cfattach piixpm_ca = {
  sizeof(struct piixpm_softc),
  piixpm_match,
@@ -127,7 +131,7 @@ piixpm_attach(struct device *parent, str
  struct piixpm_softc *sc = (struct piixpm_softc *)self;
  struct pci_attach_args *pa = aux;
  bus_space_handle_t ioh;
- u_int16_t val, smb0en;
+ u_int16_t val, smb0en, wdten = 0;
  bus_addr_t base;
  pcireg_t conf;
  pci_intr_handle_t ih;
@@ -136,6 +140,7 @@ piixpm_attach(struct device *parent, str
  int numbusses, i;
 
  sc->sc_iot = pa->pa_iot;
+ sc->sc_memt = pa->pa_memt;
  numbusses = 1;
 
  if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
@@ -160,7 +165,7 @@ piixpm_attach(struct device *parent, str
 
  /*
  * AMD Bolton matches PCI_PRODUCT_AMD_HUDSON2_SMB but
- * uses old register layout. Therefor check PCI_REVISION.
+ * uses old register layout. Therefore check PCI_REVISION.
  */
  if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
     ((PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB &&
@@ -170,6 +175,7 @@ piixpm_attach(struct device *parent, str
     AMDFCH41_PM_DECODE_EN);
  val = bus_space_read_1(sc->sc_iot, ioh, 1);
  smb0en = val & AMDFCH41_SMBUS_EN;
+ wdten = val & AMDFCH41_WDT_EN;
 
  bus_space_write_1(sc->sc_iot, ioh, 0,
     AMDFCH41_PM_DECODE_EN + 1);
@@ -282,7 +288,59 @@ piixpm_attach(struct device *parent, str
  config_found(self, &iba, iicbus_print);
  }
 
+ /* Register watchdog */
+ if (wdten && bus_space_map(sc->sc_memt, AMDFCH41_WDTREG_BASE,
+    AMDFCH41_WDTREG_SIZE, 0, &sc->sc_wdt_mh) == 0) {
+ val = bus_space_read_1(sc->sc_memt, sc->sc_wdt_mh,
+    AMDFCH41_WDTREG_CTL);
+ if (val & AMDFCH41_WDTREG_CTL_FIRED) {
+ printf("%s watchdog caused previous restart\n",
+    sc->sc_dev.dv_xname);
+ bus_space_write_1(sc->sc_memt, sc->sc_wdt_mh,
+    AMDFCH41_WDTREG_CTL,
+    val | AMDFCH41_WDTREG_CTL_FIRED);
+ }
+
+ if (val & AMDFCH41_WDTREG_CTL_DISABLED)
+ printf("%s watchdog disabled\n", sc->sc_dev.dv_xname);
+ else {
+ printf("%s watchdog found\n", sc->sc_dev.dv_xname);
+
+ /* Set 1 second counter period */
+ bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0,
+    AMDFCH41_PM_DECODE_EN + 3);
+ val = bus_space_read_1(sc->sc_iot, sc->sc_sb800_ioh, 1);
+ val = (val & ~AMDFCH41_WDT_MASK) | AMDFCH41_WDT_1S;
+ bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, val);
+
+ wdog_register(piixpm_wdt_cb, sc);
+ }
+ }
+
  return;
+}
+
+int
+piixpm_wdt_cb(void *arg, int period)
+{
+ struct piixpm_softc *sc = (struct piixpm_softc *)arg;
+ u_int16_t val;
+
+ val = bus_space_read_1(sc->sc_memt, sc->sc_wdt_mh, AMDFCH41_WDTREG_CTL);
+
+ if (period > 0xffff)
+ period = 0xffff;
+ if (period > 0) {
+ bus_space_write_2(sc->sc_memt, sc->sc_wdt_mh,
+    AMDFCH41_WDTREG_COUNT, period);
+ val |= AMDFCH41_WDTREG_CTL_RUN | AMDFCH41_WDTREG_CTL_TRIGGER;
+ }
+ else
+ val &= ~AMDFCH41_WDTREG_CTL_RUN;
+
+ bus_space_write_1(sc->sc_memt, sc->sc_wdt_mh, AMDFCH41_WDTREG_CTL, val);
+
+ return period;
 }
 
 int
Index: sys/dev/pci/piixreg.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/piixreg.h,v
retrieving revision 1.6
diff -u -p -r1.6 piixreg.h
--- sys/dev/pci/piixreg.h 21 Jan 2020 06:37:24 -0000 1.6
+++ sys/dev/pci/piixreg.h 8 Feb 2020 04:44:42 -0000
@@ -87,5 +87,19 @@
 #define AMDFCH41_PM_DECODE_EN 0x00 /* 16-bit register */
 #define AMDFCH41_PM_PORT_INDEX 0x02
 #define AMDFCH41_SMBUS_EN 0x10
+#define AMDFCH41_WDT_EN 0x80
+#define AMDFCH41_WDT_MASK 0x0f
+#define AMDFCH41_WDT_32US 0x00
+#define AMDFCH41_WDT_10MS 0x01
+#define AMDFCH41_WDT_100MS 0x02
+#define AMDFCH41_WDT_1S 0x03
+#define AMDFCH41_WDTREG_BASE 0xfeb00000
+#define AMDFCH41_WDTREG_SIZE 0x10
+#define AMDFCH41_WDTREG_CTL 0x00
+#define AMDFCH41_WDTREG_CTL_RUN 0x01
+#define AMDFCH41_WDTREG_CTL_FIRED 0x02
+#define AMDFCH41_WDTREG_CTL_DISABLED 0x08
+#define AMDFCH41_WDTREG_CTL_TRIGGER 0x80
+#define AMDFCH41_WDTREG_COUNT 0x04
 
 #endif /* !_DEV_PCI_PIIXREG_H_ */

Reply | Threaded
Open this post in threaded view
|

Re: piixpm(4) support for AMD FCH watchdog

Claudio Jeker
On Sat, Feb 08, 2020 at 01:07:01PM +0800, Nathanael Rensen wrote:

> The diff below adds support for the watchdog as found in the embedded
> AMD FCH (fusion controller hub) as found on APU2.
>
> Index: sys/dev/pci/piixpm.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/piixpm.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 piixpm.c
> --- sys/dev/pci/piixpm.c 21 Jan 2020 06:37:24 -0000 1.42
> +++ sys/dev/pci/piixpm.c 8 Feb 2020 04:44:42 -0000
> @@ -54,8 +54,10 @@ struct piixpm_softc {
>   struct device sc_dev;
>  
>   bus_space_tag_t sc_iot;
> + bus_space_tag_t sc_memt;
>   bus_space_handle_t sc_ioh;
>   bus_space_handle_t sc_sb800_ioh;
> + bus_space_handle_t sc_wdt_mh;
>   void * sc_ih;
>   int sc_poll;
>   int sc_is_sb800;
> @@ -83,6 +85,8 @@ int piixpm_i2c_exec(void *, i2c_op_t, i2
>  
>  int piixpm_intr(void *);
>  
> +int piixpm_wdt_cb(void *arg, int period);
> +
>  struct cfattach piixpm_ca = {
>   sizeof(struct piixpm_softc),
>   piixpm_match,
> @@ -127,7 +131,7 @@ piixpm_attach(struct device *parent, str
>   struct piixpm_softc *sc = (struct piixpm_softc *)self;
>   struct pci_attach_args *pa = aux;
>   bus_space_handle_t ioh;
> - u_int16_t val, smb0en;
> + u_int16_t val, smb0en, wdten = 0;
>   bus_addr_t base;
>   pcireg_t conf;
>   pci_intr_handle_t ih;
> @@ -136,6 +140,7 @@ piixpm_attach(struct device *parent, str
>   int numbusses, i;
>  
>   sc->sc_iot = pa->pa_iot;
> + sc->sc_memt = pa->pa_memt;
>   numbusses = 1;
>  
>   if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> @@ -160,7 +165,7 @@ piixpm_attach(struct device *parent, str
>  
>   /*
>   * AMD Bolton matches PCI_PRODUCT_AMD_HUDSON2_SMB but
> - * uses old register layout. Therefor check PCI_REVISION.
> + * uses old register layout. Therefore check PCI_REVISION.
>   */
>   if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
>      ((PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB &&
> @@ -170,6 +175,7 @@ piixpm_attach(struct device *parent, str
>      AMDFCH41_PM_DECODE_EN);
>   val = bus_space_read_1(sc->sc_iot, ioh, 1);
>   smb0en = val & AMDFCH41_SMBUS_EN;
> + wdten = val & AMDFCH41_WDT_EN;
>  
>   bus_space_write_1(sc->sc_iot, ioh, 0,
>      AMDFCH41_PM_DECODE_EN + 1);
> @@ -282,7 +288,59 @@ piixpm_attach(struct device *parent, str
>   config_found(self, &iba, iicbus_print);
>   }
>  

Why are you registering the watchdog late and not in the FCH specific
block above? Do other AMD SBx00 or KERNCZ chips support the watchdog in
the same way?

> + /* Register watchdog */
> + if (wdten && bus_space_map(sc->sc_memt, AMDFCH41_WDTREG_BASE,
> +    AMDFCH41_WDTREG_SIZE, 0, &sc->sc_wdt_mh) == 0) {
> + val = bus_space_read_1(sc->sc_memt, sc->sc_wdt_mh,
> +    AMDFCH41_WDTREG_CTL);
> + if (val & AMDFCH41_WDTREG_CTL_FIRED) {
> + printf("%s watchdog caused previous restart\n",
> +    sc->sc_dev.dv_xname);
> + bus_space_write_1(sc->sc_memt, sc->sc_wdt_mh,
> +    AMDFCH41_WDTREG_CTL,
> +    val | AMDFCH41_WDTREG_CTL_FIRED);
> + }
> +
> + if (val & AMDFCH41_WDTREG_CTL_DISABLED)
> + printf("%s watchdog disabled\n", sc->sc_dev.dv_xname);
> + else {
> + printf("%s watchdog found\n", sc->sc_dev.dv_xname);

The printf used here are non standard. Ideally this printing is folded
into the attach line of piixpm(4), e.g:
piixpm0 at pci0 dev 20 function 0 "AMD Hudson-2 SMBus" rev 0x42: SMI, watchdog
Same for the 'watchdog caused previous restart'. I think the common idiom
is to print ', watchdog' and ', reboot on timeout' for the two cases.

Also move the if (val & AMDFCH41_WDTREG_CTL_FIRED) check after
if (val & AMDFCH41_WDTREG_CTL_DISABLED)

> +
> + /* Set 1 second counter period */
> + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0,
> +    AMDFCH41_PM_DECODE_EN + 3);
> + val = bus_space_read_1(sc->sc_iot, sc->sc_sb800_ioh, 1);
> + val = (val & ~AMDFCH41_WDT_MASK) | AMDFCH41_WDT_1S;
> + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, val);
> +
> + wdog_register(piixpm_wdt_cb, sc);
> + }
> + }
> +
>   return;
> +}
> +
> +int
> +piixpm_wdt_cb(void *arg, int period)
> +{
> + struct piixpm_softc *sc = (struct piixpm_softc *)arg;
> + u_int16_t val;
> +
> + val = bus_space_read_1(sc->sc_memt, sc->sc_wdt_mh, AMDFCH41_WDTREG_CTL);
> +
> + if (period > 0xffff)
> + period = 0xffff;
> + if (period > 0) {
> + bus_space_write_2(sc->sc_memt, sc->sc_wdt_mh,
> +    AMDFCH41_WDTREG_COUNT, period);
> + val |= AMDFCH41_WDTREG_CTL_RUN | AMDFCH41_WDTREG_CTL_TRIGGER;
> + }
> + else

KNF, move else up

> + val &= ~AMDFCH41_WDTREG_CTL_RUN;
> +
> + bus_space_write_1(sc->sc_memt, sc->sc_wdt_mh, AMDFCH41_WDTREG_CTL, val);
> +
> + return period;
>  }

I think for watchdogs you need at least a DVACT_POWERDOWN activate handler
where wdog_shutdown() is called. Maybe more is needed for DVACT_SUSPEND
and DVACT_RESUME (stop wdog on suspend and restart wdog on resume).

>  int
> Index: sys/dev/pci/piixreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/piixreg.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 piixreg.h
> --- sys/dev/pci/piixreg.h 21 Jan 2020 06:37:24 -0000 1.6
> +++ sys/dev/pci/piixreg.h 8 Feb 2020 04:44:42 -0000
> @@ -87,5 +87,19 @@
>  #define AMDFCH41_PM_DECODE_EN 0x00 /* 16-bit register */
>  #define AMDFCH41_PM_PORT_INDEX 0x02
>  #define AMDFCH41_SMBUS_EN 0x10
> +#define AMDFCH41_WDT_EN 0x80
> +#define AMDFCH41_WDT_MASK 0x0f
> +#define AMDFCH41_WDT_32US 0x00
> +#define AMDFCH41_WDT_10MS 0x01
> +#define AMDFCH41_WDT_100MS 0x02
> +#define AMDFCH41_WDT_1S 0x03
> +#define AMDFCH41_WDTREG_BASE 0xfeb00000
> +#define AMDFCH41_WDTREG_SIZE 0x10
> +#define AMDFCH41_WDTREG_CTL 0x00
> +#define AMDFCH41_WDTREG_CTL_RUN 0x01
> +#define AMDFCH41_WDTREG_CTL_FIRED 0x02
> +#define AMDFCH41_WDTREG_CTL_DISABLED 0x08
> +#define AMDFCH41_WDTREG_CTL_TRIGGER 0x80
> +#define AMDFCH41_WDTREG_COUNT 0x04
>  
>  #endif /* !_DEV_PCI_PIIXREG_H_ */
>

--
:wq Claudio