All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
@ 2017-07-15 23:51 Guenter Roeck
  2017-08-12 11:33 ` Wolfram Sang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-07-15 23:51 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Wolfram Sang, linux-i2c, linux-kernel, Guenter Roeck

AMD Family 17h uses the KERNCZ SMBus controller. While its documentation
is not publicly available, it is documented in the BIOS and Kernel
Developer’s Guide for AMD Family 15h Models 60h-6Fh Processors.

On this SMBus controller, the port select register is at PMx register
0x02, bit 4:3 (PMx00 register bit 20:19).

Without this patch, the 4 SMBus channels on AMD Family 17h chips are
mirrored and report the same chips on all channels.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/i2c/busses/i2c-piix4.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 0ecdb47a23ab..01f767ee4546 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -94,6 +94,12 @@
 #define SB800_PIIX4_PORT_IDX_ALT	0x2e
 #define SB800_PIIX4_PORT_IDX_SEL	0x2f
 #define SB800_PIIX4_PORT_IDX_MASK	0x06
+#define SB800_PIIX4_PORT_IDX_SHIFT	1
+
+/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
+#define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
+#define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
+#define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
 
 /* insmod parameters */
 
@@ -149,6 +155,8 @@ static const struct dmi_system_id piix4_dmi_ibm[] = {
  */
 static DEFINE_MUTEX(piix4_mutex_sb800);
 static u8 piix4_port_sel_sb800;
+static u8 piix4_port_mask_sb800;
+static u8 piix4_port_shift_sb800;
 static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
 	" port 0", " port 2", " port 3", " port 4"
 };
@@ -347,7 +355,19 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 
 	/* Find which register is used for port selection */
 	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD) {
-		piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
+		switch (PIIX4_dev->device) {
+		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
+			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_KERNCZ;
+			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK_KERNCZ;
+			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ;
+			break;
+		case PCI_DEVICE_ID_AMD_HUDSON2_SMBUS:
+		default:
+			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
+			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
+			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
+			break;
+		}
 	} else {
 		mutex_lock(&piix4_mutex_sb800);
 		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
@@ -355,6 +375,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 		piix4_port_sel_sb800 = (port_sel & 0x01) ?
 				       SB800_PIIX4_PORT_IDX_ALT :
 				       SB800_PIIX4_PORT_IDX;
+		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
+		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		mutex_unlock(&piix4_mutex_sb800);
 	}
 
@@ -616,8 +638,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
 
 	port = adapdata->port;
-	if ((smba_en_lo & SB800_PIIX4_PORT_IDX_MASK) != port)
-		outb_p((smba_en_lo & ~SB800_PIIX4_PORT_IDX_MASK) | port,
+	if ((smba_en_lo & piix4_port_mask_sb800) != port)
+		outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port,
 		       SB800_PIIX4_SMB_IDX + 1);
 
 	retval = piix4_access(adap, addr, flags, read_write,
@@ -706,7 +728,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 
 	adapdata->smba = smba;
 	adapdata->sb800_main = sb800_main;
-	adapdata->port = port << 1;
+	adapdata->port = port << piix4_port_shift_sb800;
 
 	/* set up the sysfs linkage to our parent device */
 	adap->dev.parent = &dev->dev;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-07-15 23:51 [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips Guenter Roeck
@ 2017-08-12 11:33 ` Wolfram Sang
  2017-09-22 22:41   ` Guenter Roeck
  2017-10-05 14:33   ` Jean Delvare
  2017-10-05 18:34 ` Rudolf Marek
  2017-10-13 19:01 ` Wolfram Sang
  2 siblings, 2 replies; 9+ messages in thread
From: Wolfram Sang @ 2017-08-12 11:33 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3937 bytes --]

