linux-kernel - RE: [PATCH 2/3] Input: snvs_pwrkey

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 for Android: free password hash cracker in your pocket

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

Message-ID: <VE1PR04MB6638C6215F71FA2EE6C4E450891B0@VE1PR04MB6638.eurprd04.prod.outlook.com>
Date:   Wed, 12 Feb 2020 00:58:14 +0000
From:   Robin Gong <yibin.gong@....com>
To:     André Draszik <git@...red.net>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Horia Geanta <horia.geanta@....com>
CC:     "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 André Draszik <git@...red.net> wrote:
> Hi,
> 
> On Tue, 2020-02-11 at 01:54 +0000, Robin Gong wrote:
> > 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.
> 
> It seems to me though that the clock should really be moved into the (parent)
> SNVS node itself, rather than duplicating the clock in the the power key node
> and in the RTC node. Is that possible? (I don't know)...
That's possible but much code changes need to be added into snvs-rtc/snvs_pwrkey
driver. Independent patch is better.
> 
> To summarise, I'll post an updated patchset within the next couple days so to:
> 
> * keep the clock optional (for i.MX6 platforms)
> * convert to devm_clk_get_optional()
> * only enable the clock in interrupt handler and
> imx_imx_snvs_check_for_events()
>   but not during driver loading
> * update all four DTSIs: imx8mm.dtsi imx8mn.dtsi imx8mq.dtsi imx7s.dtsi
>   Note that I'll only be able to test onn the i.MX7
Correct.
> 
> 
> Cheers,
> Andre'
>