Re: [PATCH] i2c: piix4: Add second SMBus for X370/X470/X570

[Sebastian Reichel]

Hi,

On Tue, Apr 14, 2020 at 09:41:58AM +0200, Jean Delvare wrote:
> On Mon, 13 Apr 2020 17:06:34 +0200, Sebastian Reichel wrote:
> > The second interface can be found on X370, X470 and X570 according
> > to the bugzilla entry. I only tested with X570 on an ASRock X570
> > Taichi:
> > 
> > $ lspci -nnv -d 1022:790b
> > 00:14.0 SMBus [0c05]: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller [1022:790b] (rev 61)
> > 	Subsystem: ASRock Incorporation FCH SMBus Controller [1849:ffff]
> > 	Flags: 66MHz, medium devsel
> > 	Kernel driver in use: piix4_smbus
> > 	Kernel modules: i2c_piix4, sp5100_tco
> > 
> > Before the patch:
> > 
> > $ i2cdetect -l | grep PIIX4
> > i2c-1	unknown   	SMBus PIIX4 adapter port 2 at 0b00	N/A
> > i2c-0	unknown   	SMBus PIIX4 adapter port 0 at 0b00	N/A
> > 
> > After the patch:
> > 
> > $ i2cdetect -l | grep PIIX4
> > i2c-1	unknown   	SMBus PIIX4 adapter port 2 at 0b00	N/A
> > i2c-2	unknown   	SMBus PIIX4 adapter port 1 at 0b20	N/A
> > i2c-0	unknown   	SMBus PIIX4 adapter port 0 at 0b00	N/A
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202587
> > Reported-by: Adam Honse <calcprogrammer1@xxxxxxxxx>
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > ---
> > Hi,
> > 
> > The Bugzilla entry contains a second change, which reduces the timeouts
> > for quicker operation. I did not include that change, since I do not know
> > if this is a good idea for all devices supported by this driver. In any
> > case it should be a separate patch. Let's get the interface working for
> > now.
> > 
> > -- Sebastian
> > ---
> >  drivers/i2c/busses/i2c-piix4.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> > index 30ded6422e7b..3e89143a0ecf 100644
> > --- a/drivers/i2c/busses/i2c-piix4.c
> > +++ b/drivers/i2c/busses/i2c-piix4.c
> > @@ -976,9 +976,11 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  		}
> >  	}
> >  
> > -	if (dev->vendor == PCI_VENDOR_ID_AMD &&
> > -	    dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS) {
> > -		retval = piix4_setup_sb800(dev, id, 1);
> > +	if (dev->vendor == PCI_VENDOR_ID_AMD) {
> > +		if (dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS ||
> > +		    dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) {
> > +			retval = piix4_setup_sb800(dev, id, 1);
> > +		}
> >  	}
> >  
> >  	if (retval > 0) {
> 
> Adam just sent pretty much the same patch:
> 
> https://patchwork.ozlabs.org/project/linux-i2c/patch/20200410204843.3856-1-calcprogrammer1@xxxxxxxxx/
> 
> Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>
> 
> I'll leave it to Wolfram which one he wants to apply.

Ok, I wasn't aware of that and thought this patch was forgotten. In
that case Adam should receive all the credits of course. Feel free
to take over anything usable from my commit message. The patch from
Adam is

Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
Tested-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>

Thanks,

-- Sebastian

Attachment: signature.asc
Description: PGP signature