i2c: piix4: Request the SMBUS semaphore inside the mutex

SMBSLVCNT must be protected with the piix4_mutex_sb800 in order to avoid
multiple buses accessing to the semaphore at the same time.

Reported-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/i2c/busses/i2c-piix4.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Hi Ricardo,

On Thu,  2 Feb 2017 20:15:16 +0100, Ricardo Ribalda Delgado wrote:
> SMBSLVCNT must be protected with the piix4_mutex_sb800 in order to avoid
> multiple buses accessing to the semaphore at the same time.
> 
> Reported-by: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/i2c/busses/i2c-piix4.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 73cc6799cc59..c21ca7bf2efe 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -592,6 +592,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  	u8 port;
>  	int retval;
>  
> +	mutex_lock(&piix4_mutex_sb800);
> +
>  	/* Request the SMBUS semaphore, avoid conflicts with the IMC */
>  	smbslvcnt  = inb_p(SMBSLVCNT);
>  	do {
> @@ -605,10 +607,10 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  		usleep_range(1000, 2000);
>  	} while (--retries);
>  	/* SMBus is still owned by the IMC, we give up */
> -	if (!retries)
> +	if (!retries) {
> +		mutex_unlock(&piix4_mutex_sb800);
>  		return -EBUSY;
> -
> -	mutex_lock(&piix4_mutex_sb800);
> +	}
>  
>  	outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
>  	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
> @@ -623,11 +625,11 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  
>  	outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1);
>  
> -	mutex_unlock(&piix4_mutex_sb800);
> -
>  	/* Release the semaphore */
>  	outb_p(smbslvcnt | 0x20, SMBSLVCNT);
>  
> +	mutex_unlock(&piix4_mutex_sb800);
> +
>  	return retval;
>  }
>  

Thanks for the quick fix.

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

Jean,

> Thanks for the quick fix.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>

I wonder: why do you use Signed-off instead of Reviewed-by recently?

On Thu, Feb 02, 2017 at 08:15:16PM +0100, Ricardo Ribalda Delgado wrote:
> SMBSLVCNT must be protected with the piix4_mutex_sb800 in order to avoid
> multiple buses accessing to the semaphore at the same time.
> 
> Reported-by: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Applied to for-current and added the same fixes tag as the patch to the
resource size, thanks!

Hi Wolfram,

On Thu, 9 Feb 2017 17:09:26 +0100, Wolfram Sang wrote:
> > Thanks for the quick fix.
> > 
> > Signed-off-by: Jean Delvare <jdelvare@suse.de>
> 
> I wonder: why do you use Signed-off instead of Reviewed-by recently?

I use Signed-off-by for drivers I maintain, and Reviewed-by for the
rest.

But I can use whatever you prefer if that's a problem, I don't really
care.

> I use Signed-off-by for drivers I maintain, and Reviewed-by for the
> rest.

I see. I never noticed so far :)

> But I can use whatever you prefer if that's a problem, I don't really
> care.

Patchwork collects Rev-by tags from threads and shows the number of tags
in a list. This is helpful for me because I want to process already
reviewed patches earlier.

So, Rev-by would in deed be much appreciated for a better workflow.

Thanks!