[v2,1/1] i2c: mediatek: add runtime PM operations and bus regulator control
Andi Shyti
andi.shyti at kernel.org
Tue Nov 19 14:08:57 PST 2024
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
More information about the Linux-mediatek
mailing list