On 9/4/2024 01:36, Andy Shevchenko wrote:
> On Wed, Sep 04, 2024 at 01:06:16AM +0530, Shyam Sundar S K wrote:
>> +Andy (this has some ACPI handling that adds AMD ASF support to the
>> existing piix4 driver for SMBus)
>
> Thanks.
>
>> On 8/22/2024 19:51, Shyam Sundar S K wrote:
>>> The AMD ASF controller is presented to the operating system as an ACPI
>>> device. The piix4 driver can obtain the ASF handle through ACPI to
>>> retrieve information about the ASF controller's attributes, such as the
>>> ASF address space and interrupt number, and to handle ASF interrupts.
>>>
>>> Currently, the piix4 driver assumes that a specific port address is
>>> designated for AUX operations. However, with the introduction of ASF, the
>>> same port address may also be used by the ASF controller. Therefore, a
>>> check needs to be added to ensure that if ASF is advertised and enabled in
>>> ACPI, the AUX port is not set up.
>
> ...
>
>>> +static acpi_status sb800_asf_acpi_resource_cb(struct acpi_resource *resource, void *context)
>>> +{
>>> + struct sb800_asf_data *data = context;
>>> +
>>> + switch (resource->type) {
>>> + case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
>>> + data->irq = resource->data.extended_irq.interrupts[0];
>>> + break;
>>> + case ACPI_RESOURCE_TYPE_IO:
>>> + data->addr = resource->data.io.minimum;
>>> + break;
>>> + }
>>> +
>>> + return AE_OK;
>>> +}
>>> +
>>> +static int sb800_asf_add_adap(struct pci_dev *dev)
>>> +{
>>> + struct i2c_piix4_adapdata *adapdata;
>>> + struct sb800_asf_data *data;
>>> + acpi_status status;
>>> + acpi_handle handle;
>>> + int ret;
>
>>> + status = acpi_get_handle(NULL, SB800_ASF_ACPI_PATH, &handle);
>>> + if (ACPI_FAILURE(status))
>>> + return -ENODEV;
>
>>> + data = devm_kzalloc(&dev->dev, sizeof(struct sb800_asf_data), GFP_KERNEL);
>>> + if (!data)
>>> + return -ENOMEM;
>
> Why can't it be on stack?
>
>>> + status = acpi_walk_resources(handle, METHOD_NAME__CRS, sb800_asf_acpi_resource_cb, data);
>>> + if (ACPI_FAILURE(status))
>>> + return -EINVAL;
>>> +
>>> + if (!data->addr)
>>> + return -EINVAL;
>
> This is reinvention of acpi_dev_get_resources(). Many drivers are using it, you
> may found a lot of examples.
Thank you for the quick feedback. I have submitted v2 addressing your
remarks. Kindly take a look.
Thanks,
Shyam
