* [PATCH v1] platform/x86: wmi: initialize method name directly
@ 2021-09-13 11:08 Barnabás Pőcze
2021-09-13 11:53 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Barnabás Pőcze @ 2021-09-13 11:08 UTC (permalink / raw)
To: Hans de Goede, Mark Gross, platform-driver-x86
Instead of calling `snprintf()`, generate the method
name in the initializer. This way the array size
is determined automatically, specifying it explicitly
is no longer needed.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
---
drivers/platform/x86/wmi.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index e6997be206f1..1cde9dd417c4 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -184,15 +184,17 @@ static int get_subobj_info(acpi_handle handle, const char *pathname,
static acpi_status wmi_method_enable(struct wmi_block *wblock, bool enable)
{
- struct guid_block *block;
- char method[5];
+ acpi_handle handle = wblock->acpi_device->handle;
+ struct guid_block *block = &wblock->gblock;
acpi_status status;
- acpi_handle handle;
-
- block = &wblock->gblock;
- handle = wblock->acpi_device->handle;
+ char method[] = {
+ 'W',
+ 'E',
+ hex_asc_upper_hi(block->notify_id),
+ hex_asc_upper_lo(block->notify_id),
+ '\0'
+ };
- snprintf(method, 5, "WE%02X", block->notify_id);
status = acpi_execute_simple_method(handle, method, enable);
if (status == AE_NOT_FOUND)
return AE_OK;
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] platform/x86: wmi: initialize method name directly
2021-09-13 11:08 [PATCH v1] platform/x86: wmi: initialize method name directly Barnabás Pőcze
@ 2021-09-13 11:53 ` Hans de Goede
2021-09-13 12:15 ` Barnabás Pőcze
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-09-13 11:53 UTC (permalink / raw)
To: Barnabás Pőcze, Mark Gross, platform-driver-x86
Hi,
On 9/13/21 1:08 PM, Barnabás Pőcze wrote:
> Instead of calling `snprintf()`, generate the method
> name in the initializer. This way the array size
> is determined automatically, specifying it explicitly
> is no longer needed.
>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
In "platform/x86: wmi: introduce helper to generate method names"
you added a get_acpi_method_name() helper for generating WMI
method-names and you are using that everywhere else.
IMHO it would be better to also use that here. Is there any
specific reason why you are not using this here ?
Regards,
Hans
> ---
> drivers/platform/x86/wmi.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index e6997be206f1..1cde9dd417c4 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -184,15 +184,17 @@ static int get_subobj_info(acpi_handle handle, const char *pathname,
>
> static acpi_status wmi_method_enable(struct wmi_block *wblock, bool enable)
> {
> - struct guid_block *block;
> - char method[5];
> + acpi_handle handle = wblock->acpi_device->handle;
> + struct guid_block *block = &wblock->gblock;
> acpi_status status;
> - acpi_handle handle;
> -
> - block = &wblock->gblock;
> - handle = wblock->acpi_device->handle;
> + char method[] = {
> + 'W',
> + 'E',
> + hex_asc_upper_hi(block->notify_id),
> + hex_asc_upper_lo(block->notify_id),
> + '\0'
> + };
>
> - snprintf(method, 5, "WE%02X", block->notify_id);
> status = acpi_execute_simple_method(handle, method, enable);
> if (status == AE_NOT_FOUND)
> return AE_OK;
> --
> 2.33.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] platform/x86: wmi: initialize method name directly
2021-09-13 11:53 ` Hans de Goede
@ 2021-09-13 12:15 ` Barnabás Pőcze
2021-09-13 12:20 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Barnabás Pőcze @ 2021-09-13 12:15 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Gross, platform-driver-x86
Hi
2021. szeptember 13., hétfő 13:53 keltezéssel, Hans de Goede írta:
> In "platform/x86: wmi: introduce helper to generate method names"
> you added a get_acpi_method_name() helper for generating WMI
> method-names and you are using that everywhere else.
>
> IMHO it would be better to also use that here. Is there any
> specific reason why you are not using this here ?
Yes, indeed, but `get_acpi_method_name()` uses the `object_id` of the GUID block,
this one uses the `notify_id`. And it seemed problematic to find a simple
way to reconcile the differences, so I haven't really given it much thought.
I'm open to suggestions.
Regards,
Barnabás Pőcze
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] platform/x86: wmi: initialize method name directly
2021-09-13 12:15 ` Barnabás Pőcze
@ 2021-09-13 12:20 ` Hans de Goede
2021-09-13 12:39 ` Barnabás Pőcze
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-09-13 12:20 UTC (permalink / raw)
To: Barnabás Pőcze; +Cc: Mark Gross, platform-driver-x86
Hi,
On 9/13/21 2:15 PM, Barnabás Pőcze wrote:
> Hi
>
>
> 2021. szeptember 13., hétfő 13:53 keltezéssel, Hans de Goede írta:
>> In "platform/x86: wmi: introduce helper to generate method names"
>> you added a get_acpi_method_name() helper for generating WMI
>> method-names and you are using that everywhere else.
>>
>> IMHO it would be better to also use that here. Is there any
>> specific reason why you are not using this here ?
>
> Yes, indeed, but `get_acpi_method_name()` uses the `object_id` of the GUID block,
> this one uses the `notify_id`. And it seemed problematic to find a simple
> way to reconcile the differences, so I haven't really given it much thought.
> I'm open to suggestions.
Ah I see, TBH I liked your original patch just adding sizeof() better
then the new one. Or maybe replace the 2 "5"-s used in the snprintf
version with WMI_ACPI_METHOD_NAME_SIZE ?
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] platform/x86: wmi: initialize method name directly
2021-09-13 12:20 ` Hans de Goede
@ 2021-09-13 12:39 ` Barnabás Pőcze
2021-09-13 12:42 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Barnabás Pőcze @ 2021-09-13 12:39 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Gross, platform-driver-x86
Hi
2021. szeptember 13., hétfő 14:20 keltezéssel, Hans de Goede írta:
> Hi,
>
> On 9/13/21 2:15 PM, Barnabás Pőcze wrote:
> > Hi
> >
> >
> > 2021. szeptember 13., hétfő 13:53 keltezéssel, Hans de Goede írta:
> >> In "platform/x86: wmi: introduce helper to generate method names"
> >> you added a get_acpi_method_name() helper for generating WMI
> >> method-names and you are using that everywhere else.
> >>
> >> IMHO it would be better to also use that here. Is there any
> >> specific reason why you are not using this here ?
> >
> > Yes, indeed, but `get_acpi_method_name()` uses the `object_id` of the GUID block,
> > this one uses the `notify_id`. And it seemed problematic to find a simple
> > way to reconcile the differences, so I haven't really given it much thought.
> > I'm open to suggestions.
>
> Ah I see, TBH I liked your original patch just adding sizeof() better
> then the new one. Or maybe replace the 2 "5"-s used in the snprintf
> version with WMI_ACPI_METHOD_NAME_SIZE ?
> [...]
I will not resend it, but please feel free to apply the older patch if think it is better.
Personally, I don't like it, but oh well...
Best regards,
Barnabás Pőcze
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] platform/x86: wmi: initialize method name directly
2021-09-13 12:39 ` Barnabás Pőcze
@ 2021-09-13 12:42 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-09-13 12:42 UTC (permalink / raw)
To: Barnabás Pőcze; +Cc: Mark Gross, platform-driver-x86
Hi,
On 9/13/21 2:39 PM, Barnabás Pőcze wrote:
> Hi
>
>
> 2021. szeptember 13., hétfő 14:20 keltezéssel, Hans de Goede írta:
>> Hi,
>>
>> On 9/13/21 2:15 PM, Barnabás Pőcze wrote:
>>> Hi
>>>
>>>
>>> 2021. szeptember 13., hétfő 13:53 keltezéssel, Hans de Goede írta:
>>>> In "platform/x86: wmi: introduce helper to generate method names"
>>>> you added a get_acpi_method_name() helper for generating WMI
>>>> method-names and you are using that everywhere else.
>>>>
>>>> IMHO it would be better to also use that here. Is there any
>>>> specific reason why you are not using this here ?
>>>
>>> Yes, indeed, but `get_acpi_method_name()` uses the `object_id` of the GUID block,
>>> this one uses the `notify_id`. And it seemed problematic to find a simple
>>> way to reconcile the differences, so I haven't really given it much thought.
>>> I'm open to suggestions.
>>
>> Ah I see, TBH I liked your original patch just adding sizeof() better
>> then the new one. Or maybe replace the 2 "5"-s used in the snprintf
>> version with WMI_ACPI_METHOD_NAME_SIZE ?
>> [...]
>
> I will not resend it, but please feel free to apply the older patch if think it is better.
> Personally, I don't like it, but oh well...
In that case I'm just going to leave this as is, thank you for all the
(other) nice cleanups!
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-13 12:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 11:08 [PATCH v1] platform/x86: wmi: initialize method name directly Barnabás Pőcze
2021-09-13 11:53 ` Hans de Goede
2021-09-13 12:15 ` Barnabás Pőcze
2021-09-13 12:20 ` Hans de Goede
2021-09-13 12:39 ` Barnabás Pőcze
2021-09-13 12:42 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.