> Date: Sat, 12 Mar 2016 20:49:05 +0100
> From: Stefan Sperling <[email protected]>
>
> On Sat, Mar 12, 2016 at 07:57:16PM +0100, Mark Kettenis wrote:
> > Simply ignoring the SMBALERT_STS bit, like your latest diff does, is a
> > bit dangerous. It seems that on later generations of the controller
> > there is a SMBALERT_DIS bit in the SLV_CMD register that determines
> > whether SMBALERT_STS actually causes an interrupt or not. I suppose
> > this bit is set on your board. But it may be cleared on other
> > hardware. And on such hardware, simply ignoring the SMBALERT_STS will
> > trigger an interrupt storm. Not sure if clearing the SMBALERT_STS bit
> > is enough to prevent such a storm, but I think we should at least try
> > to do that, like you did in one of the earlier diffs.
>
> This diff is printing SLV_CMD (Slave Command I/O Register, Offset 11h)
> from ichiic_intr in addition to interrupt status:
>
> Index: ichreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/ichreg.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 ichreg.h
> --- ichreg.h 18 Dec 2005 12:09:04 -0000 1.7
> +++ ichreg.h 12 Mar 2016 19:05:38 -0000
> @@ -113,6 +113,7 @@
> #define ICH_SMB_SCMD_INTREN (1 << 0) /* enable interrupts on HN */
> #define ICH_SMB_SCMD_WKEN (1 << 1) /* wake on HN */
> #define ICH_SMB_SCMD_SMBALDS (1 << 2) /* disable SMBALERT# intr */
> +#define ICH_SMB_SCMD_BITS "\020\001INTREN\002WKEN\003SMBALDS"
> #define ICH_SMB_NDADDR 0x14 /* notify device address */
> #define ICH_SMB_NDADDR_ADDR(x) ((x) >> 1) /* 7-bit address */
> #define ICH_SMB_NDLOW 0x16 /* notify data low byte */
> Index: ichiic.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/ichiic.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 ichiic.c
> --- ichiic.c 7 Dec 2015 02:56:36 -0000 1.37
> +++ ichiic.c 12 Mar 2016 19:33:04 -0000
> @@ -346,8 +346,11 @@ ichiic_intr(void *arg)
> /* Interrupt was not for us */
> return (0);
>
> - DPRINTF(("%s: intr st 0x%b\n", sc->sc_dev.dv_xname, st,
> - ICH_SMB_HS_BITS));
> + printf("%s: intr st 0x%b\n", sc->sc_dev.dv_xname, st,
> + ICH_SMB_HS_BITS);
> + printf("%s: slave cmd 0x%b\n", sc->sc_dev.dv_xname,
> + bus_space_read_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_SCMD),
> + ICH_SMB_SCMD_BITS);
>
> /* Clear status bits */
> bus_space_write_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS, st);
>
> With the .03 (buggy) BIOS, I see:
>
> ichiic0: intr st 0x20<SMBAL>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x60<SMBAL,INUSE>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x20<SMBAL>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x60<SMBAL,INUSE>
> ichiic0: slave cmd 0x0
>
> With the .08 (good) BIOS, I see:
>
> ichiic0: intr st 0x42<INTR,INUSE>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x42<INTR,INUSE>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x42<INTR,INUSE>
> ichiic0: slave cmd 0x0
> ichiic0: intr st 0x42<INTR,INUSE>
> ichiic0: slave cmd 0x0
>
> So the SMBALERT_DIS bit is always cleared on this board.
> I don't think we can infer from any of this how other hardware will behave.
>
> I guess you would prefer this version for commit, then?
Indeed. ok kettenis@
> Index: ichiic.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/ichiic.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 ichiic.c
> --- ichiic.c 7 Dec 2015 02:56:36 -0000 1.37
> +++ ichiic.c 12 Mar 2016 19:45:25 -0000
> @@ -340,17 +340,19 @@ ichiic_intr(void *arg)
>
> /* Read status */
> st = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS);
> +
> + /* Clear status bits */
> + bus_space_write_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS, st);
> +
> + /* XXX Ignore SMBALERT# for now */
> if ((st & ICH_SMB_HS_BUSY) != 0 || (st & (ICH_SMB_HS_INTR |
> ICH_SMB_HS_DEVERR | ICH_SMB_HS_BUSERR | ICH_SMB_HS_FAILED |
> - ICH_SMB_HS_SMBAL | ICH_SMB_HS_BDONE)) == 0)
> + ICH_SMB_HS_BDONE)) == 0)
> /* Interrupt was not for us */
> return (0);
>
> DPRINTF(("%s: intr st 0x%b\n", sc->sc_dev.dv_xname, st,
> ICH_SMB_HS_BITS));
> -
> - /* Clear status bits */
> - bus_space_write_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS, st);
>
> /* Check for errors */
> if (st & (ICH_SMB_HS_DEVERR | ICH_SMB_HS_BUSERR | ICH_SMB_HS_FAILED)) {
>
