Hi,
On 2/12/21 10:21 AM, Barnabás Pőcze wrote:
> Hi
>
>
> 2021. február 12., péntek 9:40 keltezéssel, Hans de Goede írta:
>
>> [...]
>> I like the series but I would like to see the 3th patch to go even
>> further wrt removing duplication between thinkpad_acpi and ideapad-laptop.
>>
>> The big difference between the 2 drivers is thinkpad_acpi relying on global
>> variables while ideapad-laptop stores everything in a driver data-struct.
>>
>> What you can do is add a struct which holds all the necessary data for the
>> dytc code, struct ideapad_dytc_priv is a start for this I guess. Lets say
>> we rename struct ideapad_dytc_priv to struct dytc_priv, then thinkpad_acpi.c
>> can have a:
>>
>> static struct dytc_priv *dytc_priv;
>>
>> And there can be a shared dytc probe function like this:
>>
>> static int dytc_profile_init(struct dytc_priv **dytc_priv_ret)
>> {
>> struct dytc_priv *dytc_priv;
>>
>> ...
>>
>> *dytc_priv_ret = dytc_priv;
>> return NULL;
>>
>> error:
>> // clean stuff
>> *dytc_priv_ret = NULL;
>> return err;
>> }
>>
>> Which thinkpad_acpi can then call on its global dytc_priv pointer:
>>
>> err = dytc_profile_init(&dytc_priv);
>>
>> Where as ideapad_laptop would use the pointer inside its data struct:
>>
>> err = dytc_profile_init(&priv->dytc);
>>
>>
>> I think this way we should be able to share almost all of the dytc code
>> not just the 2 convert functions.
>>
>
> How exactly do you imagine that? In separate (e.g. "lenovo-dytc") kernel module?
That would be an option in that case this module should have a non user
selectable Kconfig option and then be select-ed by both the thinkpad_acpi
and ideapad-laptop Kconfig bits. Note that the plan is to move to select for
ACPI_PLATFORM_PROFILE too, see:
https://github.com/linux-surface/kernel/commit/d849e0e069042cbc83636496f66c09e52db4eb01
But I'm fine with just having everything as static inline in a header too,
the overhead of having another module is likely going to mean that there
will be no disk-space saving and only one of the 2 will ever be loaded in
memory. So arguably just having a header with everything static inline
is more efficient and it means a simpler/cleaner Kconfig so I have a slight
preference for just having a header with all the functions as static inline
functions.
The goal here is source-code size reduction, compiled-code size reduction
is less important.
> And I assume platform profile registration/etc. should be done by shared code as well?
Yes (if possible).
Regards,
Hans
