linux-kernel - RE: [PATCH v2] i2c: tegra: Add ACPI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR12MB5273A7628D80076F4EF2CC69C0619@BN9PR12MB5273.namprd12.prod.outlook.com>
Date:   Wed, 24 Nov 2021 07:18:43 +0000
From:   Akhil R <akhilrajeev@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>,
        "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
        "christian.koenig@....com" <christian.koenig@....com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        "linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>
CC:     Shardar Mohammed <smohammed@...dia.com>
Subject: RE: [PATCH v2] i2c: tegra: Add ACPI support

> 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@...dia.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@...roid.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.