| Message ID | 20230324075854.458341-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | ACPICA: Make check to install handler more obviously correct | expand |
On Fri, Mar 24, 2023 at 08:58:54AM +0100, Uwe Kleine-König wrote: > The loop > > for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { > if (handler_type & (i + 1)) { > ... > } > } > > looks strange. Only with knowing that ACPI_NUM_NOTIFY_TYPES == 2 you can > see that the two least significant bits are checked. Still replace > > i + 1 > > by > > 1 << i > > which shouldn't make a relevant difference to compiler and compiled > code, but is easier to understand for a human code reader. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> You need to submit this to ACPICA project first. Documentation/driver-api/acpi/linuxized-acpica.rst explains the process. Refer [1] for details for similar suggestion by Rafael.
Hello, On Fri, Mar 24, 2023 at 09:53:29AM +0000, Sudeep Holla wrote: > On Fri, Mar 24, 2023 at 08:58:54AM +0100, Uwe Kleine-König wrote: > > The loop > > > > for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { > > if (handler_type & (i + 1)) { > > ... > > } > > } > > > > looks strange. Only with knowing that ACPI_NUM_NOTIFY_TYPES == 2 you can > > see that the two least significant bits are checked. Still replace > > > > i + 1 > > > > by > > > > 1 << i > > > > which shouldn't make a relevant difference to compiler and compiled > > code, but is easier to understand for a human code reader. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > You need to submit this to ACPICA project first. > Documentation/driver-api/acpi/linuxized-acpica.rst explains the process. > Refer [1] for details for similar suggestion by Rafael. My motivation isn't big enough to even read that. If the usual kernel workflow doesn't work for ACPICA, let's drop the patch. Best regards Uwe
On Fri, Mar 24, 2023 at 11:59:38AM +0100, Uwe Kleine-König wrote: > > My motivation isn't big enough to even read that. If the usual kernel > workflow doesn't work for ACPICA, let's drop the patch. :(, but ACPICA is reused on other OSes and hence the need to be a separate project. It is very similar to the way DTC changes are done elsewhere and imported into the kernel. -- Regards, Sudeep
Hello, On Fri, Mar 24, 2023 at 02:38:49PM +0000, Sudeep Holla wrote: > On Fri, Mar 24, 2023 at 11:59:38AM +0100, Uwe Kleine-König wrote: > > > > My motivation isn't big enough to even read that. If the usual kernel > > workflow doesn't work for ACPICA, let's drop the patch. > > :(, but ACPICA is reused on other OSes and hence the need to be a separate > project. It is very similar to the way DTC changes are done elsewhere and > imported into the kernel. Feel free to pick up my patch. Best regards Uwe
diff --git a/drivers/acpi/acpica/evxface.c b/drivers/acpi/acpica/evxface.c index 18219abba108..d1b3411d2449 100644 --- a/drivers/acpi/acpica/evxface.c +++ b/drivers/acpi/acpica/evxface.c @@ -170,7 +170,7 @@ acpi_install_notify_handler(acpi_handle device, /* Install the handler at the list head(s) */ for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { - if (handler_type & (i + 1)) { + if (handler_type & (1 << i)) { handler_obj->notify.next[i] = obj_desc->common_notify.notify_list[i];
The loop for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { if (handler_type & (i + 1)) { ... } } looks strange. Only with knowing that ACPI_NUM_NOTIFY_TYPES == 2 you can see that the two least significant bits are checked. Still replace i + 1 by 1 << i which shouldn't make a relevant difference to compiler and compiled code, but is easier to understand for a human code reader. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/acpi/acpica/evxface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6