On Fri, Nov 19, 2021 at 3:37 PM Akhil R <akhilrajeev@xxxxxxxxxx> wrote:
>
> Add support for ACPI based device registration so that the driver
> can be also enabled through ACPI table.
the ACPI
...
> + if (has_acpi_companion(i2c_dev->dev)) {
You are checkin for the companion and using a handle, why not check
for a handle explicitly?
> + acpi_evaluate_object(ACPI_HANDLE(i2c_dev->dev), "_RST",
> + NULL, NULL);
> + } else {
> + err = reset_control_reset(i2c_dev->rst);
> + WARN_ON_ONCE(err);
> + }
...
> + if (i2c_dev->nclocks == 0)
> + return;
Why? Make clocks optional.
...
> - i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
> - if (IS_ERR(i2c_dev->rst)) {
> - dev_err_probe(i2c_dev->dev, PTR_ERR(i2c_dev->rst),
> - "failed to get reset control\n");
> - return PTR_ERR(i2c_dev->rst);
Besides the fact this should be as simple as
return dev_err_probe(...)
> - }
> + if (!has_acpi_companion(&pdev->dev)) {
...why do you do this?
> + i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
> + if (IS_ERR(i2c_dev->rst)) {
> + dev_err_probe(i2c_dev->dev, PTR_ERR(i2c_dev->rst),
> + "failed to get reset control\n");
> + return PTR_ERR(i2c_dev->rst);
> + }
...
> +static const struct acpi_device_id tegra_i2c_acpi_match[] = {
> + {.id = "NVDA0101", .driver_data = (kernel_ulong_t)&tegra210_i2c_hw},
> + {.id = "NVDA0201", .driver_data = (kernel_ulong_t)&tegra186_i2c_hw},
> + {.id = "NVDA0301", .driver_data = (kernel_ulong_t)&tegra194_i2c_hw},
> + { },
No comma for the terminator entry.
> +};
--
With Best Regards,
Andy Shevchenko
