[PATCH 1/4] hwmon: (f71805f) Use request_muxed_region for Super-IO accesses

[PATCH 1/4] hwmon: (f71805f) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

Super-IO accesses may fail on a system with no or unmapped LPC bus.

Unable to handle kernel paging request at virtual address ffffffbffee0002e
pgd = ffffffc1d68d4000
[ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
Internal error: Oops: 94000046 [#1] PREEMPT SMP
Modules linked in: f71805f(+) hwmon
CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
Hardware name: linux,dummy-virt (DT)
task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
PC is at f71805f_find+0x6c/0x358 [f71805f]

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: e53004e20a58e ("hwmon: New f71805f driver")
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>
---
 drivers/hwmon/f71805f.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/f71805f.c b/drivers/hwmon/f71805f.c
index 73c681162653..623736d2a7c1 100644
--- a/drivers/hwmon/f71805f.c
+++ b/drivers/hwmon/f71805f.c
@@ -96,17 +96,23 @@ superio_select(int base, int ld)
 	outb(ld, base + 1);
 }
 
-static inline void
+static inline int
 superio_enter(int base)
 {
+	if (!request_muxed_region(base, 2, DRVNAME))
+		return -EBUSY;
+
 	outb(0x87, base);
 	outb(0x87, base);
+
+	return 0;
 }
 
 static inline void
 superio_exit(int base)
 {
 	outb(0xaa, base);
+	release_region(base, 2);
 }
 
 /*
@@ -1561,7 +1567,7 @@ static int __init f71805f_device_add(unsigned short address,
 static int __init f71805f_find(int sioaddr, unsigned short *address,
 			       struct f71805f_sio_data *sio_data)
 {
-	int err = -ENODEV;
+	int err;
 	u16 devid;
 
 	static const char * const names[] = {
@@ -1569,8 +1575,11 @@ static int __init f71805f_find(int sioaddr, unsigned short *address,
 		"F71872F/FG or F71806F/FG",
 	};
 
-	superio_enter(sioaddr);
+	err = superio_enter(sioaddr);
+	if (err)
+		return err;
 
+	err = -ENODEV;
 	devid = superio_inw(sioaddr, SIO_REG_MANID);
 	if (devid != SIO_FINTEK_ID)
 		goto exit;
-- 
2.7.4

[PATCH 2/4] hwmon: (pc87427) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

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: ba224e2c4f0a7 ("hwmon: New PC87427 hardware monitoring driver")
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>
---
 drivers/hwmon/pc87427.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pc87427.c b/drivers/hwmon/pc87427.c
index d1a3f2040c00..58eee8fa3e6d 100644
--- a/drivers/hwmon/pc87427.c
+++ b/drivers/hwmon/pc87427.c
@@ -106,6 +106,13 @@ static const char *logdev_str[2] = { DRVNAME " FMC", DRVNAME " HMC" };
 #define LD_IN		1
 #define LD_TEMP		1
 
+static inline int superio_enter(int sioaddr)
+{
+	if (!request_muxed_region(sioaddr, 2, DRVNAME))
+		return -EBUSY;
+	return 0;
+}
+
 static inline void superio_outb(int sioaddr, int reg, int val)
 {
 	outb(reg, sioaddr);
@@ -122,6 +129,7 @@ static inline void superio_exit(int sioaddr)
 {
 	outb(0x02, sioaddr);
 	outb(0x02, sioaddr + 1);
+	release_region(sioaddr, 2);
 }
 
 /*
@@ -1195,7 +1203,11 @@ static int __init pc87427_find(int sioaddr, struct pc87427_sio_data *sio_data)
 {
 	u16 val;
 	u8 cfg, cfg_b;
-	int i, err = 0;
+	int i, err;
+
+	err = superio_enter(sioaddr);
+	if (err)
+		return err;
 
 	/* Identify device */
 	val = force_id ? force_id : superio_inb(sioaddr, SIOREG_DEVID);
-- 
2.7.4

[PATCH 3/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

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>
---
 drivers/hwmon/smsc47b397.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/smsc47b397.c b/drivers/hwmon/smsc47b397.c
index c0775084dde0..60e193f2e970 100644
--- a/drivers/hwmon/smsc47b397.c
+++ b/drivers/hwmon/smsc47b397.c
@@ -72,14 +72,19 @@ static inline void superio_select(int ld)
 	superio_outb(0x07, ld);
 }
 
-static inline void superio_enter(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_DEVID	0x20
@@ -300,8 +305,12 @@ static int __init smsc47b397_find(void)
 	u8 id, rev;
 	char *name;
 	unsigned short addr;
+	int err;
+
+	err = superio_enter();
+	if (err)
+		return err;
 
-	superio_enter();
 	id = force_id ? force_id : superio_inb(SUPERIO_REG_DEVID);
 
 	switch (id) {
-- 
2.7.4

[PATCH 4/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

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>
---
 drivers/hwmon/smsc47m1.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c
index c7b6a425e2c0..0736ca6a3aee 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
@@ -531,8 +536,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);
 
 	/*
-- 
2.7.4

Re: [PATCH 1/4] hwmon: (f71805f) Use request_muxed_region for Super-IO accesses

[John Garry]

On 04/04/2019 19:33, Guenter Roeck wrote:
> Super-IO accesses may fail on a system with no or unmapped LPC bus.
>
> Unable to handle kernel paging request at virtual address ffffffbffee0002e
> pgd = ffffffc1d68d4000
> [ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
> Internal error: Oops: 94000046 [#1] PREEMPT SMP
> Modules linked in: f71805f(+) hwmon
> CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
> Hardware name: linux,dummy-virt (DT)
> task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
> PC is at f71805f_find+0x6c/0x358 [f71805f]
>
> 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: e53004e20a58e ("hwmon: New f71805f driver")
> 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>
> ---
>  drivers/hwmon/f71805f.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/f71805f.c b/drivers/hwmon/f71805f.c
> index 73c681162653..623736d2a7c1 100644
> --- a/drivers/hwmon/f71805f.c
> +++ b/drivers/hwmon/f71805f.c
> @@ -96,17 +96,23 @@ superio_select(int base, int ld)
>  	outb(ld, base + 1);
>  }
>
> -static inline void
> +static inline int
>  superio_enter(int base)
>  {
> +	if (!request_muxed_region(base, 2, DRVNAME))
> +		return -EBUSY;
> +
>  	outb(0x87, base);
>  	outb(0x87, base);
> +
> +	return 0;
>  }
>
>  static inline void
>  superio_exit(int base)
>  {
>  	outb(0xaa, base);
> +	release_region(base, 2);
>  }
>
>  /*
> @@ -1561,7 +1567,7 @@ static int __init f71805f_device_add(unsigned short address,
>  static int __init f71805f_find(int sioaddr, unsigned short *address,
>  			       struct f71805f_sio_data *sio_data)
>  {
> -	int err = -ENODEV;
> +	int err;
>  	u16 devid;
>
>  	static const char * const names[] = {
> @@ -1569,8 +1575,11 @@ static int __init f71805f_find(int sioaddr, unsigned short *address,
>  		"F71872F/FG or F71806F/FG",
>  	};
>
> -	superio_enter(sioaddr);
> +	err = superio_enter(sioaddr);
> +	if (err)
> +		return err;
>
> +	err = -ENODEV;
>  	devid = superio_inw(sioaddr, SIO_REG_MANID);
>  	if (devid != SIO_FINTEK_ID)
>  		goto exit;
>

Re: [PATCH 2/4] hwmon: (pc87427) Use request_muxed_region for Super-IO accesses

[John Garry]

On 04/04/2019 19:33, 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: ba224e2c4f0a7 ("hwmon: New PC87427 hardware monitoring driver")
> 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>
> ---
>  drivers/hwmon/pc87427.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/pc87427.c b/drivers/hwmon/pc87427.c
> index d1a3f2040c00..58eee8fa3e6d 100644
> --- a/drivers/hwmon/pc87427.c
> +++ b/drivers/hwmon/pc87427.c
> @@ -106,6 +106,13 @@ static const char *logdev_str[2] = { DRVNAME " FMC", DRVNAME " HMC" };
>  #define LD_IN		1
>  #define LD_TEMP		1
>
> +static inline int superio_enter(int sioaddr)
> +{
> +	if (!request_muxed_region(sioaddr, 2, DRVNAME))
> +		return -EBUSY;
> +	return 0;
> +}
> +
>  static inline void superio_outb(int sioaddr, int reg, int val)
>  {
>  	outb(reg, sioaddr);
> @@ -122,6 +129,7 @@ static inline void superio_exit(int sioaddr)
>  {
>  	outb(0x02, sioaddr);
>  	outb(0x02, sioaddr + 1);
> +	release_region(sioaddr, 2);
>  }
>
>  /*
> @@ -1195,7 +1203,11 @@ static int __init pc87427_find(int sioaddr, struct pc87427_sio_data *sio_data)
>  {
>  	u16 val;
>  	u8 cfg, cfg_b;
> -	int i, err = 0;
> +	int i, err;
> +
> +	err = superio_enter(sioaddr);
> +	if (err)
> +		return err;
>
>  	/* Identify device */
>  	val = force_id ? force_id : superio_inb(sioaddr, SIOREG_DEVID);
>

Re: [PATCH 3/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[John Garry]

On 04/04/2019 19:33, 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>

Acked-by: John Garry <john.garry@huawei.com>

> Cc: John Garry <john.garry@huawei.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/smsc47b397.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/smsc47b397.c b/drivers/hwmon/smsc47b397.c
> index c0775084dde0..60e193f2e970 100644
> --- a/drivers/hwmon/smsc47b397.c
> +++ b/drivers/hwmon/smsc47b397.c
> @@ -72,14 +72,19 @@ static inline void superio_select(int ld)
>  	superio_outb(0x07, ld);
>  }
>
> -static inline void superio_enter(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_DEVID	0x20
> @@ -300,8 +305,12 @@ static int __init smsc47b397_find(void)
>  	u8 id, rev;
>  	char *name;
>  	unsigned short addr;
> +	int err;
> +
> +	err = superio_enter();
> +	if (err)
> +		return err;
>
> -	superio_enter();
>  	id = force_id ? force_id : superio_inb(SUPERIO_REG_DEVID);
>
>  	switch (id) {
>

Re: [PATCH 4/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[John Garry]

On 04/04/2019 19:33, 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>
> ---
>  drivers/hwmon/smsc47m1.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c
> index c7b6a425e2c0..0736ca6a3aee 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
> @@ -531,8 +536,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);
>
>  	/*
>

Re: [PATCH 4/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[John Garry]

On 05/04/2019 08:47, John Garry wrote:
> On 04/04/2019 19:33, Guenter Roeck wrote:
>> Super-IO accesses may fail on a system with no or unmapped LPC bus.
>>

BTW, these may still require attention:

sch56xx-common
smsc47m1
w83627hf
vt1211

Thanks,
John

>> 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>
>> ---
>>  drivers/hwmon/smsc47m1.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c
>> index c7b6a425e2c0..0736ca6a3aee 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
>> @@ -531,8 +536,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);
>>
>>      /*
>>
>

Re: [PATCH 4/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

On 4/5/19 1:46 AM, John Garry wrote:
> On 05/04/2019 08:47, John Garry wrote:
>> On 04/04/2019 19:33, Guenter Roeck wrote:
>>> Super-IO accesses may fail on a system with no or unmapped LPC bus.
>>>
> 
> BTW, these may still require attention:
> 
> sch56xx-common
> smsc47m1

Those two already call request_muxed_region() in superio_enter().
Though error handling in the latter is less than perfect - I may send
a patch to fix it.

> w83627hf > vt1211

I missed those two. Patches will follow.

Thanks,
Guenter

> 
> Thanks,
> John
> 
>>> 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>
>>> ---
>>>  drivers/hwmon/smsc47m1.c | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c
>>> index c7b6a425e2c0..0736ca6a3aee 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
>>> @@ -531,8 +536,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);
>>>
>>>      /*
>>>
>>
> 
> 
> 

Re: [PATCH 4/4] hwmon: (smsc47b397) Use request_muxed_region for Super-IO accesses

[Guenter Roeck]

On Fri, Apr 05, 2019 at 06:25:13AM -0700, Guenter Roeck wrote:
> On 4/5/19 1:46 AM, John Garry wrote:
> >On 05/04/2019 08:47, John Garry wrote:
> >>On 04/04/2019 19:33, Guenter Roeck wrote:
> >>>Super-IO accesses may fail on a system with no or unmapped LPC bus.
> >>>
> >
> >BTW, these may still require attention:
> >
> >sch56xx-common
> >smsc47m1
> 
> Those two already call request_muxed_region() in superio_enter().
> Though error handling in the latter is less than perfect - I may send
> a patch to fix it.
> 

... and that was because this patch has the wrong subject, and its error
handling is incomplete. I'll resend.

Guenter

> >w83627hf > vt1211
> 
> I missed those two. Patches will follow.
> 
> Thanks,
> Guenter
> 
> >
> >Thanks,
> >John
> >
> >>>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>
> >>>---
> >>> drivers/hwmon/smsc47m1.c | 13 +++++++++++--
> >>> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c
> >>>index c7b6a425e2c0..0736ca6a3aee 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
> >>>@@ -531,8 +536,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);
> >>>
> >>>     /*
> >>>
> >>
> >
> >
> >
>