Re: [PATCH v2] i2c: piix4: Support alternative port selection register

[fetzerch <fetzer.ch@gmail.com>]

Hi Jean,

On 17.02.2016 10:26, Jean Delvare wrote:
> The SB800 register reference guide says that the SMBus port selection
> bits may not always be in register Smbus0En (0x2c) but could
> alternatively be found in register Smbus0Sel (0x2e) depending on the
> settings in register Smbus0SelEn (0x2f.) Add support for this
> configuration.
> 
> The "alternative" register is the only one working for the Bolton
> (aka Hudson-2) chipset anyway. I do not have any documentation for
> the "kerncz" chipset so we treat it the same as the Bolton for now.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Tested-by: Jean Delvare <jdelvare@suse.de>
> Tested-by: Christian Fetzer <fetzer.ch@gmail.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> ---
> Changes since v1:
>  * Hard code selection register to 0x2f for Bolton and later.
> 
> Note: Christian's Tested-by and Mika's Reviewed-by actually apply to v1
> of this patch, but the changes are minor enough that I considered they
> still apply. That doesn't mean I wouldn't welcome testing and reviewing
> of v2 ;-)
> 

Done. No noticeable changes. Still works as expected.

Cheers,
Christian

>  drivers/i2c/busses/i2c-piix4.c |   36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> --- linux-4.5-rc4.orig/drivers/i2c/busses/i2c-piix4.c	2016-02-17 09:15:11.822268210 +0100
> +++ linux-4.5-rc4/drivers/i2c/busses/i2c-piix4.c	2016-02-17 10:21:15.021737695 +0100
> @@ -85,8 +85,14 @@
>  /* SB800 constants */
>  #define SB800_PIIX4_SMB_IDX		0xcd6
>  
> -/* SB800 port is selected by bits 2:1 of the smb_en register (0x2c) */
> +/*
> + * SB800 port is selected by bits 2:1 of the smb_en register (0x2c)
> + * or the smb_sel register (0x2e), depending on bit 0 of register 0x2f.
> + * Hudson-2/Bolton port is always selected by bits 2:1 of register 0x2f.
> + */
>  #define SB800_PIIX4_PORT_IDX		0x2c
> +#define SB800_PIIX4_PORT_IDX_ALT	0x2e
> +#define SB800_PIIX4_PORT_IDX_SEL	0x2f
>  #define SB800_PIIX4_PORT_IDX_MASK	0x06
>  
>  /* insmod parameters */
> @@ -136,8 +142,13 @@ static const struct dmi_system_id piix4_
>  	{ },
>  };
>  
> -/* SB800 globals */
> +/*
> + * SB800 globals
> + * piix4_mutex_sb800 protects piix4_port_sel_sb800 and the pair
> + * of I/O ports at SB800_PIIX4_SMB_IDX.
> + */
>  static DEFINE_MUTEX(piix4_mutex_sb800);
> +static u8 piix4_port_sel_sb800;
>  static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
>  	" port 0", " port 2", " port 3", " port 4"
>  };
> @@ -254,7 +265,7 @@ static int piix4_setup_sb800(struct pci_
>  			     const struct pci_device_id *id, u8 aux)
>  {
>  	unsigned short piix4_smba;
> -	u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status;
> +	u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status, port_sel;
>  	u8 i2ccfg, i2ccfg_offset = 0x10;
>  
>  	/* SB800 and later SMBus does not support forcing address */
> @@ -334,6 +345,23 @@ static int piix4_setup_sb800(struct pci_
>  		 "SMBus Host Controller at 0x%x, revision %d\n",
>  		 piix4_smba, i2ccfg >> 4);
>  
> +	/* 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;
> +	} else {
> +		mutex_lock(&piix4_mutex_sb800);
> +		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
> +		port_sel = inb_p(SB800_PIIX4_SMB_IDX + 1);
> +		piix4_port_sel_sb800 = (port_sel & 0x01) ?
> +				       SB800_PIIX4_PORT_IDX_ALT :
> +				       SB800_PIIX4_PORT_IDX;
> +		mutex_unlock(&piix4_mutex_sb800);
> +	}
> +
> +	dev_info(&PIIX4_dev->dev,
> +		 "Using register 0x%02x for SMBus port selection\n",
> +		 (unsigned int)piix4_port_sel_sb800);
> +
>  	return piix4_smba;
>  }
>  
> @@ -563,7 +591,7 @@ static s32 piix4_access_sb800(struct i2c
>  
>  	mutex_lock(&piix4_mutex_sb800);
>  
> -	outb_p(SB800_PIIX4_PORT_IDX, SB800_PIIX4_SMB_IDX);
> +	outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
>  	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
>  
>  	port = adapdata->port;
>