Re: [PATCH 2/2] platform/x86/amd/hsmp: Split ACPI and non-ACPI code

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Fri, 7 Jun 2024, Suma Hegde wrote:

> Separate the probes for ACPI and non-ACPI supported platforms.
> Provide a Kconfig option to select either the
> ACPI or the non-ACPI (platform device based driver).
> 
> This change is done to
>  - Keep the probes clean of the if else ladder
>  - Use dev_groups in platform driver structure, instead of using
>    devm_device_add_group()
>    + use is_visible() to enable the sysfs entries
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>

This will be quite tedious to review in the current form because it 
combines mass-moving of code with non-move changes. I'd expect it to be 
significantly easier to review if the existing functions would be copied 
1:1 to the respective files first and then another change does the actual 
separation.

You might need to add a few extra function prototypes into the header in 
the intermediate state but I believe it's still going to be huge win even 
if you'll need to remove some of those prototypes when the actual split 
commences in the second change.

A few general comments:

- EXPORTs should be namespaced.

- Directly include <> headers in the individual files .c, don't mass 
include them through "hsmp.h" as clearly they'll be different for 
ACPI/non-ACPI to some extent.

-- 
 i.