linux-kernel - Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices

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: <20161027143601.GA5103@gofer.mess.org>
Date:   Thu, 27 Oct 2016 15:36:01 +0100
From:   Sean Young <sean@...s.org>
To:     Andi Shyti <andi.shyti@...sung.com>
Cc:     Mauro Carvalho Chehab <mchehab@....samsung.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Andi Shyti <andi@...zian.org>,
        David Härdeman <david@...deman.nu>
Subject: Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any
 transmitting time for tx only devices

On Thu, Oct 27, 2016 at 04:44:01PM +0900, Andi Shyti wrote:
> Hi Sean,
> 
> it's been a while :)
> 
> I was going through your review fixing what needs to be fixed,
> but...
> 
> > > @@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
> > >  	}
> > >  
> > >  	ret = dev->tx_ir(dev, txbuf, count);
> > > -	if (ret < 0)
> > > +	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)
> > 
> > Just because a driver only does transmit doesn't mean its transmit ABI
> > should change.
> > 
> > Now this bit of code is pretty horrible. It ensures that the call to write()
> > takes at least as long as the length of the transmit IR by sleeping. That's
> > not much of a guarantee that the IR has been sent.
> > 
> > Note that in the case of ir-spi, since your spi transfer is sync no sleep
> > should be introduced here.
> > 
> > The gap calculation in lirc checks that if the call to write() took _longer_
> > than expected wait before sending the next IR code (when either multiple
> > IR codes or repeats are specified). Introducing the sleep in the kernel
> > here does not help at all, lirc already ensures that it waits as long as
> > the IR is long (see schedule_repeat_timer in lirc).
> > 
> > This change was introduced in 3.10, commit f8e00d5. 
> 
> ... I'm not sure what can be done here. I get your point and I
> understand that this indeed is a kind of fake sync point and by
> doing this I 

My original plan was to send a patch which just removes the silly wait,
but on further investigating debian stable and testing still carry a
lirc version that depend on it, so that's not going to fly.

> How about creating two different functions:
> 
> - ir_lirc_transmit_ir where we actually do what the function
>   already does
> - ir_lirc_transmit_no_sync where the function we don't wait
>   because the the sync is done on a different level (for example
>   in the SPI case).
> 
> SPI does approximately the same thing.

Since we have to be able to switch between waiting and not waiting, 
we need some sort of ABI for this. I think this warrants a new ioctl;
I'm not sure how else it can be done. I'll be sending out a patch
shortly.


Sean