On Thu, Nov 25, 2021 at 6:54 PM Akhil R <akhilrajeev@xxxxxxxxxx> wrote: > > Add support for the ACPI based device registration so that the driver > can be also enabled through ACPI table. > > This does not include the ACPI support for Tegra VI and DVC I2C. Thanks for an update, my comments below. ... > - err = reset_control_reset(i2c_dev->rst); > + if (handle) > + err = acpi_evaluate_object(handle, "_RST", NULL, NULL); Does it compile for CONFIG_ACPI=n case? > + else > + err = reset_control_reset(i2c_dev->rst); If not, you will need something like this instead: #ifdef CONFIG_ACPI err = acpi_evaluate_object(ACPI_HANDLE(...), "_RST", NULL, NULL); #else err = reset_control_reset(i2c_dev->rst); #endif ... > + err = device_property_read_u32(i2c_dev->dev, "clock-frequency", > + &i2c_dev->bus_clk_rate); > if (err) > i2c_dev->bus_clk_rate = I2C_MAX_STANDARD_MODE_FREQ; Actually you need to switch to use i2c_timings data structure and corresponding methods. This change will be incorporated there. I.o.w. do it as a prerequisite to this patch. ... > + if (ACPI_HANDLE(i2c_dev->dev)) > + return 0; With above mentioned ifdeffery this may be converted back to has_acpi_companion() which is slightly better in this case. > + if (ACPI_HANDLE(i2c_dev->dev)) > + return 0; Ditto. P.S> Sorry if I missed something in the previous reviews. -- With Best Regards, Andy Shevchenko