RE: [PATCH v2] i2c: tegra: Add ACPI support

Akhil R
2021-11-24T07:18:48Z

> 23.11.2021 10:15, Akhil R пишет:
> > Add support for ACPI based device registration so that the driver can
> > be also enabled through ACPI table.
> >
> > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > ---
> > drivers/i2c/busses/i2c-tegra.c | 52
> > ++++++++++++++++++++++++++++++++----------
> > 1 file changed, 40 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-tegra.c
> > b/drivers/i2c/busses/i2c-tegra.c index c883044..8e47889 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -6,6 +6,7 @@
> > * Author: Colin Cross <ccross@android.com>
> > */
> >
> > +#include <linux/acpi.h>
> > #include <linux/bitfield.h>
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > @@ -608,6 +609,7 @@ static int tegra_i2c_wait_for_config_load(struct
> > tegra_i2c_dev *i2c_dev) static int tegra_i2c_init(struct
> > tegra_i2c_dev *i2c_dev) {
> > u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh,
> > non_hs_mode;
> > + acpi_handle handle = ACPI_HANDLE(i2c_dev->dev);
> > int err;
> >
> > /*
> > @@ -618,7 +620,11 @@ static int tegra_i2c_init(struct tegra_i2c_dev
> *i2c_dev)
> > * emit a noisy warning on error, which won't stay unnoticed and
> > * won't hose machine entirely.
> > */
> > - err = reset_control_reset(i2c_dev->rst);
> > + if (handle && acpi_has_method(handle, "_RST"))
>
> Which SoC version doesn't have "_RST" method? If neither, then please remove
> this check.
>
> > + err = (acpi_evaluate_object(handle, "_RST", NULL,
> > + NULL));
>
> Please remove parens around acpi_evaluate_object(). Why you added them?
>
> > + else
> > + err = reset_control_reset(i2c_dev->rst);
> > +
> > WARN_ON_ONCE(err);
> >
> > if (i2c_dev->is_dvc)
> > @@ -1627,12 +1633,12 @@ static void tegra_i2c_parse_dt(struct
> tegra_i2c_dev *i2c_dev)
> > bool multi_mode;
> > int err;
> >
> > - err = of_property_read_u32(np, "clock-frequency",
> > - &i2c_dev->bus_clk_rate);
> > + 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;
> >
> > - multi_mode = of_property_read_bool(np, "multi-master");
> > + multi_mode = device_property_read_bool(i2c_dev->dev,
> > + "multi-master");
> > i2c_dev->multimaster_mode = multi_mode;
> >
> > if (of_device_is_compatible(np, "nvidia,tegra20-i2c-dvc")) @@
> > -1642,10 +1648,25 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev
> *i2c_dev)
> > i2c_dev->is_vi = true;
> > }
> How are you going to differentiate the VI I2C from a non-VI? This doesn't look
> right.
This patch adds the ACPI support to only non-VI I2C. The device_ids in match table
are added accordingly. I suppose, of_device_is_compatible always returns false as
there is no device tree.
Agree with the other comments.