From: Ladislav Michl <ladis@linux-mips.org>
To: Andi Shyti <andi.shyti@samsung.com>
Cc: linux-media@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Sean Young <sean@mess.org>
Subject: Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
Date: Mon, 11 Sep 2017 09:13:32 +0200 [thread overview]
Message-ID: <20170911071332.r47nt5m26l6tsrpw@lenoch> (raw)
In-Reply-To: <20170911025843.GA31540@gangnam>
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
next prev parent reply other threads:[~2017-09-11 7:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20170907233401epcas4p4424e892b32d469233705af5014e20604@epcas4p4.samsung.com>
2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
2017-09-07 23:34 ` [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info Ladislav Michl
2017-09-07 23:35 ` [PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc Ladislav Michl
2017-09-07 23:36 ` [PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device Ladislav Michl
2017-09-07 23:36 ` [PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one Ladislav Michl
2017-09-07 23:37 ` [PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device Ladislav Michl
2017-09-07 23:37 ` [PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler Ladislav Michl
2017-09-07 23:38 ` [PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq Ladislav Michl
2017-09-07 23:39 ` [PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME Ladislav Michl
2017-09-07 23:39 ` [PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data Ladislav Michl
2017-09-07 23:41 ` [PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API Ladislav Michl
2017-09-08 2:21 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Andi Shyti
2017-09-08 8:16 ` Ladislav Michl
2017-09-11 2:58 ` Andi Shyti
2017-09-11 7:13 ` Ladislav Michl [this message]
2017-09-15 21:56 ` Ladislav Michl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170911071332.r47nt5m26l6tsrpw@lenoch \
--to=ladis@linux-mips.org \
--cc=andi.shyti@samsung.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=sean@mess.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox