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 0x20000 by powers of 2, but udelay
   will result in a linkage failure when given a constant that's greater
   than 20000 (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 <tpg...@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/678
Debugged-by: Nathan Chancellor <natechancel...@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulni...@google.com>
---
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_WAIT    0x0100
 #define APPLESMC_MAX_WAIT      0x20000
+#define APPLESMC_UDELAY_MAX    20000
 
 #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

Reply via email to