Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 21 Sep 2020 13:01:59 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Dmitry Osipenko <digetx@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Laxman Dewangan <ldewangan@nvidia.com>,
        Wolfram Sang <wsa@the-dreams.de>,
        =?utf-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= <mirq-linux@rere.qmqm.pl>,
        linux-i2c <linux-i2c@vger.kernel.org>,
        linux-tegra@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 12/34] i2c: tegra: Use clk-bulk helpers
Message-ID: <20200921110159.GD3950626@ulmo>
References: <20200908224006.25636-1-digetx@gmail.com>
 <20200908224006.25636-13-digetx@gmail.com>
 <20200917113846.GX3515672@ulmo>
 <CAHp75Vd_B9WLM99LnN2YiZ045z8SUkD7KwY2wVEUeQ9Mx7fg5g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="XvKFcGCOAo53UbWW"
Content-Disposition: inline
In-Reply-To: <CAHp75Vd_B9WLM99LnN2YiZ045z8SUkD7KwY2wVEUeQ9Mx7fg5g@mail.gmail.com>
User-Agent: Mutt/1.14.6 (2020-07-11)
Precedence: bulk


--XvKFcGCOAo53UbWW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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@gmail.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 separa=
te
> > > function in order to make code cleaner.
> > >
> > > The clocks initialization now performed after reset-control initializ=
ation
> > > in order to avoid a noisy -PROBE_DEFER errors on T186+ from the clk-b=
ulk
> > > 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 du=
ring
> > > boot. Previously rst was protected by the clocks retrieval and now th=
is
> > > patch makes dev_err_probe() to be used for the rst error handling.
> > >
> > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Dmitry Osipenko <digetx@gmail.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.
>=20
> 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

--XvKFcGCOAo53UbWW
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl9oiCcACgkQ3SOs138+
s6HRLRAAqudWD8fZOoMwPwHlt5QeVrLryYwiP3koFDLCqhq7vF3gY8jr2HqwiQGg
Hf7z1iDwNdIzA0r6VsP03PnF7IwJD85xLMZedcvmDAxqVvVJzb+lEfQnY2ECeOAb
vKZI+XrAnXoksC4pAPYBzrfZpx8Sc81AMEUK6T9nhhc6UC3i1xfXsmPFdZx1S7wG
qsKAjFXzgxLFQ6rfdEUnuoU+vunl6K5f+fHAl9fHbNt2dlAjdvvcm3uT3D4cgbly
Bo7qtaqg0Zl5nJUCg0bdibW5y10Ea+3JqH0+O1MvZMMQk5oX42AZktoWby34sDEX
lM3TcFj6gDmdB0+PnKlegkH5M3MfxLAiYaAgs3gIfGlNnPWpX0LmDN+LEVQavZzp
S7aVSqY2tLplNLLyCiFd2X1kEAimkSqbUUxm+9BEANQww1Ib/LlHda42aMDEMjQL
mawgAASVt7I0t0e0r7/t99opEHETFlJAn+2G/HDx1Xpjio08ZYVYiIqzGB31jKFz
CmJqVbqAOuJSFb7ajiDE890T2XGMNOBj3N4TVjRi7BzBBEL6bk0dIS8g6hdVNiU4
IbcQzDt1ZDYE4rN7RSThJo/BhMc/PCNSoj2xZehiw2pjij+q9nlJnD9SUyT5ceSU
kItzaiXbL0QjsN3nIOxlh0+GIIX/UlvEDAIIN0j/NYvcWSDQmIY=
=9ExH
-----END PGP SIGNATURE-----

--XvKFcGCOAo53UbWW--