Hi,
On 9/2/24 10:26 PM, Matthias Fetzer wrote:
> 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?
Please submit a new patch on top of platform-drivers-x86/for-next
or on top of platform-drivers-x86/review-hans addressing just this
warning.
Regards,
Hans
