linux-kernel - Re: [v2,1/1] i2c: mediatek: add runtime PM operations and bus regulator control

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: <yi4ir6znmqjxvrbcbwnioxkdqyhsv74iwlridwh3nn7wmaat56@nqbqaa5zjil3>
Date: Tue, 19 Nov 2024 23:08:57 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Zoie Lin (林禹妡) <Zoie.Lin@...iatek.com>
Cc: "matthias.bgg@...il.com" <matthias.bgg@...il.com>, 
	Qii Wang (王琪) <Qii.Wang@...iatek.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>, 
	Teddy Chen (陳乾元) <Teddy.Chen@...iatek.com>, "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>, 
	Project_Global_Chrome_Upstream_Group <Project_Global_Chrome_Upstream_Group@...iatek.com>
Subject: Re: [v2,1/1] i2c: mediatek: add runtime PM operations and bus
 regulator control

Hi Zoie,

...

> > > @@ -1486,19 +1528,22 @@ static int mtk_i2c_probe(struct
> > > platform_device *pdev)
> > >       if (ret < 0) {
> > >               dev_err(&pdev->dev,
> > >                       "Request I2C IRQ %d fail\n", irq);
> > > -             goto err_bulk_unprepare;
> > > +             goto err_clk_bulk_unprepare;
> > >       }
> > > +     pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
> > 
> > You had comments from me and from Andi on this delay, and you
> > completely ignored
> > both of us.
> > 
> > We're still waiting for an answer to our question.
> 
> I am sorry for any confusion caused by my previous response.
> The response to your question was included in the cover letter, which
> might not have been very noticeable.
>  
> The delay before runtime_put_autosuspend() actually executes
> mtk_i2c_runtime_suspend() depends on the frequency of I2C usage by the
> devices attached to this bus. A 1000ms delay is a balanced value for
> latency and power metrics based on the MTK platform.

Still you need to make it clear in the code. Please add a good
comment to describe why this delay is needed and where this 1000
has been found.

This is a constant that is not written in the datasheet, please
report how you have achieved this result. I want to avoid
arbitrary numbers inherited from the development phase.

Thanks,
Andi