| Message ID | 20171207122545.1b8e17c6@endymion |
|---|---|
| State | Accepted |
| Headers | show |
| Series | i2c: piix4: Fix port number check on release | expand |
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); > >
> 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?
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?
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!
--- 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);
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(-)