On Sat, Jul 15, 2017 at 04:51:26PM -0700, Guenter Roeck wrote:
> AMD Family 17h uses the KERNCZ SMBus controller. While its documentation
> is not publicly available, it is documented in the BIOS and Kernel
> Developer’s Guide for AMD Family 15h Models 60h-6Fh Processors.
> 
> On this SMBus controller, the port select register is at PMx register
> 0x02, bit 4:3 (PMx00 register bit 20:19).
> 
> Without this patch, the 4 SMBus channels on AMD Family 17h chips are
> mirrored and report the same chips on all channels.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

I need input from Jean for this one.

> ---
>  drivers/i2c/busses/i2c-piix4.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 0ecdb47a23ab..01f767ee4546 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -94,6 +94,12 @@
>  #define SB800_PIIX4_PORT_IDX_ALT	0x2e
>  #define SB800_PIIX4_PORT_IDX_SEL	0x2f
>  #define SB800_PIIX4_PORT_IDX_MASK	0x06
> +#define SB800_PIIX4_PORT_IDX_SHIFT	1
> +
> +/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
> +#define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
> +#define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
> +#define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
>  
>  /* insmod parameters */
>  
> @@ -149,6 +155,8 @@ static const struct dmi_system_id piix4_dmi_ibm[] = {
>   */
>  static DEFINE_MUTEX(piix4_mutex_sb800);
>  static u8 piix4_port_sel_sb800;
> +static u8 piix4_port_mask_sb800;
> +static u8 piix4_port_shift_sb800;
>  static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
>  	" port 0", " port 2", " port 3", " port 4"
>  };
> @@ -347,7 +355,19 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  
>  	/* Find which register is used for port selection */
>  	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD) {
> -		piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
> +		switch (PIIX4_dev->device) {
> +		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
> +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_KERNCZ;
> +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK_KERNCZ;
> +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ;
> +			break;
> +		case PCI_DEVICE_ID_AMD_HUDSON2_SMBUS:
> +		default:
> +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
> +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
> +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
> +			break;
> +		}
>  	} else {
>  		mutex_lock(&piix4_mutex_sb800);
>  		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
> @@ -355,6 +375,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  		piix4_port_sel_sb800 = (port_sel & 0x01) ?
>  				       SB800_PIIX4_PORT_IDX_ALT :
>  				       SB800_PIIX4_PORT_IDX;
> +		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
> +		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
>  		mutex_unlock(&piix4_mutex_sb800);
>  	}
>  
> @@ -616,8 +638,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
>  
>  	port = adapdata->port;
> -	if ((smba_en_lo & SB800_PIIX4_PORT_IDX_MASK) != port)
> -		outb_p((smba_en_lo & ~SB800_PIIX4_PORT_IDX_MASK) | port,
> +	if ((smba_en_lo & piix4_port_mask_sb800) != port)
> +		outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port,
>  		       SB800_PIIX4_SMB_IDX + 1);
>  
>  	retval = piix4_access(adap, addr, flags, read_write,
> @@ -706,7 +728,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>  
>  	adapdata->smba = smba;
>  	adapdata->sb800_main = sb800_main;
> -	adapdata->port = port << 1;
> +	adapdata->port = port << piix4_port_shift_sb800;
>  
>  	/* set up the sysfs linkage to our parent device */
>  	adap->dev.parent = &dev->dev;
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-08-12 11:33 ` Wolfram Sang
@ 2017-09-22 22:41   ` Guenter Roeck
  2017-10-05 14:33   ` Jean Delvare
  1 sibling, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-09-22 22:41 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Jean Delvare, linux-i2c, linux-kernel

On 08/12/2017 04:33 AM, Wolfram Sang wrote:
> On Sat, Jul 15, 2017 at 04:51:26PM -0700, Guenter Roeck wrote:
>> AMD Family 17h uses the KERNCZ SMBus controller. While its documentation
>> is not publicly available, it is documented in the BIOS and Kernel
>> Developer’s Guide for AMD Family 15h Models 60h-6Fh Processors.
>>
>> On this SMBus controller, the port select register is at PMx register
>> 0x02, bit 4:3 (PMx00 register bit 20:19).
>>
>> Without this patch, the 4 SMBus channels on AMD Family 17h chips are
>> mirrored and report the same chips on all channels.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> I need input from Jean for this one.
> 
I am getting feedback from others that both Ryzen and Threadripper CPUs are affected.
It would be great if we can find a means to move this forward or find a better fix.

Thanks,
Guenter

>> ---
>>   drivers/i2c/busses/i2c-piix4.c | 30 ++++++++++++++++++++++++++----
>>   1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
>> index 0ecdb47a23ab..01f767ee4546 100644
>> --- a/drivers/i2c/busses/i2c-piix4.c
>> +++ b/drivers/i2c/busses/i2c-piix4.c
>> @@ -94,6 +94,12 @@
>>   #define SB800_PIIX4_PORT_IDX_ALT	0x2e
>>   #define SB800_PIIX4_PORT_IDX_SEL	0x2f
>>   #define SB800_PIIX4_PORT_IDX_MASK	0x06
>> +#define SB800_PIIX4_PORT_IDX_SHIFT	1
>> +
>> +/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
>> +#define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
>> +#define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
>> +#define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
>>   
>>   /* insmod parameters */
>>   
>> @@ -149,6 +155,8 @@ static const struct dmi_system_id piix4_dmi_ibm[] = {
>>    */
>>   static DEFINE_MUTEX(piix4_mutex_sb800);
>>   static u8 piix4_port_sel_sb800;
>> +static u8 piix4_port_mask_sb800;
>> +static u8 piix4_port_shift_sb800;
>>   static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
>>   	" port 0", " port 2", " port 3", " port 4"
>>   };
>> @@ -347,7 +355,19 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>>   
>>   	/* Find which register is used for port selection */
>>   	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD) {
>> -		piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
>> +		switch (PIIX4_dev->device) {
>> +		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
>> +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_KERNCZ;
>> +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK_KERNCZ;
>> +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ;
>> +			break;
>> +		case PCI_DEVICE_ID_AMD_HUDSON2_SMBUS:
>> +		default:
>> +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
>> +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
>> +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
>> +			break;
>> +		}
>>   	} else {
>>   		mutex_lock(&piix4_mutex_sb800);
>>   		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
>> @@ -355,6 +375,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>>   		piix4_port_sel_sb800 = (port_sel & 0x01) ?
>>   				       SB800_PIIX4_PORT_IDX_ALT :
>>   				       SB800_PIIX4_PORT_IDX;
>> +		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
>> +		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
>>   		mutex_unlock(&piix4_mutex_sb800);
>>   	}
>>   
>> @@ -616,8 +638,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>>   	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
>>   
>>   	port = adapdata->port;
>> -	if ((smba_en_lo & SB800_PIIX4_PORT_IDX_MASK) != port)
>> -		outb_p((smba_en_lo & ~SB800_PIIX4_PORT_IDX_MASK) | port,
>> +	if ((smba_en_lo & piix4_port_mask_sb800) != port)
>> +		outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port,
>>   		       SB800_PIIX4_SMB_IDX + 1);
>>   
>>   	retval = piix4_access(adap, addr, flags, read_write,
>> @@ -706,7 +728,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>>   
>>   	adapdata->smba = smba;
>>   	adapdata->sb800_main = sb800_main;
>> -	adapdata->port = port << 1;
>> +	adapdata->port = port << piix4_port_shift_sb800;
>>   
>>   	/* set up the sysfs linkage to our parent device */
>>   	adap->dev.parent = &dev->dev;
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-08-12 11:33 ` Wolfram Sang
  2017-09-22 22:41   ` Guenter Roeck
@ 2017-10-05 14:33   ` Jean Delvare
  2017-10-05 15:11     ` Wolfram Sang
  1 sibling, 1 reply; 9+ messages in thread
From: Jean Delvare @ 2017-10-05 14:33 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Guenter Roeck, linux-i2c, linux-kernel

On Sat, 12 Aug 2017 13:33:57 +0200, Wolfram Sang wrote:
> On Sat, Jul 15, 2017 at 04:51:26PM -0700, Guenter Roeck wrote:
> > AMD Family 17h uses the KERNCZ SMBus controller. While its documentation
> > is not publicly available, it is documented in the BIOS and Kernel
> > Developer’s Guide for AMD Family 15h Models 60h-6Fh Processors.
> > 
> > On this SMBus controller, the port select register is at PMx register
> > 0x02, bit 4:3 (PMx00 register bit 20:19).
> > 
> > Without this patch, the 4 SMBus channels on AMD Family 17h chips are
> > mirrored and report the same chips on all channels.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>  
> 
> I need input from Jean for this one.
> 
> > ---
> >  drivers/i2c/busses/i2c-piix4.c | 30 ++++++++++++++++++++++++++----
> >  1 file changed, 26 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> > index 0ecdb47a23ab..01f767ee4546 100644
> > --- a/drivers/i2c/busses/i2c-piix4.c
> > +++ b/drivers/i2c/busses/i2c-piix4.c
> > @@ -94,6 +94,12 @@
> >  #define SB800_PIIX4_PORT_IDX_ALT	0x2e
> >  #define SB800_PIIX4_PORT_IDX_SEL	0x2f
> >  #define SB800_PIIX4_PORT_IDX_MASK	0x06
> > +#define SB800_PIIX4_PORT_IDX_SHIFT	1
> > +
> > +/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
> > +#define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
> > +#define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
> > +#define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
> >  
> >  /* insmod parameters */
> >  
> > @@ -149,6 +155,8 @@ static const struct dmi_system_id piix4_dmi_ibm[] = {
> >   */
> >  static DEFINE_MUTEX(piix4_mutex_sb800);
> >  static u8 piix4_port_sel_sb800;
> > +static u8 piix4_port_mask_sb800;
> > +static u8 piix4_port_shift_sb800;
> >  static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
> >  	" port 0", " port 2", " port 3", " port 4"
> >  };
> > @@ -347,7 +355,19 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
> >  
> >  	/* Find which register is used for port selection */
> >  	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD) {
> > -		piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
> > +		switch (PIIX4_dev->device) {
> > +		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
> > +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_KERNCZ;
> > +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK_KERNCZ;
> > +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ;
> > +			break;
> > +		case PCI_DEVICE_ID_AMD_HUDSON2_SMBUS:
> > +		default:
> > +			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
> > +			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
> > +			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
> > +			break;
> > +		}
> >  	} else {
> >  		mutex_lock(&piix4_mutex_sb800);
> >  		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
> > @@ -355,6 +375,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
> >  		piix4_port_sel_sb800 = (port_sel & 0x01) ?
> >  				       SB800_PIIX4_PORT_IDX_ALT :
> >  				       SB800_PIIX4_PORT_IDX;
> > +		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
> > +		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
> >  		mutex_unlock(&piix4_mutex_sb800);
> >  	}
> >  
> > @@ -616,8 +638,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> >  	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
> >  
> >  	port = adapdata->port;
> > -	if ((smba_en_lo & SB800_PIIX4_PORT_IDX_MASK) != port)
> > -		outb_p((smba_en_lo & ~SB800_PIIX4_PORT_IDX_MASK) | port,
> > +	if ((smba_en_lo & piix4_port_mask_sb800) != port)
> > +		outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port,
> >  		       SB800_PIIX4_SMB_IDX + 1);
> >  
> >  	retval = piix4_access(adap, addr, flags, read_write,
> > @@ -706,7 +728,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
> >  
> >  	adapdata->smba = smba;
> >  	adapdata->sb800_main = sb800_main;
> > -	adapdata->port = port << 1;
> > +	adapdata->port = port << piix4_port_shift_sb800;
> >  
> >  	/* set up the sysfs linkage to our parent device */
> >  	adap->dev.parent = &dev->dev;

Sorry for the delay. Looks all good to me (as I'd expect from Gunter...)

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-10-05 14:33   ` Jean Delvare
@ 2017-10-05 15:11     ` Wolfram Sang
  2017-10-05 17:43       ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2017-10-05 15:11 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Guenter Roeck, linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]


> Sorry for the delay. Looks all good to me (as I'd expect from Gunter...)
> 
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Thanks, Jean!

Guenter: I will queue it to for-current, when I am back home. Does it
need a stable tag?

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-10-05 15:11     ` Wolfram Sang
@ 2017-10-05 17:43       ` Guenter Roeck
  0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-10-05 17:43 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Jean Delvare, linux-i2c, linux-kernel

On Thu, Oct 05, 2017 at 05:11:15PM +0200, Wolfram Sang wrote:
> 
> > Sorry for the delay. Looks all good to me (as I'd expect from Gunter...)
> > 
> > Reviewed-by: Jean Delvare <jdelvare@suse.de>
> 
> Thanks, Jean!
> 
> Guenter: I will queue it to for-current, when I am back home. Does it
> need a stable tag?
> 
I would suggest yes, since otherwise anyone using a Ryzen or Threadripper CPU
will see duplicate i2c controllers and devices. Some DDR4 chips have temperature
sensors, so this becomes easily visible (this is how I found the problem).

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-07-15 23:51 [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips Guenter Roeck
  2017-08-12 11:33 ` Wolfram Sang
@ 2017-10-05 18:34 ` Rudolf Marek
  2017-10-06 17:18   ` Guenter Roeck
  2017-10-13 19:01 ` Wolfram Sang
  2 siblings, 1 reply; 9+ messages in thread
From: Rudolf Marek @ 2017-10-05 18:34 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Wolfram Sang, linux-i2c, linux-kernel

Hi Guys,

Even in "AMD Family 15h Models 60h-6Fh Processors" [1]
are ports3 and ports4 marked as "Reserved". Maybe we should limit "KERN" to 2 ports?

Thanks
Rudolf

[1] http://support.amd.com/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-10-05 18:34 ` Rudolf Marek
@ 2017-10-06 17:18   ` Guenter Roeck
  0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-10-06 17:18 UTC (permalink / raw)
  To: Rudolf Marek; +Cc: Jean Delvare, Wolfram Sang, linux-i2c, linux-kernel

On Thu, Oct 05, 2017 at 08:34:01PM +0200, Rudolf Marek wrote:
> Hi Guys,
> 
> Even in "AMD Family 15h Models 60h-6Fh Processors" [1]
> are ports3 and ports4 marked as "Reserved". Maybe we should limit "KERN" to 2 ports?
> 

Maybe, if we are sure that this applies to all CPU models supported
by the driver. That should be a separate patch, though. 

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips
  2017-07-15 23:51 [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips Guenter Roeck
  2017-08-12 11:33 ` Wolfram Sang
  2017-10-05 18:34 ` Rudolf Marek
@ 2017-10-13 19:01 ` Wolfram Sang
  2 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2017-10-13 19:01 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On Sat, Jul 15, 2017 at 04:51:26PM -0700, Guenter Roeck wrote:
> AMD Family 17h uses the KERNCZ SMBus controller. While its documentation
> is not publicly available, it is documented in the BIOS and Kernel
> Developer’s Guide for AMD Family 15h Models 60h-6Fh Processors.
> 
> On this SMBus controller, the port select register is at PMx register
> 0x02, bit 4:3 (PMx00 register bit 20:19).
> 
> Without this patch, the 4 SMBus channels on AMD Family 17h chips are
> mirrored and report the same chips on all channels.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-10-13 19:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-15 23:51 [PATCH] i2c: piix4: Fix SMBus port selection for AMD Family 17h chips Guenter Roeck
2017-08-12 11:33 ` Wolfram Sang
2017-09-22 22:41   ` Guenter Roeck
2017-10-05 14:33   ` Jean Delvare
2017-10-05 15:11     ` Wolfram Sang
2017-10-05 17:43       ` Guenter Roeck
2017-10-05 18:34 ` Rudolf Marek
2017-10-06 17:18   ` Guenter Roeck
2017-10-13 19:01 ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.