Re: [PATCH v2 1/4] lirc.4: timeout reports are enabled by default

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

 



Hello Sean,

On 11/4/18 1:26 PM, Sean Young wrote:
> Hi Micheal,
> 
> On Sun, Nov 04, 2018 at 12:17:08PM +0100, Michael Kerrisk (man-pages) wrote:
>> Hi Sean,
>>
>> On 11/3/18 12:22 PM, Sean Young wrote:
>>> Hi Michael,
>>>
>>> On Fri, Nov 02, 2018 at 12:43:41PM +0100, Michael Kerrisk (man-pages) wrote:
>>>> Hi Sean,
>>>>
>>>> On 11/02/2018 12:04 PM, Sean Young wrote:
>>>>> Signed-off-by: Sean Young <sean@xxxxxxxx>
>>>>> ---
>>>>>  man4/lirc.4 | 9 ++++++++-
>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/man4/lirc.4 b/man4/lirc.4
>>>>> index 526d8432c..6e9a94a66 100644
>>>>> --- a/man4/lirc.4
>>>>> +++ b/man4/lirc.4
>>>>> @@ -273,7 +273,14 @@ is 1) or disable
>>>>>  .RI ( val
>>>>>  is 0) timeout packages in
>>>>>  .BR LIRC_MODE_MODE2 .
>>>>> -By default, timeout reports should be turned off.
>>>>> +Each time the
>>>>> +.BR lirc
>>>>> +device is opened, timeout reports are enabled for that
>>>>> +file descriptor since kernel release 4.16.
>>>>> +In earlier releases, timeout reports were disabled by default, and once
>>>>> +enabled, enabled for all
>>>>> +.BR lirc
>>>>> +file descriptors until turned off again.
>>>>>  .TP
>>>>>  .BR LIRC_SET_REC_CARRIER " (\fIint\fP)"
>>>>>  Set the receive carrier frequency (Hz).
>>>>
>>>> I've applied this patch, and also tweaked the text somewhat.
>>>> Could you take a look please?
>>>
>>> Looks good I think. Thanks for fixing up my badly-phrases phrases. :)
>>
>> No problem :-). Thanks for all the patches.
>>>  
>>>> One thing I'm still not clear about... Since kernel 4.16,
>>>> does enabling LIRC_SET_REC_TIMEOUT on an FD referring to a
>>>> lirc device cause all subsequently opened FDs on the lirc
>>>> device to have timeouts enabled? The existing text suggests
>>>> that, but it sounds like odd behavior.
>>>
>>> The LIRC_SET_REC_TIMEOUT feature is maintained per-fd, so for each newly
>>> opened fd, it is enabled.
>>
>> Sorry, I'm still not clear. Let's look at the current
>> text, as it looks after my light edits of your patch:
>>
>>        LIRC_SET_REC_TIMEOUT_REPORTS (int)
>>               Enable (val is 1) or disable (val is 0) timeout packages in
>>               LIRC_MODE_MODE2.  The behavior of this operation has varied
>>               across kernel versions:
>>
>>               *  Since  Linux  4.16: each time the lirc device is opened,
>>                  timeout reports are enabled for that file descriptor.
>>
>>               *  In Linux 4.15 and earlier: timeout reports were disabled
>>                  by  default,  and  enabling them had effect for all lirc
>>                  file descriptors until timeouts were disabled again.
>>
>> The 4.16+ text makes it sound like:
>>
>> a) You use the ioctl() to set a timeout attribute on the lirc device,
>>    and
>> b) All subsequently opened FDs on that device inherit that attribute.
>>
>> But, I think that interpretation is not correct(?).
>>
>> So, explain to me, in Linux 4.16+ what code steps would I make
>> to open a lirc device and set the timeout. (And would those
>> steps have any effect on already opened FDs for the lirc device,
>> or for ant subsequently opened FDs?)
> 
> So, linux 4.16+:
> 	int p, fd = open("/dev/lirc0", O_RDWR);
> 	int reports = 0;
> 	//uncomment the following to turn timeout reports off
> 	//ioctl(fd, LIRC_SET_REC_TIMEOUT_REPORTS, &reports);
> 	while (read(fd, &p, sizeof(p)) == sizeof(p)) {
> 		if ((p & 0xff000000) == LIRC_MODE2_TIMEOUT) {
> 			printf("timeout of %d\n", p & 0xffffff);
> 		}
> 	}
> 
> So you _will_ always get these timeouts in 4.16+ unless you call the ioctl
> to turn them off, no matter what any other process has done in the system.
> 
> The rationale is that previously, lircd would turn timeout reports on
> globally, and all users space programs could deal with them fine; however,
> if lircd was not running, which is more common nowadays, timeout reports 
> were not being received. Basically you want these on.


So, I think the Linux 4.16+ text should read more like:

[[
Since  Linux  4.16: each time the lirc device is opened, timeout
reports are by default enabled for the resulting file descriptor.
The LIRC_SET_REC_TIMEOUT operation can be used to disable (and,
if desired, to later re-enable) the timeout on the file descriptor.
]]

Seem okay?

> The ioctl should really be removed.
> 
>> Also, I read the Linux <= 4.15 text to mean that enabling or
>> disabling timeouts on one lirc FDs affects all currently open
>> FDs for that lirc device, as well as any subsequently opened 
>> devices. Is that correct?
> 
> Yep.

So, I think the pre-4.16 behavior could also be further clarified.
How about something like:

[[
In Linux 4.15 and earlier: timeout reports are disabled
by  default,  and  enabling them on any file descriptor
associated with the lirc device has the effect of
enabling timeouts for all file descriptors referring to
that lirc device (until timeouts are disabled again).
]]

Seem okay?

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/



[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux