On Mon, Dec 16, 2019 at 12:37:51PM +0100, Claudio Jeker wrote: > This diff should add support for newer smbus controllers used on newer AMD > chipsets. Especially Hudson-2 and Kerncz based chipsets. On my Ryzen 5 the > iic(4) busses attach but there is nothing detected on them (well possible > that I missed something). I also implemented the up to 4 busses available > on chipsets of the SBx00 series (on Hudson-2 and Kerncz only 2 ports). > > I would be interested if on systems with Ryzen CPUs something attaches to > those iic(4) busses. Could be that I missed something and fail to properly > access the bus. > -- > :wq Claudio
I had a similar diff (except without the additional busses), and didn't go further as none of my machines show any devices either (not even spdmem(4)), I assumed it was on another bus.. but maybe it's on an entirely different controller now. bios0: vendor American Megatrends Inc. version "5220" date 09/11/2019 bios0: ASUSTeK COMPUTER INC. PRIME X470-PRO .. piixpm0 at pci0 dev 20 function 0 "AMD FCH SMBus" rev 0x59: polling iic0 at piixpm0 iic1 at piixpm0 Otherwise, diff "looks" ok. I'll try it on my MateBook later, but I have a feeling it was the same story. Otherwise, diff "looks" ok. > Index: piixpm.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/piixpm.c,v > retrieving revision 1.39 > diff -u -p -r1.39 piixpm.c > --- piixpm.c 1 Oct 2013 20:06:02 -0000 1.39 > +++ piixpm.c 16 Dec 2019 11:26:11 -0000 > @@ -45,15 +45,24 @@ > #define PIIXPM_DELAY 200 > #define PIIXPM_TIMEOUT 1 > > +struct piixpm_smbus { > + int sb_bus; > + struct piixpm_softc *sb_sc; > +}; > + > struct piixpm_softc { > struct device sc_dev; > > bus_space_tag_t sc_iot; > bus_space_handle_t sc_ioh; > + bus_space_handle_t sc_sb800_ioh; > void * sc_ih; > int sc_poll; > + int sc_is_sb800; > + int sc_is_kerncz; > > - struct i2c_controller sc_i2c_tag; > + struct piixpm_smbus sc_busses[4]; > + struct i2c_controller sc_i2c_tag[4]; > struct rwlock sc_i2c_lock; > struct { > i2c_op_t op; > @@ -86,6 +95,7 @@ struct cfdriver piixpm_cd = { > > const struct pci_matchid piixpm_ids[] = { > { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_HUDSON2_SMB }, > + { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_KERNCZ_SMB }, > > { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB200_SMB }, > { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB300_SMB }, > @@ -117,17 +127,21 @@ 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 smb0en; > + u_int16_t val, smb0en; > bus_addr_t base; > pcireg_t conf; > pci_intr_handle_t ih; > const char *intrstr = NULL; > struct i2cbus_attach_args iba; > + int numbusses, i; > > sc->sc_iot = pa->pa_iot; > + numbusses = 1; > > if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD && > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB) || > + (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD && > + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_KERNCZ_SMB) || > (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_ATI && > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_ATI_SBX00_SMB && > PCI_REVISION(pa->pa_class) >= 0x40)) { > @@ -136,10 +150,7 @@ piixpm_attach(struct device *parent, str > * hidden. We need to look at the "SMBus0En" Power > * Management register to find out where they live. > * We use indirect IO access through the index/data > - * pair at 0xcd6/0xcd7 to access "SMBus0En". Since > - * the index/data pair may be needed by other drivers, > - * we only map them for the duration that we actually > - * need them. > + * pair at 0xcd6/0xcd7 to access "SMBus0En". > */ > if (bus_space_map(sc->sc_iot, SB800_PMREG_BASE, > SB800_PMREG_SIZE, 0, &ioh) != 0) { > @@ -147,29 +158,61 @@ piixpm_attach(struct device *parent, str > return; > } > > - /* Read "SmBus0En" */ > - bus_space_write_1(sc->sc_iot, ioh, 0, SB800_PMREG_SMB0EN); > - smb0en = bus_space_read_1(sc->sc_iot, ioh, 1); > - bus_space_write_1(sc->sc_iot, ioh, 0, SB800_PMREG_SMB0EN + 1); > - smb0en |= (bus_space_read_1(sc->sc_iot, ioh, 1) << 8); > - > - bus_space_unmap(sc->sc_iot, ioh, SB800_PMREG_SIZE); > + if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD && > + (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB || > + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_KERNCZ_SMB)) { > + bus_space_write_1(sc->sc_iot, ioh, 0, > + AMDFCH41_PM_DECODE_EN); > + val = bus_space_read_1(sc->sc_iot, ioh, 1); > + smb0en = val & AMDFCH41_SMBUS_EN; > + > + bus_space_write_1(sc->sc_iot, ioh, 0, > + AMDFCH41_PM_DECODE_EN + 1); > + val = bus_space_read_1(sc->sc_iot, ioh, 1) << 8; > + base = val; > + } else { > + /* Read "SmBus0En" */ > + bus_space_write_1(sc->sc_iot, ioh, 0, > + SB800_PMREG_SMB0EN); > + val = bus_space_read_1(sc->sc_iot, ioh, 1); > + > + bus_space_write_1(sc->sc_iot, ioh, 0, > + SB800_PMREG_SMB0EN + 1); > + val |= (bus_space_read_1(sc->sc_iot, ioh, 1) << 8); > + smb0en = val & SB800_SMB0EN_EN; > + base = val & SB800_SMB0EN_BASE_MASK; > + } > > - if ((smb0en & SB800_SMB0EN_EN) == 0) { > + if (smb0en == 0) { > printf(": SMBus disabled\n"); > + bus_space_unmap(sc->sc_iot, ioh, SB800_PMREG_SIZE); > return; > } > > + sc->sc_sb800_ioh = ioh; > + if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD && > + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_KERNCZ_SMB) || > + (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD && > + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB && > + PCI_REVISION(pa->pa_class) >= 0x1F)) { > + sc->sc_is_kerncz = 1; > + numbusses = 2; > + } else { > + sc->sc_is_sb800 = 1; > + numbusses = 4; > + } > + > /* Map I/O space */ > - base = smb0en & SB800_SMB0EN_BASE_MASK; > if (base == 0 || bus_space_map(sc->sc_iot, base, > SB800_SMB_SIZE, 0, &sc->sc_ioh)) { > printf(": can't map i/o space"); > + bus_space_unmap(sc->sc_iot, ioh, SB800_PMREG_SIZE); > return; > } > > /* Read configuration */ > - conf = bus_space_read_1(sc->sc_iot, sc->sc_ioh, > SB800_SMB_HOSTC); > + conf = bus_space_read_1(sc->sc_iot, sc->sc_ioh, > + SB800_SMB_HOSTC); > if (conf & SB800_SMB_HOSTC_SMI) > conf = PIIX_SMB_HOSTC_SMI; > else > @@ -220,15 +263,19 @@ piixpm_attach(struct device *parent, str > > /* Attach I2C bus */ > rw_init(&sc->sc_i2c_lock, "iiclk"); > - sc->sc_i2c_tag.ic_cookie = sc; > - sc->sc_i2c_tag.ic_acquire_bus = piixpm_i2c_acquire_bus; > - sc->sc_i2c_tag.ic_release_bus = piixpm_i2c_release_bus; > - sc->sc_i2c_tag.ic_exec = piixpm_i2c_exec; > - > - bzero(&iba, sizeof(iba)); > - iba.iba_name = "iic"; > - iba.iba_tag = &sc->sc_i2c_tag; > - config_found(self, &iba, iicbus_print); > + for (i = 0; i < numbusses; i++) { > + sc->sc_busses[i].sb_bus = i; > + sc->sc_busses[i].sb_sc = sc; > + sc->sc_i2c_tag[i].ic_cookie = &sc->sc_busses[i]; > + sc->sc_i2c_tag[i].ic_acquire_bus = piixpm_i2c_acquire_bus; > + sc->sc_i2c_tag[i].ic_release_bus = piixpm_i2c_release_bus; > + sc->sc_i2c_tag[i].ic_exec = piixpm_i2c_exec; > + > + bzero(&iba, sizeof(iba)); > + iba.iba_name = "iic"; > + iba.iba_tag = &sc->sc_i2c_tag[i]; > + config_found(self, &iba, iicbus_print); > + } > > return; > } > @@ -236,18 +283,45 @@ piixpm_attach(struct device *parent, str > int > piixpm_i2c_acquire_bus(void *cookie, int flags) > { > - struct piixpm_softc *sc = cookie; > - > - if (cold || sc->sc_poll || (flags & I2C_F_POLL)) > - return (0); > + struct piixpm_smbus *smbus = cookie; > + struct piixpm_softc *sc = smbus->sb_sc; > + int rc = 0; > + > + if (!cold && !sc->sc_poll && !(flags & I2C_F_POLL)) > + rc = rw_enter(&sc->sc_i2c_lock, RW_WRITE | RW_INTR); > + > + if (sc->sc_is_kerncz) { > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0, > + AMDFCH41_PM_PORT_INDEX); > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, > + smbus->sb_bus << 3); > + } else if (sc->sc_is_sb800) { > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0, > + SB800_PMREG_SMB0SEL); > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, > + smbus->sb_bus << 1); > + } > > - return (rw_enter(&sc->sc_i2c_lock, RW_WRITE | RW_INTR)); > + return (rc); > } > > void > piixpm_i2c_release_bus(void *cookie, int flags) > { > - struct piixpm_softc *sc = cookie; > + struct piixpm_smbus *smbus = cookie; > + struct piixpm_softc *sc = smbus->sb_sc; > + > + if (sc->sc_is_kerncz) { > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0, > + AMDFCH41_PM_PORT_INDEX); > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, > + 0); > + } else if (sc->sc_is_sb800) { > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 0, > + SB800_PMREG_SMB0SEL); > + bus_space_write_1(sc->sc_iot, sc->sc_sb800_ioh, 1, > + 0); > + } > > if (cold || sc->sc_poll || (flags & I2C_F_POLL)) > return; > @@ -259,7 +333,8 @@ int > piixpm_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr, > const void *cmdbuf, size_t cmdlen, void *buf, size_t len, int flags) > { > - struct piixpm_softc *sc = cookie; > + struct piixpm_smbus *smbus = cookie; > + struct piixpm_softc *sc = smbus->sb_sc; > u_int8_t *b; > u_int8_t ctl, st; > int retries; > Index: piixreg.h > =================================================================== > RCS file: /cvs/src/sys/dev/pci/piixreg.h,v > retrieving revision 1.4 > diff -u -p -r1.4 piixreg.h > --- piixreg.h 28 May 2011 14:56:32 -0000 1.4 > +++ piixreg.h 16 Dec 2019 10:49:28 -0000 > @@ -69,6 +69,7 @@ > #define SB800_PMREG_BASE 0xcd6 > #define SB800_PMREG_SIZE 2 /* index/data pair */ > #define SB800_PMREG_SMB0EN 0x2c /* 16-bit register */ > +#define SB800_PMREG_SMB0SEL 0x2e /* bus selection */ > #define SB800_SMB0EN_EN 0x0001 > #define SB800_SMB0EN_BASE_MASK 0xffe0 > > @@ -76,5 +77,13 @@ > #define SB800_SMB_HOSTC_SMI (1 << 0) /* SMI */ > > #define SB800_SMB_SIZE 0x14 /* SMBus I/O space size */ > + > +/* > + * Newer FCH registers in the PMIO space. > + * See BKDG for Family 16h Models 30h-3Fh 3.26.13 PMx00 and PMx04. > + */ > +#define AMDFCH41_PM_DECODE_EN 0x00 /* 16-bit register */ > +#define AMDFCH41_PM_PORT_INDEX 0x02 > +#define AMDFCH41_SMBUS_EN 0x10 > > #endif /* !_DEV_PCI_PIIXREG_H_ */ > >