[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <VE1PR04MB6638761F5F8549C6528FE6B989180@VE1PR04MB6638.eurprd04.prod.outlook.com>
Date: Tue, 11 Feb 2020 01:54:58 +0000
From: Robin Gong <yibin.gong@....com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Horia Geanta <horia.geanta@....com>
CC: André Draszik <git@...red.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Anson Huang <anson.huang@....com>,
Aymen Sghaier <aymen.sghaier@....com>,
Herbert Xu <herbert@...dor.apana.org.au>,
"David S. Miller" <davem@...emloft.net>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH 2/3] Input: snvs_pwrkey - enable snvs clock as needed
On 2020/02/11 Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
> On Mon, Feb 10, 2020 at 06:33:30AM +0000, Horia Geanta wrote:
> > On 2/10/2020 4:03 AM, Robin Gong wrote:
> > > On 2020/02/10 Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
> > >> On Fri, Feb 07, 2020 at 08:10:22AM +0000, Horia Geanta wrote:
> > >>> On 1/30/2020 10:45 PM, André Draszik wrote:
> > >>>> @@ -140,6 +148,25 @@ static int imx_snvs_pwrkey_probe(struct
> > >> platform_device *pdev)
> > >>>> if (pdata->irq < 0)
> > >>>> return -EINVAL;
> > >>>>
> > >>>> + pdata->clk = devm_clk_get(&pdev->dev, "snvs-pwrkey");
> > >>>> + if (IS_ERR(pdata->clk)) {
> > >>>> + pdata->clk = NULL;
> > >>> Using devm_clk_get_optional() would simplify error handling.
> > >>
> > >> It sounds to me that this clock is not at all optional and the
> > >> driver currently "works" only by accident and therefore optional is not
> suitable here.
> > > Yes, then we need to add all snvs clk in dts for on legacy i.MX
> > > chips in this patchset to avoid any potential function broken.
>
> How many are there? I am not too terribly opposed of having the driver handle
> missing clk if there are very many legacy DTSes out there. But then we need to
> handle it properly (i.e. current iteration does not handle referral properly for
> example).
There are four dtsi which have clock support in snvs-rtc including i.mx7s/i.mx8mq/8mm/8mn. So for this patch set, it's better update
i.mx8mX dtsi except i.mx7s.
>
> > In that case the DT binding should be updated too, to make the clock
> > mandatory.
>
> I think this should be done in either case, as as far I understand the part can not
> function without the clock and it worked purely by chance on some systems as
> something else was turning the clock on.
Yes, for all chips snvs clk management added, snvs clock also has to been add
in snvs_pwrkey dts, but for others legacy chips like i.mx6X which have no snvs
clk management, snvs clock is always on, so no need such clk in snvs_pwrkey
dts either, optional is better.
>
> Thanks.
>
> --
> Dmitry
Powered by blists - more mailing lists