[PATCH 1/3] media: imon_raw: simplify and explain bit operations

* [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 related	[flat|nested] 5+ messages in thread