Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update

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

 



On Mon, Sep 11, 2017 at 11:58:43AM +0900, Andi Shyti wrote:
> Hi Ladislav,
> 
> > > > Serie was rebased on top of current linux.git, but something
> > > > happened there and my userspace decoder no longer works: driver
> > > > reports completely bogus timing such as (rc-5):
> > > > ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> > > > ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> > > > (^ used for pulse and _ for space)
> > > > As it has nothing to do with my changes, I'm sending it anyway
> > > > for review, which I do not expect to happen until merge window
> > > > ends.
> > > 
> > > This means that your patch is anyway untested.
> > 
> > Previous version is pretty well tested. GPIO IR stopped working
> > after pulling other changes from linux.git yesterday. And does not
> > work even without this patch set. I'll try to bisect later as omiting
> > linux-media merge did not fix it either.
> 
> OK

In all truth, changes were developed on top of 4.9.40, testing was done at
customer's site and for generally usefull changes usual attempt for merge
was done :-) I tried unmodified driver over weekend:
4.9.0: Not OK
4.9.40: OK
4.9.13: Not OK
linux.git: Not OK
Tested on IGEPv2 board based on DM3730, so something went wrong again.
Based on driver principle I suspect either ktime_get returning "strange"
values or interrupts are broken again (but that does not explain those
short pulses and spaces):
https://www.spinics.net/lists/linux-omap/msg135915.html
Verifying that would require some test setup with signal generator and
scope, as described in said email thread; or combine that test driver
with this one and look what is really happening. Unfortunately I'm not
able to do this any time soon, so if someone has hardware reliably working
with current mainline and is willing to test this patch serie I would
be very happy to see it happen.

> > > In any case I don't see much use if patch 1/10 as it doesn't
> > > simplify much, but the rest (from 2 to 10) looks good to me.
> > 
> > Just look at patch 9 and imagine how it would look without this
> > change. If you are still unconvinced I'll drop this change.
> 
> You don't need to, that's just my personal taste and I'm not
> strong with it. Patch 1 is quite common and not a big deal anyway.
> If you like it you like you can leave it :)
> 
> > > P.S. I don't see in this V2 the changelog from V1. Next time,
> > > please add the changelog.
> > 
> > It was just a rebase with conflicts resolved. I do not see how
> > to describe it better than I did.
> 
> You could write the above (i.e. V2 fixed rebase conflicts :) ).
> The reason is that as no change is stated, I have to anyway, as
> reviewer, compare side by side all patches to figure out if there
> is any difference (even if small) that is not expected.

Fair enough. Let's wait if there will any more comments and I'll do
better in V3.

> Thanks,
> Andi

Best regards,
	ladis



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux