| Message ID | 3089655.5fSG56mABF@kreacher (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | None | expand |
On 10/12/21 19:46, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael@kernel.org> > > The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION() > macro and the ACPI handle produced by the former comes from the > ACPI device object produced by the latter, so it is way more > straightforward to evaluate the latter directly instead of passing > the handle produced by the former to acpi_bus_get_device(). > > Modify mshw0011_notify() accordingly (no intentional functional > impact). > > Signed-off-by: Rafael J. Wysocki <rafael@kernel.org> Looks mostly good to me, small comment/question inline. > --- > drivers/platform/surface/surface3_power.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > Index: linux-pm/drivers/platform/surface/surface3_power.c > =================================================================== > --- linux-pm.orig/drivers/platform/surface/surface3_power.c > +++ linux-pm/drivers/platform/surface/surface3_power.c > @@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd > { > union acpi_object *obj; > struct acpi_device *adev; > - acpi_handle handle; > unsigned int i; > > - handle = ACPI_HANDLE(&cdata->adp1->dev); > - if (!handle || acpi_bus_get_device(handle, &adev)) > + adev = ACPI_COMPANION(&cdata->adp1->dev); > + if (!adev) > return -ENODEV; Do we need to get the ACPI device (adev) here? To me it looks like only its handle is actually used so why not keep ACPI_HANDLE() and remove the acpi_bus_get_device() call instead? > > - obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL, > - ACPI_TYPE_BUFFER); > + obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2, > + NULL, ACPI_TYPE_BUFFER); > if (!obj) { > dev_err(&cdata->adp1->dev, "device _DSM execution failed\n"); > return -ENODEV; > > > Regards, Max
On Tue, Oct 12, 2021 at 8:21 PM Maximilian Luz <luzmaximilian@gmail.com> wrote: > > On 10/12/21 19:46, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael@kernel.org> > > > > The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION() > > macro and the ACPI handle produced by the former comes from the > > ACPI device object produced by the latter, so it is way more > > straightforward to evaluate the latter directly instead of passing > > the handle produced by the former to acpi_bus_get_device(). > > > > Modify mshw0011_notify() accordingly (no intentional functional > > impact). > > > > Signed-off-by: Rafael J. Wysocki <rafael@kernel.org> > > Looks mostly good to me, small comment/question inline. > > > --- > > drivers/platform/surface/surface3_power.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > Index: linux-pm/drivers/platform/surface/surface3_power.c > > =================================================================== > > --- linux-pm.orig/drivers/platform/surface/surface3_power.c > > +++ linux-pm/drivers/platform/surface/surface3_power.c > > @@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd > > { > > union acpi_object *obj; > > struct acpi_device *adev; > > - acpi_handle handle; > > unsigned int i; > > > > - handle = ACPI_HANDLE(&cdata->adp1->dev); > > - if (!handle || acpi_bus_get_device(handle, &adev)) > > + adev = ACPI_COMPANION(&cdata->adp1->dev); > > + if (!adev) > > return -ENODEV; > > Do we need to get the ACPI device (adev) here? To me it looks like only > its handle is actually used so why not keep ACPI_HANDLE() and remove the > acpi_bus_get_device() call instead? It actually doesn't really matter, but you're right, acpi_bus_get_device() is simply redundant here, so ACPI_HANDLE() is sufficient. I'll send a v2 of this one. > > > > - obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL, > > - ACPI_TYPE_BUFFER); > > + obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2, > > + NULL, ACPI_TYPE_BUFFER); > > if (!obj) { > > dev_err(&cdata->adp1->dev, "device _DSM execution failed\n"); > > return -ENODEV; > > > > > > > > Regards, > Max
Index: linux-pm/drivers/platform/surface/surface3_power.c =================================================================== --- linux-pm.orig/drivers/platform/surface/surface3_power.c +++ linux-pm/drivers/platform/surface/surface3_power.c @@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd { union acpi_object *obj; struct acpi_device *adev; - acpi_handle handle; unsigned int i; - handle = ACPI_HANDLE(&cdata->adp1->dev); - if (!handle || acpi_bus_get_device(handle, &adev)) + adev = ACPI_COMPANION(&cdata->adp1->dev); + if (!adev) return -ENODEV; - obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL, - ACPI_TYPE_BUFFER); + obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2, + NULL, ACPI_TYPE_BUFFER); if (!obj) { dev_err(&cdata->adp1->dev, "device _DSM execution failed\n"); return -ENODEV;