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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux