Re: [bug report] platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

Am 27.08.24 um 11:09 schrieb Hans de Goede:
Hi Dan,

On 8/27/24 10:45 AM, Dan Carpenter wrote:
Hello Matthias Fetzer,

Commit 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge
E531 fan support") from Aug 16, 2024 (linux-next), leads to the
following Smatch static checker warning:

	drivers/platform/x86/thinkpad_acpi.c:8387 fan_set_enable()
	error: uninitialized symbol 's'.

drivers/platform/x86/thinkpad_acpi.c
     8319 static int fan_set_enable(void)
     8320 {
     8321         u8 s;
     8322         int rc;
     8323
     8324         if (!fan_control_allowed)
     8325                 return -EPERM;
     8326
     8327         if (mutex_lock_killable(&fan_mutex))
     8328                 return -ERESTARTSYS;
     8329
     8330         switch (fan_control_access_mode) {
     8331         case TPACPI_FAN_WR_ACPI_FANS:
     8332         case TPACPI_FAN_WR_TPEC:
     8333                 rc = fan_get_status(&s);
     8334                 if (rc)
     8335                         break;
     8336
     8337                 /* Don't go out of emergency fan mode */
     8338                 if (s != 7) {
     8339                         s &= 0x07;
     8340                         s |= TP_EC_FAN_AUTO | 4; /* min fan speed 4 */
     8341                 }
     8342
     8343                 if (!acpi_ec_write(fan_status_offset, s))
     8344                         rc = -EIO;
     8345                 else {
     8346                         tp_features.fan_ctrl_status_undef = 0;
     8347                         rc = 0;
     8348                 }
     8349                 break;
     8350
     8351         case TPACPI_FAN_WR_ACPI_SFAN:
     8352                 rc = fan_get_status(&s);
     8353                 if (rc)
     8354                         break;
     8355
     8356                 s &= 0x07;
     8357
     8358                 /* Set fan to at least level 4 */
     8359                 s |= 4;
     8360
     8361                 if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", s))
     8362                         rc = -EIO;
     8363                 else
     8364                         rc = 0;
     8365                 break;
     8366
     8367         case TPACPI_FAN_WR_ACPI_FANW:
     8368                 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) {
     8369                         rc = -EIO;
     8370                         break;
     8371                 }
     8372                 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) {
     8373                         rc = -EIO;
     8374                         break;
     8375                 }
     8376
     8377                 rc = 0;

s isn't set on this path

     8378                 break;
     8379
     8380         default:
     8381                 rc = -ENXIO;
     8382         }
     8383
     8384         mutex_unlock(&fan_mutex);
     8385
     8386         if (!rc)
--> 8387                 vdbg_printk(TPACPI_DBG_FAN,
     8388                         "fan control: set fan control register to 0x%02x\n",
     8389                         s);
                                  ^
Here

     8390         return rc;
     8391 }

Thank you for reporting this.

Since this is just a debug print I think it is best to fix this by just
initializing s to 0 when it is declared and then just log 0
in the TPACPI_FAN_WR_ACPI_FANW case.

Anyone got any better suggestions ?

Regards,

Hans



Since there do not seem to be any other suggestions I'll fix it that
way.
Shall I make a completely new patch for this since it is already on your
review branch or would a fixed v5 be enough?

Best regards,
Matthias




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux