Re: [PATCH v2 3/6] ACPI: AC: Replace acpi_driver with platform_driver
From: Rafael J. Wysocki
Date: Mon Oct 09 2023 - 13:51:41 EST
On Mon, Oct 9, 2023 at 3:04 PM Wilczynski, Michal
<michal.wilczynski@xxxxxxxxx> wrote:
>
>
>
> On 10/9/2023 2:27 PM, Rafael J. Wysocki wrote:
> > On Mon, Oct 9, 2023 at 10:40 AM Wilczynski, Michal
> > <michal.wilczynski@xxxxxxxxx> wrote:
> >>
[cut]
> >> Yeah we could add platform device without removing acpi device, and
> >> yes that would introduce data duplication, like Andy noticed.
> > But he hasn't answered my question regarding what data duplication he
> > meant, exactly.
> >
> > So what data duplication do you mean?
>
> So what I meant was that many drivers would find it useful to have
> a struct device embedded in their 'private structure', and in that case
> there would be a duplication of platform_device and acpi_device as
> both pointers would be needed.
It all depends on how often each of them is going to be used in the
given driver.
This particular driver only needs a struct acpi_device pointer if I'm
not mistaken.
> Mind this if you only have struct device
> it's easy to get acpi device, but it's not the case the other way around.
>
> So for this driver it's just a matter of sticking to convention,
There is no convention in this respect and there is always a tradeoff
between using more memory and using more CPU time in computing in
general, but none of them should be wasted just for the sake of
following a convention.
> for the others like it would be duplication.
So I'm only talking about the driver modified by the patch at hand.
> In my version of this patch we managed to avoid this duplication, thanks
> to the contextual argument introduced before, but look at this patch:
> https://github.com/mwilczy/linux-pm/commit/cc8ef52707341e67a12067d6ead991d56ea017ca
>
> Author of this patch had to introduce platform_device and acpi_device to the struct ac, so
> there was some duplication. That is the case for many drivers to come, so I decided it's better
> to change this and have a pattern with keeping only 'struct device'.
Well, if the only thing you need from a struct device is its
ACPI_COMPANION(), it is better to store a pointer to the latter.