| Message ID | 20220130184130.176646-10-terry.bowman@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses | expand |
Hi Terry, On Sun, 30 Jan 2022 12:41:30 -0600, Terry Bowman wrote: > Enable EFCH MMIO using check for SMBus PCI revision ID value 0x51 or > greater. SMBus PCI revision ID 0x51 is first used by family 17h. This > PCI revision ID check will also enable future AMD processors with the > same EFCH SMBus controller HW. > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > --- > drivers/i2c/busses/i2c-piix4.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index c5325cadaf55..6a9495d994bc 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -101,6 +101,8 @@ > #define SB800_PIIX4_FCH_PM_ADDR 0xFED80300 > #define SB800_PIIX4_FCH_PM_SIZE 8 > > +#define AMD_PCI_SMBUS_REVISION_MMIO 0x51 > + I don't think that was worth a define. You only use the value once, in a context where the symbolic name doesn't add much value IMHO.
On Tue, Feb 8, 2022 at 6:33 PM Jean Delvare <jdelvare@suse.de> wrote: > On Sun, 30 Jan 2022 12:41:30 -0600, Terry Bowman wrote: ... > > +#define AMD_PCI_SMBUS_REVISION_MMIO 0x51 > > I don't think that was worth a define. You only use the value once, in > a context where the symbolic name doesn't add much value IMHO. I don't remember the code context here, but it would be nice in such a case to convert this definition to a comment (if it's not crystal clear what this magic number is about).
Hi Andy, On 2/8/22 14:10, Andy Shevchenko wrote: > On Tue, Feb 8, 2022 at 6:33 PM Jean Delvare <jdelvare@suse.de> wrote: >> On Sun, 30 Jan 2022 12:41:30 -0600, Terry Bowman wrote: > > ... > >>> +#define AMD_PCI_SMBUS_REVISION_MMIO 0x51 >> >> I don't think that was worth a define. You only use the value once, in >> a context where the symbolic name doesn't add much value IMHO. > > I don't remember the code context here, but it would be nice in such a > case to convert this definition to a comment (if it's not crystal > clear what this magic number is about). > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index 239df17ce02b..218ed8efb83e 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -229,6 +229,13 @@ static void piix4_sb800_region_release(struct device *dev, release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE); } +static bool piix4_sb800_use_mmio(struct pci_dev *PIIX4_dev) +{ + return (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && + PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS && + PIIX4_dev->revision >= AMD_PCI_SMBUS_REVISION_MMIO); +} + static int piix4_setup(struct pci_dev *PIIX4_dev, const struct pci_device_id *id) { ---- I added the context above. I'll add a comment in v5 to describe what and why 0x51 is used. Regards, Terry
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index c5325cadaf55..6a9495d994bc 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -101,6 +101,8 @@ #define SB800_PIIX4_FCH_PM_ADDR 0xFED80300 #define SB800_PIIX4_FCH_PM_SIZE 8 +#define AMD_PCI_SMBUS_REVISION_MMIO 0x51 + /* insmod parameters */ /* If force is set to anything different from 0, we forcibly enable the @@ -229,6 +231,13 @@ static void piix4_sb800_region_release(struct device *dev, release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE); } +static bool piix4_sb800_use_mmio(struct pci_dev *PIIX4_dev) +{ + return (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && + PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS && + PIIX4_dev->revision >= AMD_PCI_SMBUS_REVISION_MMIO); +} + static int piix4_setup(struct pci_dev *PIIX4_dev, const struct pci_device_id *id) { @@ -339,7 +348,7 @@ static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev, u8 smba_en_hi; int retval; - mmio_cfg.use_mmio = 0; + mmio_cfg.use_mmio = piix4_sb800_use_mmio(PIIX4_dev); retval = piix4_sb800_region_request(&PIIX4_dev->dev, &mmio_cfg); if (retval) return retval; @@ -943,7 +952,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, return -ENOMEM; } - adapdata->mmio_cfg.use_mmio = 0; + adapdata->mmio_cfg.use_mmio = piix4_sb800_use_mmio(dev); adapdata->smba = smba; adapdata->sb800_main = sb800_main; adapdata->port = port << piix4_port_shift_sb800;
Enable EFCH MMIO using check for SMBus PCI revision ID value 0x51 or greater. SMBus PCI revision ID 0x51 is first used by family 17h. This PCI revision ID check will also enable future AMD processors with the same EFCH SMBus controller HW. Signed-off-by: Terry Bowman <terry.bowman@amd.com> --- drivers/i2c/busses/i2c-piix4.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)