Skip to contentSkip to conversation

i2c: piix4: Fix port number check on release

PATCH In review Jean Delvare 3 participants 5 messages Dec 7, 2017 – Dec 12, 2017

Subsystems (2)
I2C subsystem host drivers I2C/smbus controller drivers for pc
lore
Share & tools
lore

Jean Delvare

author
#0 lore raw

PATCH 1/1 i2c: piix4: Fix port number check on release

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 <[email protected]>
Fixes 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
Cc Guenter Roeck <[email protected]>
Cc Wolfram Sang <[email protected]>

---
Sorry for missing this while reviewing the original patch.

 drivers/i2c/busses/i2c-piix4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-piix4.c +1 -1
@@ -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);
Signature · 2 lines
Jean Delvare
SUSE L3 Support
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · 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.

Quoted 7 lines
Signed-off-by Jean Delvare <[email protected]>
Fixes 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
Cc Guenter Roeck <[email protected]>
Cc Wolfram Sang <[email protected]>

---
Sorry for missing this while reviewing the original patch.

Sorry for missing it.

Reviewed-by Guenter Roeck <[email protected]>
Quoted 16 lines
  drivers/i2c/busses/i2c-piix4.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-piix4.c +1 -1
@@ -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);
author
#3 lore raw
Quoted 4 lines · Wolfram Sang
Quoted 2 lines

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?

Signature · 2 lines
Jean Delvare
SUSE L3 Support
#4 lore raw
In reply to Jean Delvare
Quoted 10 lines · 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 <[email protected]>
Fixes 0fe16195f891 ("i2c: piix4: Fix SMBus port selection for AMD Family 17h chips")
Cc Guenter Roeck <[email protected]>
Cc Wolfram Sang <[email protected]>

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