Re: [PATCH v2] hwmon: (applesmc) fix UB and udelay overflow
On Tue, Sep 24, 2019 at 12:36:44PM -0700, Nick Desaulniers wrote:
> On Tue, Sep 24, 2019 at 11:38 AM Nathan Chancellor
> wrote:
> >
> > On Tue, Sep 24, 2019 at 10:47:28AM -0700, Nick Desaulniers wrote:
> > > Fixes the following 2 issues in the driver:
> > > 1. Left shifting a signed integer is undefined behavior. Unsigned
> > >integral types should be used for bitwise operations.
> > > 2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
> > >will result in a linkage failure when given a constant that's greater
> > >than 2 (0x4E20). Agressive loop unrolling can fully unroll the
> > >loop, resulting in later iterations overflowing the call to udelay.
> > >
> > > 2 is fixed via splitting the loop in two, iterating the first up to the
> > > point where udelay would overflow, then switching to mdelay, as
> > > suggested in Documentation/timers/timers-howto.rst.
> > >
> > > Reported-by: Tomasz Paweł Gajc
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/678
> > > Debugged-by: Nathan Chancellor
> > > Signed-off-by: Nick Desaulniers
> > > ---
> > > Changes V1 -> V2:
> > > * The first loop in send_byte() needs to break out on the same condition
> > > now. Technically, the loop condition could even be removed. The diff
> > > looks funny because of the duplicated logic between existing and newly
> > > added for loops.
> > >
> > > drivers/hwmon/applesmc.c | 35 +++
> > > 1 file changed, 31 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> > > index 183ff3d25129..c76adb504dff 100644
> > > --- a/drivers/hwmon/applesmc.c
> > > +++ b/drivers/hwmon/applesmc.c
> > > @@ -46,6 +46,7 @@
> > > #define APPLESMC_MIN_WAIT0x0010
> > > #define APPLESMC_RETRY_WAIT 0x0100
> > > #define APPLESMC_MAX_WAIT0x2
> > > +#define APPLESMC_UDELAY_MAX 2
> > >
> > > #define APPLESMC_READ_CMD0x10
> > > #define APPLESMC_WRITE_CMD 0x11
> > > @@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
> > > static int wait_read(void)
> > > {
> > > u8 status;
> > > - int us;
> > > - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > > + unsigned int us;
> > > +
> > > + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> > > udelay(us);
> > > status = inb(APPLESMC_CMD_PORT);
> > > /* read: wait for smc to settle */
> > > if (status & 0x01)
> > > return 0;
> > > }
> > > + /* switch to mdelay for longer sleeps */
> > > + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > > + mdelay(us);
> > > + status = inb(APPLESMC_CMD_PORT);
> > > + /* read: wait for smc to settle */
> > > + if (status & 0x01)
> > > + return 0;
> > > + }
> > >
> > > pr_warn("wait_read() fail: 0x%02x\n", status);
> > > return -EIO;
> > > @@ -177,10 +187,10 @@ static int wait_read(void)
> > > static int send_byte(u8 cmd, u16 port)
> > > {
> > > u8 status;
> > > - int us;
> > > + unsigned int us;
> > >
> > > outb(cmd, port);
> > > - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > > + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> > > udelay(us);
> > > status = inb(APPLESMC_CMD_PORT);
> > > /* write: wait for smc to settle */
> > > @@ -190,6 +200,23 @@ static int send_byte(u8 cmd, u16 port)
> > > if (status & 0x04)
> > > return 0;
> > > /* timeout: give up */
> > > + if (us << 1 == APPLESMC_UDELAY_MAX)
> > > + break;
> > > + /* busy: long wait and resend */
> > > + udelay(APPLESMC_RETRY_WAIT);
> > > + outb(cmd, port);
> > > + }
> > > + /* switch to mdelay for longer sleeps */
> > > + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > > + mdelay(us);
> > > + status = inb(APPLESMC_CMD_PORT);
> > > + /* write: wait for smc to settle */
> > > + if (status & 0x02)
> > > + continue;
> > > + /* ready: cmd accepted, return */
> > > + if (status & 0x04)
> > > + return 0;
> > > + /* timeout: give up */
> > > if (us << 1 == APPLESMC_MAX_WAIT)
> > > break;
> > > /* busy: long wait and resend */
> > > --
> > > 2.23.0.351.gc4317032e6-goog
> > >
> >
> > This resolves the __bad_udelay appearance at -O3 for me. I am not
> > familiar enough with this code to give a reviewed by though!
>
> Does that constitute a Tested-by tag?
Given that this could have real implications for the hardware, I would
think tested by would imply running this on said hardware. FWIW:
Build-tested-by: Nathan C
Re: [PATCH v2] hwmon: (applesmc) fix UB and udelay overflow
On Tue, Sep 24, 2019 at 11:38 AM Nathan Chancellor
wrote:
>
> On Tue, Sep 24, 2019 at 10:47:28AM -0700, Nick Desaulniers wrote:
> > Fixes the following 2 issues in the driver:
> > 1. Left shifting a signed integer is undefined behavior. Unsigned
> >integral types should be used for bitwise operations.
> > 2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
> >will result in a linkage failure when given a constant that's greater
> >than 2 (0x4E20). Agressive loop unrolling can fully unroll the
> >loop, resulting in later iterations overflowing the call to udelay.
> >
> > 2 is fixed via splitting the loop in two, iterating the first up to the
> > point where udelay would overflow, then switching to mdelay, as
> > suggested in Documentation/timers/timers-howto.rst.
> >
> > Reported-by: Tomasz Paweł Gajc
> > Link: https://github.com/ClangBuiltLinux/linux/issues/678
> > Debugged-by: Nathan Chancellor
> > Signed-off-by: Nick Desaulniers
> > ---
> > Changes V1 -> V2:
> > * The first loop in send_byte() needs to break out on the same condition
> > now. Technically, the loop condition could even be removed. The diff
> > looks funny because of the duplicated logic between existing and newly
> > added for loops.
> >
> > drivers/hwmon/applesmc.c | 35 +++
> > 1 file changed, 31 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> > index 183ff3d25129..c76adb504dff 100644
> > --- a/drivers/hwmon/applesmc.c
> > +++ b/drivers/hwmon/applesmc.c
> > @@ -46,6 +46,7 @@
> > #define APPLESMC_MIN_WAIT0x0010
> > #define APPLESMC_RETRY_WAIT 0x0100
> > #define APPLESMC_MAX_WAIT0x2
> > +#define APPLESMC_UDELAY_MAX 2
> >
> > #define APPLESMC_READ_CMD0x10
> > #define APPLESMC_WRITE_CMD 0x11
> > @@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
> > static int wait_read(void)
> > {
> > u8 status;
> > - int us;
> > - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > + unsigned int us;
> > +
> > + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> > udelay(us);
> > status = inb(APPLESMC_CMD_PORT);
> > /* read: wait for smc to settle */
> > if (status & 0x01)
> > return 0;
> > }
> > + /* switch to mdelay for longer sleeps */
> > + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > + mdelay(us);
> > + status = inb(APPLESMC_CMD_PORT);
> > + /* read: wait for smc to settle */
> > + if (status & 0x01)
> > + return 0;
> > + }
> >
> > pr_warn("wait_read() fail: 0x%02x\n", status);
> > return -EIO;
> > @@ -177,10 +187,10 @@ static int wait_read(void)
> > static int send_byte(u8 cmd, u16 port)
> > {
> > u8 status;
> > - int us;
> > + unsigned int us;
> >
> > outb(cmd, port);
> > - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> > udelay(us);
> > status = inb(APPLESMC_CMD_PORT);
> > /* write: wait for smc to settle */
> > @@ -190,6 +200,23 @@ static int send_byte(u8 cmd, u16 port)
> > if (status & 0x04)
> > return 0;
> > /* timeout: give up */
> > + if (us << 1 == APPLESMC_UDELAY_MAX)
> > + break;
> > + /* busy: long wait and resend */
> > + udelay(APPLESMC_RETRY_WAIT);
> > + outb(cmd, port);
> > + }
> > + /* switch to mdelay for longer sleeps */
> > + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> > + mdelay(us);
> > + status = inb(APPLESMC_CMD_PORT);
> > + /* write: wait for smc to settle */
> > + if (status & 0x02)
> > + continue;
> > + /* ready: cmd accepted, return */
> > + if (status & 0x04)
> > + return 0;
> > + /* timeout: give up */
> > if (us << 1 == APPLESMC_MAX_WAIT)
> > break;
> > /* busy: long wait and resend */
> > --
> > 2.23.0.351.gc4317032e6-goog
> >
>
> This resolves the __bad_udelay appearance at -O3 for me. I am not
> familiar enough with this code to give a reviewed by though!
Does that constitute a Tested-by tag?
>
> Also, for some odd reason, I couldn't apply your patch with 'git apply':
>
> % curl -LSs
> https://lore.kernel.org/lkml/20190924174728.201464-1-ndesaulni...@google.com/raw
> | git apply
> error: corrupt patch at line 117
>
> It looks like some of the '=' got changed into =3D and some spaces got
> changed into =20. Weird encoding glitch?
The text in my email client shows no encoding error; the link above
shows the issue. Attaching a copy here,
Re: [PATCH v2] hwmon: (applesmc) fix UB and udelay overflow
On Tue, Sep 24, 2019 at 10:47:28AM -0700, Nick Desaulniers wrote:
> Fixes the following 2 issues in the driver:
> 1. Left shifting a signed integer is undefined behavior. Unsigned
>integral types should be used for bitwise operations.
> 2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
>will result in a linkage failure when given a constant that's greater
>than 2 (0x4E20). Agressive loop unrolling can fully unroll the
>loop, resulting in later iterations overflowing the call to udelay.
>
> 2 is fixed via splitting the loop in two, iterating the first up to the
> point where udelay would overflow, then switching to mdelay, as
> suggested in Documentation/timers/timers-howto.rst.
>
> Reported-by: Tomasz Paweł Gajc
> Link: https://github.com/ClangBuiltLinux/linux/issues/678
> Debugged-by: Nathan Chancellor
> Signed-off-by: Nick Desaulniers
> ---
> Changes V1 -> V2:
> * The first loop in send_byte() needs to break out on the same condition
> now. Technically, the loop condition could even be removed. The diff
> looks funny because of the duplicated logic between existing and newly
> added for loops.
>
> drivers/hwmon/applesmc.c | 35 +++
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 183ff3d25129..c76adb504dff 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -46,6 +46,7 @@
> #define APPLESMC_MIN_WAIT0x0010
> #define APPLESMC_RETRY_WAIT 0x0100
> #define APPLESMC_MAX_WAIT0x2
> +#define APPLESMC_UDELAY_MAX 2
>
> #define APPLESMC_READ_CMD0x10
> #define APPLESMC_WRITE_CMD 0x11
> @@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
> static int wait_read(void)
> {
> u8 status;
> - int us;
> - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + unsigned int us;
> +
> + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> udelay(us);
> status = inb(APPLESMC_CMD_PORT);
> /* read: wait for smc to settle */
> if (status & 0x01)
> return 0;
> }
> + /* switch to mdelay for longer sleeps */
> + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + mdelay(us);
> + status = inb(APPLESMC_CMD_PORT);
> + /* read: wait for smc to settle */
> + if (status & 0x01)
> + return 0;
> + }
>
> pr_warn("wait_read() fail: 0x%02x\n", status);
> return -EIO;
> @@ -177,10 +187,10 @@ static int wait_read(void)
> static int send_byte(u8 cmd, u16 port)
> {
> u8 status;
> - int us;
> + unsigned int us;
>
> outb(cmd, port);
> - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> udelay(us);
> status = inb(APPLESMC_CMD_PORT);
> /* write: wait for smc to settle */
> @@ -190,6 +200,23 @@ static int send_byte(u8 cmd, u16 port)
> if (status & 0x04)
> return 0;
> /* timeout: give up */
> + if (us << 1 == APPLESMC_UDELAY_MAX)
> + break;
> + /* busy: long wait and resend */
> + udelay(APPLESMC_RETRY_WAIT);
> + outb(cmd, port);
> + }
> + /* switch to mdelay for longer sleeps */
> + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + mdelay(us);
> + status = inb(APPLESMC_CMD_PORT);
> + /* write: wait for smc to settle */
> + if (status & 0x02)
> + continue;
> + /* ready: cmd accepted, return */
> + if (status & 0x04)
> + return 0;
> + /* timeout: give up */
> if (us << 1 == APPLESMC_MAX_WAIT)
> break;
> /* busy: long wait and resend */
> --
> 2.23.0.351.gc4317032e6-goog
>
This resolves the __bad_udelay appearance at -O3 for me. I am not
familiar enough with this code to give a reviewed by though!
Also, for some odd reason, I couldn't apply your patch with 'git apply':
% curl -LSs
https://lore.kernel.org/lkml/20190924174728.201464-1-ndesaulni...@google.com/raw
| git apply
error: corrupt patch at line 117
It looks like some of the '=' got changed into =3D and some spaces got
changed into =20. Weird encoding glitch?
Cheers,
Nathan
[PATCH v2] hwmon: (applesmc) fix UB and udelay overflow
Fixes the following 2 issues in the driver:
1. Left shifting a signed integer is undefined behavior. Unsigned
integral types should be used for bitwise operations.
2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
will result in a linkage failure when given a constant that's greater
than 2 (0x4E20). Agressive loop unrolling can fully unroll the
loop, resulting in later iterations overflowing the call to udelay.
2 is fixed via splitting the loop in two, iterating the first up to the
point where udelay would overflow, then switching to mdelay, as
suggested in Documentation/timers/timers-howto.rst.
Reported-by: Tomasz Paweł Gajc
Link: https://github.com/ClangBuiltLinux/linux/issues/678
Debugged-by: Nathan Chancellor
Signed-off-by: Nick Desaulniers
---
Changes V1 -> V2:
* The first loop in send_byte() needs to break out on the same condition
now. Technically, the loop condition could even be removed. The diff
looks funny because of the duplicated logic between existing and newly
added for loops.
drivers/hwmon/applesmc.c | 35 +++
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 183ff3d25129..c76adb504dff 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -46,6 +46,7 @@
#define APPLESMC_MIN_WAIT 0x0010
#define APPLESMC_RETRY_WAIT0x0100
#define APPLESMC_MAX_WAIT 0x2
+#define APPLESMC_UDELAY_MAX2
#define APPLESMC_READ_CMD 0x10
#define APPLESMC_WRITE_CMD 0x11
@@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
static int wait_read(void)
{
u8 status;
- int us;
- for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ unsigned int us;
+
+ for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
udelay(us);
status = inb(APPLESMC_CMD_PORT);
/* read: wait for smc to settle */
if (status & 0x01)
return 0;
}
+ /* switch to mdelay for longer sleeps */
+ for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ mdelay(us);
+ status = inb(APPLESMC_CMD_PORT);
+ /* read: wait for smc to settle */
+ if (status & 0x01)
+ return 0;
+ }
pr_warn("wait_read() fail: 0x%02x\n", status);
return -EIO;
@@ -177,10 +187,10 @@ static int wait_read(void)
static int send_byte(u8 cmd, u16 port)
{
u8 status;
- int us;
+ unsigned int us;
outb(cmd, port);
- for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
udelay(us);
status = inb(APPLESMC_CMD_PORT);
/* write: wait for smc to settle */
@@ -190,6 +200,23 @@ static int send_byte(u8 cmd, u16 port)
if (status & 0x04)
return 0;
/* timeout: give up */
+ if (us << 1 == APPLESMC_UDELAY_MAX)
+ break;
+ /* busy: long wait and resend */
+ udelay(APPLESMC_RETRY_WAIT);
+ outb(cmd, port);
+ }
+ /* switch to mdelay for longer sleeps */
+ for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ mdelay(us);
+ status = inb(APPLESMC_CMD_PORT);
+ /* write: wait for smc to settle */
+ if (status & 0x02)
+ continue;
+ /* ready: cmd accepted, return */
+ if (status & 0x04)
+ return 0;
+ /* timeout: give up */
if (us << 1 == APPLESMC_MAX_WAIT)
break;
/* busy: long wait and resend */
--
2.23.0.351.gc4317032e6-goog
Re: [PATCH] hwmon: (applesmc) fix UB and udelay overflow
On Tue, Sep 24, 2019 at 10:37 AM Nick Desaulniers
wrote:
>
> Fixes the following 2 issues in the driver:
> 1. Left shifting a signed integer is undefined behavior. Unsigned
>integral types should be used for bitwise operations.
> 2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
>will result in a linkage failure when given a constant that's greater
>than 2 (0x4E20). Agressive loop unrolling can fully unroll the
>loop, resulting in later iterations overflowing the call to udelay.
>
> 2 is fixed via splitting the loop in two, iterating the first up to the
> point where udelay would overflow, then switching to mdelay, as
> suggested in Documentation/timers/timers-howto.rst.
>
> Reported-by: Tomasz Paweł Gajc
> Link: https://github.com/ClangBuiltLinux/linux/issues/678
> Debugged-by: Nathan Chancellor
> Signed-off-by: Nick Desaulniers
> ---
> drivers/hwmon/applesmc.c | 35 +++
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 183ff3d25129..2bc12812f52f 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -46,6 +46,7 @@
> #define APPLESMC_MIN_WAIT 0x0010
> #define APPLESMC_RETRY_WAIT0x0100
> #define APPLESMC_MAX_WAIT 0x2
> +#define APPLESMC_UDELAY_MAX2
>
> #define APPLESMC_READ_CMD 0x10
> #define APPLESMC_WRITE_CMD 0x11
> @@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
> static int wait_read(void)
> {
> u8 status;
> - int us;
> - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + unsigned int us;
> +
> + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> udelay(us);
> status = inb(APPLESMC_CMD_PORT);
> /* read: wait for smc to settle */
> if (status & 0x01)
> return 0;
> }
> + /* switch to mdelay for longer sleeps */
> + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + mdelay(us);
> + status = inb(APPLESMC_CMD_PORT);
> + /* read: wait for smc to settle */
> + if (status & 0x01)
> + return 0;
> + }
>
> pr_warn("wait_read() fail: 0x%02x\n", status);
> return -EIO;
> @@ -177,10 +187,10 @@ static int wait_read(void)
> static int send_byte(u8 cmd, u16 port)
> {
> u8 status;
> - int us;
> + unsigned int us;
>
> outb(cmd, port);
> - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
> udelay(us);
> status = inb(APPLESMC_CMD_PORT);
> /* write: wait for smc to settle */
> @@ -196,6 +206,23 @@ static int send_byte(u8 cmd, u16 port)
> udelay(APPLESMC_RETRY_WAIT);
> outb(cmd, port);
> }
> + /* switch to mdelay for longer sleeps */
> + for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
> + mdelay(us);
> + status = inb(APPLESMC_CMD_PORT);
> + /* write: wait for smc to settle */
> + if (status & 0x02)
> + continue;
> + /* ready: cmd accepted, return */
> + if (status & 0x04)
> + return 0;
> + /* timeout: give up */
> + if (us << 1 == APPLESMC_MAX_WAIT)
> + break;
Sorry, I need to modify the first for loop in this function to break
out properly. v2 inbound.
> + /* busy: long wait and resend */
> + udelay(APPLESMC_RETRY_WAIT);
> + outb(cmd, port);
> + }
>
> pr_warn("send_byte(0x%02x, 0x%04x) fail: 0x%02x\n", cmd, port,
> status);
> return -EIO;
> --
> 2.23.0.351.gc4317032e6-goog
>
--
Thanks,
~Nick Desaulniers
[PATCH] hwmon: (applesmc) fix UB and udelay overflow
Fixes the following 2 issues in the driver:
1. Left shifting a signed integer is undefined behavior. Unsigned
integral types should be used for bitwise operations.
2. The delay scales from 0x0010 to 0x2 by powers of 2, but udelay
will result in a linkage failure when given a constant that's greater
than 2 (0x4E20). Agressive loop unrolling can fully unroll the
loop, resulting in later iterations overflowing the call to udelay.
2 is fixed via splitting the loop in two, iterating the first up to the
point where udelay would overflow, then switching to mdelay, as
suggested in Documentation/timers/timers-howto.rst.
Reported-by: Tomasz Paweł Gajc
Link: https://github.com/ClangBuiltLinux/linux/issues/678
Debugged-by: Nathan Chancellor
Signed-off-by: Nick Desaulniers
---
drivers/hwmon/applesmc.c | 35 +++
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 183ff3d25129..2bc12812f52f 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -46,6 +46,7 @@
#define APPLESMC_MIN_WAIT 0x0010
#define APPLESMC_RETRY_WAIT0x0100
#define APPLESMC_MAX_WAIT 0x2
+#define APPLESMC_UDELAY_MAX2
#define APPLESMC_READ_CMD 0x10
#define APPLESMC_WRITE_CMD 0x11
@@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq;
static int wait_read(void)
{
u8 status;
- int us;
- for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ unsigned int us;
+
+ for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
udelay(us);
status = inb(APPLESMC_CMD_PORT);
/* read: wait for smc to settle */
if (status & 0x01)
return 0;
}
+ /* switch to mdelay for longer sleeps */
+ for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ mdelay(us);
+ status = inb(APPLESMC_CMD_PORT);
+ /* read: wait for smc to settle */
+ if (status & 0x01)
+ return 0;
+ }
pr_warn("wait_read() fail: 0x%02x\n", status);
return -EIO;
@@ -177,10 +187,10 @@ static int wait_read(void)
static int send_byte(u8 cmd, u16 port)
{
u8 status;
- int us;
+ unsigned int us;
outb(cmd, port);
- for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) {
udelay(us);
status = inb(APPLESMC_CMD_PORT);
/* write: wait for smc to settle */
@@ -196,6 +206,23 @@ static int send_byte(u8 cmd, u16 port)
udelay(APPLESMC_RETRY_WAIT);
outb(cmd, port);
}
+ /* switch to mdelay for longer sleeps */
+ for (; us < APPLESMC_MAX_WAIT; us <<= 1) {
+ mdelay(us);
+ status = inb(APPLESMC_CMD_PORT);
+ /* write: wait for smc to settle */
+ if (status & 0x02)
+ continue;
+ /* ready: cmd accepted, return */
+ if (status & 0x04)
+ return 0;
+ /* timeout: give up */
+ if (us << 1 == APPLESMC_MAX_WAIT)
+ break;
+ /* busy: long wait and resend */
+ udelay(APPLESMC_RETRY_WAIT);
+ outb(cmd, port);
+ }
pr_warn("send_byte(0x%02x, 0x%04x) fail: 0x%02x\n", cmd, port, status);
return -EIO;
--
2.23.0.351.gc4317032e6-goog
