Re: [PATCH v5 07/11] platform/x86: int3472: Split into 2 drivers

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Tue, Nov 2, 2021 at 11:49 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The intel_skl_int3472.ko module contains 2 separate drivers,
> the int3472_discrete platform driver and the int3472_tps68470
> I2C-driver.
>
> These 2 drivers contain very little shared code, only
> skl_int3472_get_acpi_buffer() and skl_int3472_fill_cldb() are
> shared.
>
> Split the module into 2 drivers, linking the little shared code
> directly into both.
>
> This will allow us to add soft-module dependencies for the
> tps68470 clk, gpio and regulator drivers to the new
> intel_skl_int3472_tps68470.ko to help with probe ordering issues
> without causing these modules to get loaded on boards which only
> use the int3472_discrete platform driver.
>
> While at it also rename the .c and .h files to remove the
> cumbersome intel_skl_int3472_ prefix.

...

> +union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *id)
> +{
> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       acpi_handle handle = adev->handle;
> +       union acpi_object *obj;
> +       acpi_status status;
> +
> +       status = acpi_evaluate_object(handle, id, NULL, &buffer);
> +       if (ACPI_FAILURE(status))
> +               return ERR_PTR(-ENODEV);
> +
> +       obj = buffer.pointer;
> +       if (!obj)
> +               return ERR_PTR(-ENODEV);
> +
> +       if (obj->type != ACPI_TYPE_BUFFER) {
> +               acpi_handle_err(handle, "%s object is not an ACPI buffer\n", id);

> +               kfree(obj);

I'm wondering if we should use more of the ACPI_FREE() calls as
opposed to ACPI_ALLOCATE_BUFFER. Ditto for all such cases.

> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       return obj;
> +}

-- 
With Best Regards,
Andy Shevchenko