| Message ID | 1554480333-17945-1-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] hwmon: (smsc47m1) Use request_muxed_region for Super-IO accesses | expand |
On 05/04/2019 17:05, Guenter Roeck wrote: > Super-IO accesses may fail on a system with no or unmapped LPC bus. > > Also, other drivers may attempt to access the LPC bus at the same time, > resulting in undefined behavior. > > Use request_muxed_region() to ensure that IO access on the requested > address space is supported, and to ensure that access by multiple drivers > is synchronized. > > Fixes: 8d5d45fb1468 ("I2C: Move hwmon drivers (2/3)") > Reported-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Reported-by: John Garry <john.garry@huawei.com> > Cc: John Garry <john.garry@huawei.com> Acked-by: John Garry <john.garry@huawei.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Fix subject, handle error in smsc47m1_restore() > > drivers/hwmon/smsc47m1.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c > index 5d0c6eaae6f2..5f92eab24c62 100644 > --- a/drivers/hwmon/smsc47m1.c > +++ b/drivers/hwmon/smsc47m1.c > @@ -73,16 +73,21 @@ superio_inb(int reg) > /* logical device for fans is 0x0A */ > #define superio_select() superio_outb(0x07, 0x0A) > > -static inline void > +static inline int > superio_enter(void) > { > + if (!request_muxed_region(REG, 2, DRVNAME)) I note that some other drivers give an error message here. > + return -EBUSY; > + > outb(0x55, REG); > + return 0; > } Maybe this can be factored out later into a common helper, as there does seem to be a bit duplication in kernel drivers (including gpio and wdt also) But I'm not sure if it's worth it now. Thanks, > static inline void > superio_exit(void) > { > outb(0xAA, REG); > + release_region(REG, 2); > } > > #define SUPERIO_REG_ACT 0x30 > @@ -535,8 +540,12 @@ static int __init smsc47m1_find(struct smsc47m1_sio_data *sio_data) > { > u8 val; > unsigned short addr; > + int err; > + > + err = superio_enter(); > + if (err) > + return err; > > - superio_enter(); > val = force_id ? force_id : superio_inb(SUPERIO_REG_DEVID); > > /* > @@ -612,13 +621,14 @@ static int __init smsc47m1_find(struct smsc47m1_sio_data *sio_data) > static void smsc47m1_restore(const struct smsc47m1_sio_data *sio_data) > { > if ((sio_data->activate & 0x01) == 0) { > - superio_enter(); > - superio_select(); > - > - pr_info("Disabling device\n"); > - superio_outb(SUPERIO_REG_ACT, sio_data->activate); > - > - superio_exit(); > + if (!superio_enter()) { > + superio_select(); > + pr_info("Disabling device\n"); > + superio_outb(SUPERIO_REG_ACT, sio_data->activate); > + superio_exit(); > + } else { > + pr_warn("Failed to disable device\n"); > + } > } > } > >
diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c index 5d0c6eaae6f2..5f92eab24c62 100644 --- a/drivers/hwmon/smsc47m1.c +++ b/drivers/hwmon/smsc47m1.c @@ -73,16 +73,21 @@ superio_inb(int reg) /* logical device for fans is 0x0A */ #define superio_select() superio_outb(0x07, 0x0A) -static inline void +static inline int superio_enter(void) { + if (!request_muxed_region(REG, 2, DRVNAME)) + return -EBUSY; + outb(0x55, REG); + return 0; } static inline void superio_exit(void) { outb(0xAA, REG); + release_region(REG, 2); } #define SUPERIO_REG_ACT 0x30 @@ -535,8 +540,12 @@ static int __init smsc47m1_find(struct smsc47m1_sio_data *sio_data) { u8 val; unsigned short addr; + int err; + + err = superio_enter(); + if (err) + return err; - superio_enter(); val = force_id ? force_id : superio_inb(SUPERIO_REG_DEVID); /* @@ -612,13 +621,14 @@ static int __init smsc47m1_find(struct smsc47m1_sio_data *sio_data) static void smsc47m1_restore(const struct smsc47m1_sio_data *sio_data) { if ((sio_data->activate & 0x01) == 0) { - superio_enter(); - superio_select(); - - pr_info("Disabling device\n"); - superio_outb(SUPERIO_REG_ACT, sio_data->activate); - - superio_exit(); + if (!superio_enter()) { + superio_select(); + pr_info("Disabling device\n"); + superio_outb(SUPERIO_REG_ACT, sio_data->activate); + superio_exit(); + } else { + pr_warn("Failed to disable device\n"); + } } }
Super-IO accesses may fail on a system with no or unmapped LPC bus. Also, other drivers may attempt to access the LPC bus at the same time, resulting in undefined behavior. Use request_muxed_region() to ensure that IO access on the requested address space is supported, and to ensure that access by multiple drivers is synchronized. Fixes: 8d5d45fb1468 ("I2C: Move hwmon drivers (2/3)") Reported-by: Kefeng Wang <wangkefeng.wang@huawei.com> Reported-by: John Garry <john.garry@huawei.com> Cc: John Garry <john.garry@huawei.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- v2: Fix subject, handle error in smsc47m1_restore() drivers/hwmon/smsc47m1.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)