linux-kernel - RE: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle system

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <DB8PR04MB6795CB9F519D2BD277654B29E63E0@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Thu, 17 Sep 2020 09:12:32 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Sean Young <sean@...s.org>
CC:     "mchehab@...nel.org" <mchehab@...nel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle
 system

> -----Original Message-----
> From: Sean Young <sean@...s.org>
> Sent: 2020年9月15日 17:34
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: mchehab@...nel.org; linux-media@...r.kernel.org;
> linux-kernel@...r.kernel.org; dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle
> system
> 
> 

[...]
> > @@ -92,6 +113,12 @@ static int gpio_ir_recv_probe(struct
> > platform_device *pdev)
> >
> >  	platform_set_drvdata(pdev, gpio_dev);
> >
> > +
> > +	pm_runtime_set_autosuspend_delay(dev, (rcdev->timeout / 1000 /
> > +1000));
> 
> rcdev->timeout is in microseconds (since very recently), so this is wrong.
> Also, the timeout can be changed using the LIRC_SET_REC_TIMEOUT ioctl
> (using ir-ctl -t in userspace). The autosuspend delay should be updated when
> this happens. This can be done by implementing the s_timeout rcdev function.

Hi Sean,

I come across a problem when implementing this feature.

At probe stage, devm_rc_register_device -> change_protocol, then timeout set to 125ms.

When echo sony or nec to protocols, will call change_protocol changing the timeout value, that timeout would change to handler->min_timeout + 10ms. For sony is 16000000ns, for 15625000ns.
This is not the way I want to take before, this would frequently disable/enable cpuidle. So is it necessary to provide s_timeout, this callback should be used to change protocols' timeout?
If implement s_timeout, users need change the timeout value from userspace, this is a mandatory operation and unfriendly. And it will affect protocol's timeout.

Autosuspend delay should be fixed value, should be set to gpio device timeout value, which is 125ms.

Best Regards,
Joakim Zhang
> > +	pm_runtime_use_autosuspend(dev);
> > +	pm_runtime_set_suspended(dev);
> > +	pm_runtime_enable(dev);
> > +
> >  	return devm_request_irq(dev, gpio_dev->irq, gpio_ir_recv_irq,
> >  				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> >  				"gpio-ir-recv-irq", gpio_dev);