On 10/29/20 12:47 PM, Zou Wei wrote:
The status local variable is assigned but never read:
[...]
static int s3_wmi_query_block(const char *guid, int instance, int *ret)
{
struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
- acpi_status status;
union acpi_object *obj;
int error = 0;
mutex_lock(&s3_wmi_lock);
- status = wmi_query_block(guid, instance, &output);
I assume you want to remove the "status =" and not the whole line...
Regardless, I personally would prefer explicit error handling (as is
done in other contexts with wmi_query_block(), see e.g.
platform/x86/intel-wmi-sbl-fw-update.c. Although, yes, the "if (!obj |
..." should cover the error case as far as I can tell.
Also there seems to be the same issue in platform/x86/msi-wmi.c.
On a related note:
Does anyone know if the mutex here is really required? If there are any
problems, I believe it should rather go to s3_wmi_send_lid_state() and
also wrap the input_report_switch() / input_sync(). Otherwise, if there
were any race-type situations, not covering that could lead to wrong
updates:
Thread 1 Thread 2
s3_query_lid() returns old
s3_query_lid() returns new
input_report_switch()
input_sync()
input_report_switch()
input_sync()
I don't really expect those situations here as s3_wmi_send_lid_state()
is only used in probe(), resume(), and notify() (please correct me if
I'm wrong), so the mutex does seem weird to me here.
Otherwise if it's needed, a comment explaining why wouldn't hurt.
Regards,
Max
