[PATCH] i2c: piix4: Fix port number check on release

[PATCH] i2c: piix4: Fix port number check on release

[Jean Delvare]

The port number shift is still hard-coded to 1 while it now depends
on the hardware.

Thankfully 0 is always 0 no matter how you shift it, so this was a
bug without consequences.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Fixes: 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Wolfram Sang <wsa@the-dreams.de>
---
Sorry for missing this while reviewing the original patch.

 drivers/i2c/busses/i2c-piix4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-4.14.orig/drivers/i2c/busses/i2c-piix4.c	2017-12-07 11:06:05.198260340 +0100
+++ linux-4.14/drivers/i2c/busses/i2c-piix4.c	2017-12-07 12:18:56.405907856 +0100
@@ -989,7 +989,7 @@ static void piix4_adap_remove(struct i2c
 
 	if (adapdata->smba) {
 		i2c_del_adapter(adap);
-		if (adapdata->port == (0 << 1)) {
+		if (adapdata->port == (0 << piix4_port_shift_sb800)) {
 			release_region(adapdata->smba, SMBIOSIZE);
 			if (adapdata->sb800_main)
 				release_region(SB800_PIIX4_SMB_IDX, 2);


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: piix4: Fix port number check on release

[Guenter Roeck]

On 12/07/2017 03:25 AM, Jean Delvare wrote:
> The port number shift is still hard-coded to 1 while it now depends
> on the hardware.
> 
> Thankfully 0 is always 0 no matter how you shift it, so this was a
> bug without consequences.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Fixes: 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> ---
> Sorry for missing this while reviewing the original patch.
> 

Sorry for missing it.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

>   drivers/i2c/busses/i2c-piix4.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-4.14.orig/drivers/i2c/busses/i2c-piix4.c	2017-12-07 11:06:05.198260340 +0100
> +++ linux-4.14/drivers/i2c/busses/i2c-piix4.c	2017-12-07 12:18:56.405907856 +0100
> @@ -989,7 +989,7 @@ static void piix4_adap_remove(struct i2c
>   
>   	if (adapdata->smba) {
>   		i2c_del_adapter(adap);
> -		if (adapdata->port == (0 << 1)) {
> +		if (adapdata->port == (0 << piix4_port_shift_sb800)) {
>   			release_region(adapdata->smba, SMBIOSIZE);
>   			if (adapdata->sb800_main)
>   				release_region(SB800_PIIX4_SMB_IDX, 2);
> 
> 

Re: [PATCH] i2c: piix4: Fix port number check on release

[Wolfram Sang]

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


> Thankfully 0 is always 0 no matter how you shift it, so this was a
> bug without consequences.

So, why do we shift at all?


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

Re: [PATCH] i2c: piix4: Fix port number check on release

[Jean Delvare]

On Thu, 7 Dec 2017 15:33:48 +0100, Wolfram Sang wrote:
> > Thankfully 0 is always 0 no matter how you shift it, so this was a
> > bug without consequences.  
> 
> So, why do we shift at all?

You are joking, right?

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: piix4: Fix port number check on release

[Wolfram Sang]

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

On Thu, Dec 07, 2017 at 12:25:45PM +0100, Jean Delvare wrote:
> The port number shift is still hard-coded to 1 while it now depends
> on the hardware.
> 
> Thankfully 0 is always 0 no matter how you shift it, so this was a
> bug without consequences.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Fixes: 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Wolfram Sang <wsa@the-dreams.de>

I still think it is weird, but applied to for-current nonetheless,
thanks!


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