platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "Armin Wolf" <W_Armin@gmx.de>, "Barnabás Pőcze" <pobrn@protonmail.com>
Cc: markgross@kernel.org, rafael@kernel.org, lenb@kernel.org,
	hmh@hmh.eng.br, matan@svgalib.org, corentin.chary@gmail.com,
	jeremy@system76.com, productdev@system76.com,
	platform-driver-x86@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/5] ACPI: battery: Allow battery hooks to be registered multiple times.
Date: Mon, 19 Sep 2022 12:18:53 +0100	[thread overview]
Message-ID: <a60266df-bf73-57e7-129a-f5395d373f1f@redhat.com> (raw)
In-Reply-To: <155062a9-8d1a-e771-1bee-35580b1b2b73@gmx.de>

Hi,

On 9/12/22 18:29, Armin Wolf wrote:
> Am 12.09.22 um 18:42 schrieb Barnabás Pőcze:
> 
>> Hi
>>
>> 2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
>>
>>> Registering multiple instances of a battery hook is beneficial
>>> for drivers which can be instantiated multiple times. Until now,
>>> this would mean that such a driver would have to implement some
>>> logic to manage battery hooks.
>>>
>>> Extend the battery hook handling instead.
>> I think this is already possible by embedding the acpi_battery_hook
>> object inside the driver's device specific data object, no?
>>
>> Regards,
>> Barnabás Pőcze
>>
>>
> Yes, it indeed is. However afaik it is not possible to pass instance-specific
> data to such an embedded battery hook. It could be possible by passing the
> battery hook as an argument to add_battery()/remove_battery() and using container_of(),
> but in my opinion this would be too much of a quick hack.

Actually thinking more about this (after my reviewed-by of 4/5) I believe
that leaving the lifetime management of the struct acpi_battery_hook hook
in the caller and then modifying 4/4 to pass the hook to the callback,
so that the callback can indeed do container_of to get its top-level
driver-data struct would be a better/cleaner solution then this patch +
patch 4/5 .

Doing things this way is quite normal in the kernel and doing a single
large alloc is better then a bunch of small allocs. In this case it does
not really matter, but if we do things like this over all drivers
eventually all the small mallocs add up.

Doing things this way would also reduce the amount of churn in this
series a bit.

Regards,

Hans


  reply	other threads:[~2022-09-19 11:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12 12:53 [PATCH 0/5] platform/x86: dell: Add new dell-wmi-ddv driver Armin Wolf
2022-09-12 12:53 ` [PATCH 1/5] ACPI: battery: Do not unload battery hooks on single error Armin Wolf
2022-09-19 10:42   ` Hans de Goede
2022-09-19 16:27     ` Rafael J. Wysocki
2022-09-19 19:12       ` Armin Wolf
2022-09-19 20:35         ` Armin Wolf
2022-09-27 14:29           ` Hans de Goede
2022-09-27 15:44             ` Armin Wolf
2022-09-12 12:53 ` [PATCH 2/5] ACPI: battery: Simplify battery_hook_unregister() Armin Wolf
2022-09-19 10:42   ` Hans de Goede
2022-09-12 12:53 ` [PATCH 3/5] ACPI: battery: Allow battery hooks to be registered multiple times Armin Wolf
2022-09-12 16:42   ` Barnabás Pőcze
2022-09-12 17:29     ` Armin Wolf
2022-09-19 11:18       ` Hans de Goede [this message]
2022-09-19 17:42       ` Barnabás Pőcze
2022-09-12 12:53 ` [PATCH 4/5] ACPI: battery: Allow for passing data to battery hooks Armin Wolf
2022-09-19 11:08   ` Hans de Goede
2022-09-12 12:53 ` [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver Armin Wolf
2022-09-12 21:56   ` Randy Dunlap
2022-09-13 14:40     ` Armin Wolf
2022-09-13 16:08       ` Limonciello, Mario
2022-09-13 16:45         ` Armin Wolf
2022-09-13 18:27         ` Randy Dunlap
2022-09-13 18:30           ` Limonciello, Mario
2022-09-13 18:37             ` Randy Dunlap
2022-09-19 11:33   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a60266df-bf73-57e7-129a-f5395d373f1f@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=W_Armin@gmx.de \
    --cc=corentin.chary@gmail.com \
    --cc=hmh@hmh.eng.br \
    --cc=jeremy@system76.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=matan@svgalib.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=pobrn@protonmail.com \
    --cc=productdev@system76.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).