Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] media: imon_raw: simplify and explain bit operations
@ 2019-08-10 11:44 Sean Young
  2019-08-10 11:44 ` [PATCH 2/3] media: imon_raw: prevent "nonsensical timing event of duration 0" Sean Young
  2019-08-10 11:44 ` [PATCH 3/3] selftests: ir: fix ir_loopback test failure Sean Young
  0 siblings, 2 replies; 5+ messages in thread
From: Sean Young @ 2019-08-10 11:44 UTC (permalink / raw)
  To: linux-media

This code needs some explanation.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/imon_raw.c | 43 +++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
index 25e56c5b13c0..e6723993b466 100644
--- a/drivers/media/rc/imon_raw.c
+++ b/drivers/media/rc/imon_raw.c
@@ -14,7 +14,7 @@ struct imon {
 	struct device *dev;
 	struct urb *ir_urb;
 	struct rc_dev *rcdev;
-	u8 ir_buf[8] __aligned(__alignof__(u64));
+	__be64 ir_buf;
 	char phys[64];
 };
 
@@ -29,14 +29,35 @@ struct imon {
 static void imon_ir_data(struct imon *imon)
 {
 	struct ir_raw_event rawir = {};
-	u64 d = be64_to_cpup((__be64 *)imon->ir_buf) >> 24;
+	u64 data = be64_to_cpu(imon->ir_buf);
+	u8 packet_no = data & 0xff;
 	int offset = 40;
 	int bit;
 
-	dev_dbg(imon->dev, "data: %*ph", 8, imon->ir_buf);
+	if (packet_no == 0xff)
+		return;
+
+	dev_dbg(imon->dev, "data: %*ph", 8, &imon->ir_buf);
+
+	/*
+	 * Only the first 5 bytes contain IR data. Right shift so we move
+	 * the IR bits to the lower 40 bits.
+	 */
+	data >>= 24;
 
 	do {
-		bit = fls64(d & (BIT_ULL(offset) - 1));
+		/*
+		 * Find highest set bit which is less or equal to offset
+		 *
+		 * offset is the bit above (base 0) where we start looking.
+		 *
+		 * data & (BIT_ULL(offset) - 1) masks off any unwanted bits,
+		 * so we have just bits less than offset.
+		 *
+		 * fls will tell us the highest bit set plus 1 (or 0 if no
+		 * bits are set).
+		 */
+		bit = fls64(data & (BIT_ULL(offset) - 1));
 		if (bit < offset) {
 			dev_dbg(imon->dev, "pulse: %d bits", offset - bit);
 			rawir.pulse = true;
@@ -49,7 +70,12 @@ static void imon_ir_data(struct imon *imon)
 			offset = bit;
 		}
 
-		bit = fls64(~d & (BIT_ULL(offset) - 1));
+		/*
+		 * Find highest clear bit which is less than offset.
+		 *
+		 * Just invert the data and use same trick as above.
+		 */
+		bit = fls64(~data & (BIT_ULL(offset) - 1));
 		dev_dbg(imon->dev, "space: %d bits", offset - bit);
 
 		rawir.pulse = false;
@@ -59,7 +85,7 @@ static void imon_ir_data(struct imon *imon)
 		offset = bit;
 	} while (offset > 0);
 
-	if (imon->ir_buf[7] == 0x0a) {
+	if (packet_no == 0x0a) {
 		ir_raw_event_set_idle(imon->rcdev, true);
 		ir_raw_event_handle(imon->rcdev);
 	}
@@ -72,8 +98,7 @@ static void imon_ir_rx(struct urb *urb)
 
 	switch (urb->status) {
 	case 0:
-		if (imon->ir_buf[7] != 0xff)
-			imon_ir_data(imon);
+		imon_ir_data(imon);
 		break;
 	case -ECONNRESET:
 	case -ENOENT:
@@ -129,7 +154,7 @@ static int imon_probe(struct usb_interface *intf,
 	imon->dev = &intf->dev;
 	usb_fill_int_urb(imon->ir_urb, udev,
 			 usb_rcvintpipe(udev, ir_ep->bEndpointAddress),
-			 imon->ir_buf, sizeof(imon->ir_buf),
+			 &imon->ir_buf, sizeof(imon->ir_buf),
 			 imon_ir_rx, imon, ir_ep->bInterval);
 
 	rcdev = devm_rc_allocate_device(&intf->dev, RC_DRIVER_IR_RAW);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] media: imon_raw: prevent "nonsensical timing event of duration 0"
  2019-08-10 11:44 [PATCH 1/3] media: imon_raw: simplify and explain bit operations Sean Young
@ 2019-08-10 11:44 ` Sean Young
  2019-08-10 11:44 ` [PATCH 3/3] selftests: ir: fix ir_loopback test failure Sean Young
  1 sibling, 0 replies; 5+ messages in thread
From: Sean Young @ 2019-08-10 11:44 UTC (permalink / raw)
  To: linux-media

Sometimes the device sends IR data which is all space, no pulses whatsoever.
Add the end of this the driver will put the rc device into idle mode when
it already is in idle mode. The following will be logged:

rc rc0: nonsensical timing event of duration 0
rc rc0: two consecutive events of type space

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/imon_raw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
index e6723993b466..d4aedcf76418 100644
--- a/drivers/media/rc/imon_raw.c
+++ b/drivers/media/rc/imon_raw.c
@@ -85,7 +85,7 @@ static void imon_ir_data(struct imon *imon)
 		offset = bit;
 	} while (offset > 0);
 
-	if (packet_no == 0x0a) {
+	if (packet_no == 0x0a && !imon->rcdev->idle) {
 		ir_raw_event_set_idle(imon->rcdev, true);
 		ir_raw_event_handle(imon->rcdev);
 	}
-- 
2.21.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] selftests: ir: fix ir_loopback test failure
  2019-08-10 11:44 [PATCH 1/3] media: imon_raw: simplify and explain bit operations Sean Young
  2019-08-10 11:44 ` [PATCH 2/3] media: imon_raw: prevent "nonsensical timing event of duration 0" Sean Young
@ 2019-08-10 11:44 ` Sean Young
  2019-08-12 14:25   ` shuah
  1 sibling, 1 reply; 5+ messages in thread
From: Sean Young @ 2019-08-10 11:44 UTC (permalink / raw)
  To: linux-media; +Cc: Shuah Khan

The decoder is called rc-mm, not rcmm. This was renamed late in the cycle
so this bug crept in.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Sean Young <sean@mess.org>
---
 tools/testing/selftests/ir/ir_loopback.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ir/ir_loopback.c b/tools/testing/selftests/ir/ir_loopback.c
index e700e09e3682..af7f9c7d59bc 100644
--- a/tools/testing/selftests/ir/ir_loopback.c
+++ b/tools/testing/selftests/ir/ir_loopback.c
@@ -54,9 +54,9 @@ static const struct {
 	{ RC_PROTO_RC6_MCE, "rc-6-mce", 0x00007fff, "rc-6" },
 	{ RC_PROTO_SHARP, "sharp", 0x1fff, "sharp" },
 	{ RC_PROTO_IMON, "imon", 0x7fffffff, "imon" },
-	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rcmm" },
-	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rcmm" },
-	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rcmm" },
+	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rc-mm" },
+	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rc-mm" },
+	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rc-mm" },
 };
 
 int lirc_open(const char *rc)
-- 
2.21.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] selftests: ir: fix ir_loopback test failure
  2019-08-10 11:44 ` [PATCH 3/3] selftests: ir: fix ir_loopback test failure Sean Young
@ 2019-08-12 14:25   ` shuah
  2019-08-13  7:37     ` Sean Young
  0 siblings, 1 reply; 5+ messages in thread
From: shuah @ 2019-08-12 14:25 UTC (permalink / raw)
  To: Sean Young, linux-media, open list:KERNEL SELFTEST FRAMEWORK, shuah

On 8/10/19 5:44 AM, Sean Young wrote:
> The decoder is called rc-mm, not rcmm. This was renamed late in the cycle
> so this bug crept in.
> 
> Cc: Shuah Khan <shuah@kernel.org>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
>   tools/testing/selftests/ir/ir_loopback.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/ir/ir_loopback.c b/tools/testing/selftests/ir/ir_loopback.c
> index e700e09e3682..af7f9c7d59bc 100644
> --- a/tools/testing/selftests/ir/ir_loopback.c
> +++ b/tools/testing/selftests/ir/ir_loopback.c
> @@ -54,9 +54,9 @@ static const struct {
>   	{ RC_PROTO_RC6_MCE, "rc-6-mce", 0x00007fff, "rc-6" },
>   	{ RC_PROTO_SHARP, "sharp", 0x1fff, "sharp" },
>   	{ RC_PROTO_IMON, "imon", 0x7fffffff, "imon" },
> -	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rcmm" },
> -	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rcmm" },
> -	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rcmm" },
> +	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rc-mm" },
> +	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rc-mm" },
> +	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rc-mm" },
>   };
>   
>   int lirc_open(const char *rc)
> 

Thanks Sean! Please cc - linux-keseltest makling list on these patches.

I can take this through my tree or here is my Ack for it go through
media tree

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] selftests: ir: fix ir_loopback test failure
  2019-08-12 14:25   ` shuah
@ 2019-08-13  7:37     ` Sean Young
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Young @ 2019-08-13  7:37 UTC (permalink / raw)
  To: shuah; +Cc: linux-media, open list:KERNEL SELFTEST FRAMEWORK

On Mon, Aug 12, 2019 at 08:25:41AM -0600, shuah wrote:
> On 8/10/19 5:44 AM, Sean Young wrote:
> > The decoder is called rc-mm, not rcmm. This was renamed late in the cycle
> > so this bug crept in.
> > 
> > Cc: Shuah Khan <shuah@kernel.org>
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >   tools/testing/selftests/ir/ir_loopback.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ir/ir_loopback.c b/tools/testing/selftests/ir/ir_loopback.c
> > index e700e09e3682..af7f9c7d59bc 100644
> > --- a/tools/testing/selftests/ir/ir_loopback.c
> > +++ b/tools/testing/selftests/ir/ir_loopback.c
> > @@ -54,9 +54,9 @@ static const struct {
> >   	{ RC_PROTO_RC6_MCE, "rc-6-mce", 0x00007fff, "rc-6" },
> >   	{ RC_PROTO_SHARP, "sharp", 0x1fff, "sharp" },
> >   	{ RC_PROTO_IMON, "imon", 0x7fffffff, "imon" },
> > -	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rcmm" },
> > -	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rcmm" },
> > -	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rcmm" },
> > +	{ RC_PROTO_RCMM12, "rcmm-12", 0x00000fff, "rc-mm" },
> > +	{ RC_PROTO_RCMM24, "rcmm-24", 0x00ffffff, "rc-mm" },
> > +	{ RC_PROTO_RCMM32, "rcmm-32", 0xffffffff, "rc-mm" },
> >   };
> >   int lirc_open(const char *rc)
> > 
> 
> Thanks Sean! Please cc - linux-keseltest makling list on these patches.

I'll do that next time, thanks.

> I can take this through my tree or here is my Ack for it go through
> media tree
> 
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>

I'm just preparing a pull request for Mauro so I'll put it in there
with your Ack.

Thank you

Sean

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-10 11:44 [PATCH 1/3] media: imon_raw: simplify and explain bit operations Sean Young
2019-08-10 11:44 ` [PATCH 2/3] media: imon_raw: prevent "nonsensical timing event of duration 0" Sean Young
2019-08-10 11:44 ` [PATCH 3/3] selftests: ir: fix ir_loopback test failure Sean Young
2019-08-12 14:25   ` shuah
2019-08-13  7:37     ` Sean Young

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox