linux-kernel - Re: [PATCH v7 12/34] i2c: tegra: Use clk-bulk helpers

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]

Date:   Mon, 21 Sep 2020 13:01:59 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Dmitry Osipenko <digetx@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Wolfram Sang <wsa@...-dreams.de>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        linux-tegra@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 12/34] i2c: tegra: Use clk-bulk helpers

On Thu, Sep 17, 2020 at 04:54:28PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 2:38 PM Thierry Reding <thierry.reding@...il.com> wrote:
> > On Wed, Sep 09, 2020 at 01:39:44AM +0300, Dmitry Osipenko wrote:
> > > Use clk-bulk helpers and factor out clocks initialization into separate
> > > function in order to make code cleaner.
> > >
> > > The clocks initialization now performed after reset-control initialization
> > > in order to avoid a noisy -PROBE_DEFER errors on T186+ from the clk-bulk
> > > helper which doesn't silence this error code. Hence reset_control_get()
> > > now may return -EPROBE_DEFER on newer Tegra SoCs because they use BPMP
> > > driver that provides reset controls and BPMP doesn't come up early during
> > > boot. Previously rst was protected by the clocks retrieval and now this
> > > patch makes dev_err_probe() to be used for the rst error handling.
> > >
> > > Suggested-by: Andy Shevchenko <andy.shevchenko@...il.com>
> > > Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> > > ---
> > >  drivers/i2c/busses/i2c-tegra.c | 187 ++++++++++++---------------------
> > >  1 file changed, 67 insertions(+), 120 deletions(-)
> >
> > This is tempting from a diffstat point of view, but the downside is that
> > we can now no longer validate that all of the necessary clocks are given
> > in device tree.
> >
> > Previously the driver would fail to probe the I2C controller if any of
> > the expected clocks were not defined in device tree, but now it's just
> > going to continue without it and not give any indication as to what's
> > wrong.
> 
> You may print an error in the error path as previously. Since both
> clocks are mandatory (as far as I understood the code) user will need
> to check DT in any case.

The problem is that the number of required clocks depends on the variant
of the IP block that's implemented. Some require just one clock and
others require two or three. With this patch the driver is just going to
pick whatever clocks are given in device tree, but it removes any
possibility of detecting whether the device trees contain the correct
clocks. So we may very well run into a situation where the driver now
successfully probes but then malfunctions because one or more of the
clocks were not specified in device tree.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)