All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/9] Fix coding style in en50221 CAM functions
@ 2017-07-12 23:00 Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 1/9] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread Jasmin J.
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

These patch series is the V2 version adapted to the already merged patches
from V1 and other merged patches. It does no longer rename constants, as
stated by Mauro as a no-go. It still fixed nearly all the style issues 
reported by checkpatch.pl in dvb-core/dvb_ca_en50221.c, but keeps more open
than the V1 version for good reasons. I also renamed the patch title,
because "Make checkpatch happy ?" is a bad title and checkpatch complained
about it. I also refactored the stat machine a bit more and added the new
function dvb_ca_en50221_poll_cam_gone in a new patch.
 
Two of them are "WARNING: memory barrier without comment". I have really
no clue why there is a call to "mb()" in that file, so I can't fill in a
good comment.
 
I kept some of the "WARNING: line over 80 characters" findings, which are 
strings for debugging, which shouldn't be split in several lines (will 
give other warning). And some lines, where a change would decrease the
readability. There it would be better to split the functions in new
subroutines, which I currently didn't.
 
And finally one "WARNING: Prefer [subsystem eg: netdev]_dbg", complaining
about the "dprintk" macro. In my opinion it is correctly used and it is
normally disabled anyway.
 
The main problem of the original code was the size of the lines and the
structural complexity of some functions. Beside refactoring of the thread
state machine, I used in nearly every function a helper pointer "sl" (for
"slot" structure) instead the whole structure path. This saved also a lot
of characters in long lines and made the code more readable in my opinion.
 
I split the patch set is small pieces for easier review, compiled each
step and tested the resulting driver on my hardware with the DD DuoFlex CI
(single) card. I took a lot of care in the first two patches to really
keep the code as is without loosing any line during refactoring.

Jasmin Jessich (9):
  [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread
  [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone
  [media] dvb-core/dvb_ca_en50221.c: use usleep_range
  [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs
  [media] dvb-core/dvb_ca_en50221.c: Used a helper variable
  [media] dvb-core/dvb_ca_en50221.c: Added line breaks
  [media] dvb-core/dvb_ca_en50221.c: Removed useless braces
  [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit

 drivers/media/dvb-core/dvb_ca_en50221.c | 774 ++++++++++++++++++--------------
 1 file changed, 441 insertions(+), 333 deletions(-)

-- 
2.7.4

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

* [PATCH V2 1/9] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 2/9] [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone Jasmin J.
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Refactored "dvb_ca_en50221_thread" by moving the state machine into the
new function "dvb_ca_en50221_thread_state_machine". This reduces the
thread function size and reduces the structural complexity and of course
gives us more space to meet the line length goal in the new function.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 357 +++++++++++++++++---------------
 1 file changed, 192 insertions(+), 165 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 17970cd..19d0e9a 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1063,207 +1063,234 @@ static void dvb_ca_en50221_thread_update_delay(struct dvb_ca_private *ca)
 	ca->delay = curdelay;
 }
 
-
-
 /**
- * Kernel thread which monitors CA slots for CAM changes, and performs data transfers.
+ * Thread state machine for one CA slot to perform the data transfer.
+ *
+ * @ca: CA instance.
+ * @slot: Slot to process.
  */
-static int dvb_ca_en50221_thread(void *data)
+static void dvb_ca_en50221_thread_state_machine(struct dvb_ca_private *ca,
+						int slot)
 {
-	struct dvb_ca_private *ca = data;
-	int slot;
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int flags;
 	int status;
 	int pktcount;
 	void *rxbuf;
 
-	dprintk("%s\n", __func__);
+	mutex_lock(&sl->slot_lock);
 
-	/* choose the correct initial delay */
-	dvb_ca_en50221_thread_update_delay(ca);
+	/* check the cam status + deal with CAMCHANGEs */
+	while (dvb_ca_en50221_check_camstatus(ca, slot)) {
+		/* clear down an old CI slot if necessary */
+		if (sl->slot_state != DVB_CA_SLOTSTATE_NONE)
+			dvb_ca_en50221_slot_shutdown(ca, slot);
 
-	/* main loop */
-	while (!kthread_should_stop()) {
-		/* sleep for a bit */
-		if (!ca->wakeup) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(ca->delay);
-			if (kthread_should_stop())
-				return 0;
-		}
-		ca->wakeup = 0;
+		/* if a CAM is NOW present, initialise it */
+		if (sl->camchange_type == DVB_CA_EN50221_CAMCHANGE_INSERTED)
+			sl->slot_state = DVB_CA_SLOTSTATE_UNINITIALISED;
 
-		/* go through all the slots processing them */
-		for (slot = 0; slot < ca->slot_count; slot++) {
-
-			mutex_lock(&ca->slot_info[slot].slot_lock);
-
-			// check the cam status + deal with CAMCHANGEs
-			while (dvb_ca_en50221_check_camstatus(ca, slot)) {
-				/* clear down an old CI slot if necessary */
-				if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_NONE)
-					dvb_ca_en50221_slot_shutdown(ca, slot);
-
-				/* if a CAM is NOW present, initialise it */
-				if (ca->slot_info[slot].camchange_type == DVB_CA_EN50221_CAMCHANGE_INSERTED) {
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_UNINITIALISED;
-				}
+		/* we've handled one CAMCHANGE */
+		dvb_ca_en50221_thread_update_delay(ca);
+		atomic_dec(&sl->camchange_count);
+	}
 
-				/* we've handled one CAMCHANGE */
-				dvb_ca_en50221_thread_update_delay(ca);
-				atomic_dec(&ca->slot_info[slot].camchange_count);
-			}
+	/* CAM state machine */
+	switch (sl->slot_state) {
+	case DVB_CA_SLOTSTATE_NONE:
+	case DVB_CA_SLOTSTATE_INVALID:
+		/* no action needed */
+		break;
 
-			// CAM state machine
-			switch (ca->slot_info[slot].slot_state) {
-			case DVB_CA_SLOTSTATE_NONE:
-			case DVB_CA_SLOTSTATE_INVALID:
-				// no action needed
-				break;
+	case DVB_CA_SLOTSTATE_UNINITIALISED:
+		sl->slot_state = DVB_CA_SLOTSTATE_WAITREADY;
+		ca->pub->slot_reset(ca->pub, slot);
+		sl->timeout = jiffies + (INIT_TIMEOUT_SECS * HZ);
+		break;
 
-			case DVB_CA_SLOTSTATE_UNINITIALISED:
-				ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_WAITREADY;
-				ca->pub->slot_reset(ca->pub, slot);
-				ca->slot_info[slot].timeout = jiffies + (INIT_TIMEOUT_SECS * HZ);
-				break;
+	case DVB_CA_SLOTSTATE_WAITREADY:
+		if (time_after(jiffies, sl->timeout)) {
+			pr_err("dvb_ca adaptor %d: PC card did not respond :(\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
+		/* no other action needed; will automatically change state when
+		 * ready
+		 */
+		break;
 
-			case DVB_CA_SLOTSTATE_WAITREADY:
-				if (time_after(jiffies, ca->slot_info[slot].timeout)) {
-					pr_err("dvb_ca adaptor %d: PC card did not respond :(\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
+	case DVB_CA_SLOTSTATE_VALIDATE:
+		if (dvb_ca_en50221_parse_attributes(ca, slot) != 0) {
+			/* we need this extra check for annoying interfaces like
+			 * the budget-av
+			 */
+			if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
+			    && (ca->pub->poll_slot_status)) {
+				status = ca->pub->poll_slot_status(ca->pub,
+								   slot, 0);
+				if (!(status &
+				      DVB_CA_EN50221_POLL_CAM_PRESENT)) {
+					sl->slot_state = DVB_CA_SLOTSTATE_NONE;
 					dvb_ca_en50221_thread_update_delay(ca);
 					break;
 				}
-				// no other action needed; will automatically change state when ready
-				break;
+			}
 
-			case DVB_CA_SLOTSTATE_VALIDATE:
-				if (dvb_ca_en50221_parse_attributes(ca, slot) != 0) {
-					/* we need this extra check for annoying interfaces like the budget-av */
-					if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)) &&
-					    (ca->pub->poll_slot_status)) {
-						status = ca->pub->poll_slot_status(ca->pub, slot, 0);
-						if (!(status & DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-							ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
-							dvb_ca_en50221_thread_update_delay(ca);
-							break;
-						}
-					}
-
-					pr_err("dvb_ca adapter %d: Invalid PC card inserted :(\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
-				if (dvb_ca_en50221_set_configoption(ca, slot) != 0) {
-					pr_err("dvb_ca adapter %d: Unable to initialise CAM :(\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
-				if (ca->pub->write_cam_control(ca->pub, slot,
-							       CTRLIF_COMMAND, CMDREG_RS) != 0) {
-					pr_err("dvb_ca adapter %d: Unable to reset CAM IF\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
-				dprintk("DVB CAM validated successfully\n");
+			pr_err("dvb_ca adapter %d: Invalid PC card inserted :(\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
+		if (dvb_ca_en50221_set_configoption(ca, slot) != 0) {
+			pr_err("dvb_ca adapter %d: Unable to initialise CAM :(\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
+		if (ca->pub->write_cam_control(ca->pub, slot,
+					       CTRLIF_COMMAND,
+					       CMDREG_RS) != 0) {
+			pr_err("dvb_ca adapter %d: Unable to reset CAM IF\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
+		dprintk("DVB CAM validated successfully\n");
 
-				ca->slot_info[slot].timeout = jiffies + (INIT_TIMEOUT_SECS * HZ);
-				ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_WAITFR;
-				ca->wakeup = 1;
-				break;
+		sl->timeout = jiffies + (INIT_TIMEOUT_SECS * HZ);
+		sl->slot_state = DVB_CA_SLOTSTATE_WAITFR;
+		ca->wakeup = 1;
+		break;
 
-			case DVB_CA_SLOTSTATE_WAITFR:
-				if (time_after(jiffies, ca->slot_info[slot].timeout)) {
-					pr_err("dvb_ca adapter %d: DVB CAM did not respond :(\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
+	case DVB_CA_SLOTSTATE_WAITFR:
+		if (time_after(jiffies, sl->timeout)) {
+			pr_err("dvb_ca adapter %d: DVB CAM did not respond :(\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
 
-				flags = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
-				if (flags & STATUSREG_FR) {
-					ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_LINKINIT;
-					ca->wakeup = 1;
-				}
-				break;
+		flags = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
+		if (flags & STATUSREG_FR) {
+			sl->slot_state = DVB_CA_SLOTSTATE_LINKINIT;
+			ca->wakeup = 1;
+		}
+		break;
 
-			case DVB_CA_SLOTSTATE_LINKINIT:
-				if (dvb_ca_en50221_link_init(ca, slot) != 0) {
-					/* we need this extra check for annoying interfaces like the budget-av */
-					if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)) &&
-					    (ca->pub->poll_slot_status)) {
-						status = ca->pub->poll_slot_status(ca->pub, slot, 0);
-						if (!(status & DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-							ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
-							dvb_ca_en50221_thread_update_delay(ca);
-							break;
-						}
-					}
-
-					pr_err("dvb_ca adapter %d: DVB CAM link initialisation failed :(\n",
-					       ca->dvbdev->adapter->num);
-					ca->slot_info[slot].slot_state =
-						DVB_CA_SLOTSTATE_UNINITIALISED;
+	case DVB_CA_SLOTSTATE_LINKINIT:
+		if (dvb_ca_en50221_link_init(ca, slot) != 0) {
+			/* we need this extra check for annoying interfaces like
+			 * the budget-av
+			 */
+			if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
+			    && (ca->pub->poll_slot_status)) {
+				status = ca->pub->poll_slot_status(ca->pub,
+								   slot, 0);
+				if (!(status &
+					DVB_CA_EN50221_POLL_CAM_PRESENT)) {
+					sl->slot_state = DVB_CA_SLOTSTATE_NONE;
 					dvb_ca_en50221_thread_update_delay(ca);
 					break;
 				}
+			}
 
-				if (ca->slot_info[slot].rx_buffer.data == NULL) {
-					rxbuf = vmalloc(RX_BUFFER_SIZE);
-					if (rxbuf == NULL) {
-						pr_err("dvb_ca adapter %d: Unable to allocate CAM rx buffer :(\n",
-						       ca->dvbdev->adapter->num);
-						ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_INVALID;
-						dvb_ca_en50221_thread_update_delay(ca);
-						break;
-					}
-					dvb_ringbuffer_init(&ca->slot_info[slot].rx_buffer, rxbuf, RX_BUFFER_SIZE);
-				}
+			pr_err("dvb_ca adapter %d: DVB CAM link initialisation failed :(\n",
+			       ca->dvbdev->adapter->num);
+			sl->slot_state = DVB_CA_SLOTSTATE_UNINITIALISED;
+			dvb_ca_en50221_thread_update_delay(ca);
+			break;
+		}
 
-				ca->pub->slot_ts_enable(ca->pub, slot);
-				ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_RUNNING;
-				dvb_ca_en50221_thread_update_delay(ca);
-				pr_err("dvb_ca adapter %d: DVB CAM detected and initialised successfully\n",
+		if (sl->rx_buffer.data == NULL) {
+			rxbuf = vmalloc(RX_BUFFER_SIZE);
+			if (rxbuf == NULL) {
+				pr_err("dvb_ca adapter %d: Unable to allocate CAM rx buffer :(\n",
 				       ca->dvbdev->adapter->num);
+				sl->slot_state = DVB_CA_SLOTSTATE_INVALID;
+				dvb_ca_en50221_thread_update_delay(ca);
 				break;
+			}
+			dvb_ringbuffer_init(&sl->rx_buffer, rxbuf,
+					    RX_BUFFER_SIZE);
+		}
 
-			case DVB_CA_SLOTSTATE_RUNNING:
-				if (!ca->open)
-					break;
+		ca->pub->slot_ts_enable(ca->pub, slot);
+		sl->slot_state = DVB_CA_SLOTSTATE_RUNNING;
+		dvb_ca_en50221_thread_update_delay(ca);
+		pr_err("dvb_ca adapter %d: DVB CAM detected and initialised successfully\n",
+		       ca->dvbdev->adapter->num);
+		break;
 
-				// poll slots for data
-				pktcount = 0;
-				while ((status = dvb_ca_en50221_read_data(ca, slot, NULL, 0)) > 0) {
-					if (!ca->open)
-						break;
-
-					/* if a CAMCHANGE occurred at some point, do not do any more processing of this slot */
-					if (dvb_ca_en50221_check_camstatus(ca, slot)) {
-						// we dont want to sleep on the next iteration so we can handle the cam change
-						ca->wakeup = 1;
-						break;
-					}
-
-					/* check if we've hit our limit this time */
-					if (++pktcount >= MAX_RX_PACKETS_PER_ITERATION) {
-						// dont sleep; there is likely to be more data to read
-						ca->wakeup = 1;
-						break;
-					}
-				}
+	case DVB_CA_SLOTSTATE_RUNNING:
+		if (!ca->open)
+			break;
+
+		/* poll slots for data */
+		pktcount = 0;
+		while (dvb_ca_en50221_read_data(ca, slot, NULL, 0) > 0) {
+			if (!ca->open)
+				break;
+
+			/* if a CAMCHANGE occurred at some point, do not do any
+			 * more processing of this slot
+			 */
+			if (dvb_ca_en50221_check_camstatus(ca, slot)) {
+				/* we dont want to sleep on the next iteration
+				 * so we can handle the cam change
+				 */
+				ca->wakeup = 1;
 				break;
 			}
 
-			mutex_unlock(&ca->slot_info[slot].slot_lock);
+			/* check if we've hit our limit this time */
+			if (++pktcount >= MAX_RX_PACKETS_PER_ITERATION) {
+				/* dont sleep; there is likely to be more data
+				 * to read
+				 */
+				ca->wakeup = 1;
+				break;
+			}
+		}
+		break;
+	}
+
+	mutex_unlock(&sl->slot_lock);
+}
+
+/**
+ * Kernel thread which monitors CA slots for CAM changes, and performs data
+ * transfers.
+ */
+static int dvb_ca_en50221_thread(void *data)
+{
+	struct dvb_ca_private *ca = data;
+	int slot;
+
+	dprintk("%s\n", __func__);
+
+	/* choose the correct initial delay */
+	dvb_ca_en50221_thread_update_delay(ca);
+
+	/* main loop */
+	while (!kthread_should_stop()) {
+		/* sleep for a bit */
+		if (!ca->wakeup) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule_timeout(ca->delay);
+			if (kthread_should_stop())
+				return 0;
 		}
+		ca->wakeup = 0;
+
+		/* go through all the slots processing them */
+		for (slot = 0; slot < ca->slot_count; slot++)
+			dvb_ca_en50221_thread_state_machine(ca, slot);
 	}
 
 	return 0;
-- 
2.7.4

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

* [PATCH V2 2/9] [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 1/9] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 3/9] [media] dvb-core/dvb_ca_en50221.c: use usleep_range Jasmin J.
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

The CAM poll code for the budget-av is exactly the same on several
places. Extracting the code to a new function improves maintainability.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 63 ++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 19d0e9a..66a58ed 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1064,6 +1064,36 @@ static void dvb_ca_en50221_thread_update_delay(struct dvb_ca_private *ca)
 }
 
 /**
+ * Poll if the CAM is gone.
+ *
+ * @ca: CA instance.
+ * @slot: Slot to process.
+ * @return: 0 .. no change
+ *          1 .. CAM state changed
+ */
+
+static int dvb_ca_en50221_poll_cam_gone(struct dvb_ca_private *ca, int slot)
+{
+	int changed = 0;
+	int status;
+
+	/* we need this extra check for annoying interfaces like the
+	 * budget-av
+	 */
+	if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
+	    && (ca->pub->poll_slot_status)) {
+		status = ca->pub->poll_slot_status(ca->pub, slot, 0);
+		if (!(status &
+			DVB_CA_EN50221_POLL_CAM_PRESENT)) {
+			ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
+			dvb_ca_en50221_thread_update_delay(ca);
+			changed = 1;
+		}
+	}
+	return changed;
+}
+
+/**
  * Thread state machine for one CA slot to perform the data transfer.
  *
  * @ca: CA instance.
@@ -1074,7 +1104,6 @@ static void dvb_ca_en50221_thread_state_machine(struct dvb_ca_private *ca,
 {
 	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int flags;
-	int status;
 	int pktcount;
 	void *rxbuf;
 
@@ -1123,20 +1152,8 @@ static void dvb_ca_en50221_thread_state_machine(struct dvb_ca_private *ca,
 
 	case DVB_CA_SLOTSTATE_VALIDATE:
 		if (dvb_ca_en50221_parse_attributes(ca, slot) != 0) {
-			/* we need this extra check for annoying interfaces like
-			 * the budget-av
-			 */
-			if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
-			    && (ca->pub->poll_slot_status)) {
-				status = ca->pub->poll_slot_status(ca->pub,
-								   slot, 0);
-				if (!(status &
-				      DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-					sl->slot_state = DVB_CA_SLOTSTATE_NONE;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
-			}
+			if (dvb_ca_en50221_poll_cam_gone(ca, slot))
+				break;
 
 			pr_err("dvb_ca adapter %d: Invalid PC card inserted :(\n",
 			       ca->dvbdev->adapter->num);
@@ -1185,20 +1202,8 @@ static void dvb_ca_en50221_thread_state_machine(struct dvb_ca_private *ca,
 
 	case DVB_CA_SLOTSTATE_LINKINIT:
 		if (dvb_ca_en50221_link_init(ca, slot) != 0) {
-			/* we need this extra check for annoying interfaces like
-			 * the budget-av
-			 */
-			if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
-			    && (ca->pub->poll_slot_status)) {
-				status = ca->pub->poll_slot_status(ca->pub,
-								   slot, 0);
-				if (!(status &
-					DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-					sl->slot_state = DVB_CA_SLOTSTATE_NONE;
-					dvb_ca_en50221_thread_update_delay(ca);
-					break;
-				}
-			}
+			if (dvb_ca_en50221_poll_cam_gone(ca, slot))
+				break;
 
 			pr_err("dvb_ca adapter %d: DVB CAM link initialisation failed :(\n",
 			       ca->dvbdev->adapter->num);
-- 
2.7.4

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

* [PATCH V2 3/9] [media] dvb-core/dvb_ca_en50221.c: use usleep_range
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 1/9] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 2/9] [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments Jasmin J.
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed all:
  WARNING: msleep < 20ms can sleep for up to 20ms
by using usleep_range.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 66a58ed..c0fd63a 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -313,7 +313,7 @@ static int dvb_ca_en50221_wait_if_status(struct dvb_ca_private *ca, int slot,
 		}
 
 		/* wait for a bit */
-		msleep(1);
+		usleep_range(1000, 1100);
 	}
 
 	dprintk("%s failed timeout:%lu\n", __func__, jiffies - start);
@@ -1484,7 +1484,7 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 			if (status != -EAGAIN)
 				goto exit;
 
-			msleep(1);
+			usleep_range(1000, 1100);
 		}
 		if (!written) {
 			status = -EIO;
-- 
2.7.4

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

* [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (2 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 3/9] [media] dvb-core/dvb_ca_en50221.c: use usleep_range Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:17   ` Antti Palosaari
  2017-07-12 23:00 ` [PATCH V2 5/9] [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs Jasmin J.
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed all:
  WARNING: Block comments use * on subsequent lines

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index c0fd63a..317968b 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -343,7 +343,8 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	ca->slot_info[slot].da_irq_supported = 0;
 
 	/* set the host link buffer size temporarily. it will be overwritten with the
-	 * real negotiated size later. */
+	 * real negotiated size later.
+	 */
 	ca->slot_info[slot].link_buf_size = 2;
 
 	/* read the buffer size from the CAM */
@@ -797,9 +798,10 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 		return ca->pub->write_data(ca->pub, slot, buf, bytes_write);
 
 	/* it is possible we are dealing with a single buffer implementation,
-	   thus if there is data available for read or if there is even a read
-	   already in progress, we do nothing but awake the kernel thread to
-	   process the data if necessary. */
+	 * thus if there is data available for read or if there is even a read
+	 * already in progress, we do nothing but awake the kernel thread to
+	 * process the data if necessary.
+	 */
 	if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0)
 		goto exitnowrite;
 	if (status & (STATUSREG_DA | STATUSREG_RE)) {
@@ -899,8 +901,9 @@ static int dvb_ca_en50221_slot_shutdown(struct dvb_ca_private *ca, int slot)
 	ca->pub->slot_shutdown(ca->pub, slot);
 	ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
 
-	/* need to wake up all processes to check if they're now
-	   trying to write to a defunct CAM */
+	/* need to wake up all processes to check if they're now trying to
+	 * write to a defunct CAM
+	 */
 	wake_up_interruptible(&ca->wait_queue);
 
 	dprintk("Slot %i shutdown\n", slot);
@@ -1681,8 +1684,10 @@ static int dvb_ca_en50221_io_open(struct inode *inode, struct file *file)
 
 		if (ca->slot_info[i].slot_state == DVB_CA_SLOTSTATE_RUNNING) {
 			if (ca->slot_info[i].rx_buffer.data != NULL) {
-				/* it is safe to call this here without locks because
-				 * ca->open == 0. Data is not read in this case */
+				/* it is safe to call this here without locks
+				 * because ca->open == 0. Data is not read in
+				 * this case
+				 */
 				dvb_ringbuffer_flush(&ca->slot_info[i].rx_buffer);
 			}
 		}
-- 
2.7.4

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

* [PATCH V2 5/9] [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (3 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 6/9] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable Jasmin J.
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed all:
  ERROR: do not use assignment in if condition

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 97 ++++++++++++++++++++++-----------
 1 file changed, 64 insertions(+), 33 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 317968b..02b8785 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -348,14 +348,18 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	ca->slot_info[slot].link_buf_size = 2;
 
 	/* read the buffer size from the CAM */
-	if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN | CMDREG_SR)) != 0)
+	ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
+					 IRQEN | CMDREG_SR);
+	if (ret != 0)
 		return ret;
 	ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_DA, HZ);
 	if (ret != 0)
 		return ret;
-	if ((ret = dvb_ca_en50221_read_data(ca, slot, buf, 2)) != 2)
+	ret = dvb_ca_en50221_read_data(ca, slot, buf, 2);
+	if (ret != 2)
 		return -EIO;
-	if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN)) != 0)
+	ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN);
+	if (ret != 0)
 		return ret;
 
 	/* store it, and choose the minimum of our buffer and the CAM's buffer size */
@@ -368,13 +372,18 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	dprintk("Chosen link buffer size of %i\n", buf_size);
 
 	/* write the buffer size to the CAM */
-	if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN | CMDREG_SW)) != 0)
+	ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
+					 IRQEN | CMDREG_SW);
+	if (ret != 0)
 		return ret;
-	if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_FR, HZ / 10)) != 0)
+	ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_FR, HZ / 10);
+	if (ret != 0)
 		return ret;
-	if ((ret = dvb_ca_en50221_write_data(ca, slot, buf, 2)) != 2)
+	ret = dvb_ca_en50221_write_data(ca, slot, buf, 2);
+	if (ret != 2)
 		return -EIO;
-	if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN)) != 0)
+	ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN);
+	if (ret != 0)
 		return ret;
 
 	/* success */
@@ -403,7 +412,8 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private *ca, int slot,
 	int _address = *address;
 
 	/* grab the next tuple length and type */
-	if ((_tupleType = ca->pub->read_attribute_mem(ca->pub, slot, _address)) < 0)
+	_tupleType = ca->pub->read_attribute_mem(ca->pub, slot, _address);
+	if (_tupleType < 0)
 		return _tupleType;
 	if (_tupleType == 0xff) {
 		dprintk("END OF CHAIN TUPLE type:0x%x\n", _tupleType);
@@ -412,7 +422,8 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private *ca, int slot,
 		*tupleLength = 0;
 		return 0;
 	}
-	if ((_tupleLength = ca->pub->read_attribute_mem(ca->pub, slot, _address + 2)) < 0)
+	_tupleLength = ca->pub->read_attribute_mem(ca->pub, slot, _address + 2);
+	if (_tupleLength < 0)
 		return _tupleLength;
 	_address += 4;
 
@@ -461,8 +472,9 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 
 	// CISTPL_DEVICE_0A
-	if ((status =
-	     dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType, &tupleLength, tuple)) < 0)
+	status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
+					   &tupleLength, tuple);
+	if (status < 0)
 		return status;
 	if (tupleType != 0x1D)
 		return -EINVAL;
@@ -470,8 +482,9 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 
 	// CISTPL_DEVICE_0C
-	if ((status =
-	     dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType, &tupleLength, tuple)) < 0)
+	status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
+					   &tupleLength, tuple);
+	if (status < 0)
 		return status;
 	if (tupleType != 0x1C)
 		return -EINVAL;
@@ -479,8 +492,9 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 
 	// CISTPL_VERS_1
-	if ((status =
-	     dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType, &tupleLength, tuple)) < 0)
+	status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
+					   &tupleLength, tuple);
+	if (status < 0)
 		return status;
 	if (tupleType != 0x15)
 		return -EINVAL;
@@ -488,8 +502,9 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 
 	// CISTPL_MANFID
-	if ((status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
-						&tupleLength, tuple)) < 0)
+	status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
+							&tupleLength, tuple);
+	if (status < 0)
 		return status;
 	if (tupleType != 0x20)
 		return -EINVAL;
@@ -501,8 +516,9 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 
 	// CISTPL_CONFIG
-	if ((status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
-						&tupleLength, tuple)) < 0)
+	status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
+					   &tupleLength, tuple);
+	if (status < 0)
 		return status;
 	if (tupleType != 0x1A)
 		return -EINVAL;
@@ -535,8 +551,10 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 
 	/* process the CFTABLE_ENTRY tuples, and any after those */
 	while ((!end_chain) && (address < 0x1000)) {
-		if ((status = dvb_ca_en50221_read_tuple(ca, slot, &address, &tupleType,
-							&tupleLength, tuple)) < 0)
+		status = dvb_ca_en50221_read_tuple(ca, slot, &address,
+						   &tupleType, &tupleLength,
+						   tuple);
+		if (status < 0)
 			return status;
 		switch (tupleType) {
 		case 0x1B:	// CISTPL_CFTABLE_ENTRY
@@ -802,7 +820,8 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 	 * already in progress, we do nothing but awake the kernel thread to
 	 * process the data if necessary.
 	 */
-	if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0)
+	status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
+	if (status < 0)
 		goto exitnowrite;
 	if (status & (STATUSREG_DA | STATUSREG_RE)) {
 		if (status & STATUSREG_DA)
@@ -813,12 +832,14 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 	}
 
 	/* OK, set HC bit */
-	if ((status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
-						 IRQEN | CMDREG_HC)) != 0)
+	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
+					    IRQEN | CMDREG_HC);
+	if (status != 0)
 		goto exit;
 
 	/* check if interface is still free */
-	if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0)
+	status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
+	if (status < 0)
 		goto exit;
 	if (!(status & STATUSREG_FR)) {
 		/* it wasn't free => try again later */
@@ -850,20 +871,26 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 	}
 
 	/* send the amount of data */
-	if ((status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_HIGH, bytes_write >> 8)) != 0)
+	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_HIGH,
+					    bytes_write >> 8);
+	if (status != 0)
 		goto exit;
-	if ((status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_LOW,
-						 bytes_write & 0xff)) != 0)
+	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_LOW,
+					    bytes_write & 0xff);
+	if (status != 0)
 		goto exit;
 
 	/* send the buffer */
 	for (i = 0; i < bytes_write; i++) {
-		if ((status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_DATA, buf[i])) != 0)
+		status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_DATA,
+						    buf[i]);
+		if (status != 0)
 			goto exit;
 	}
 
 	/* check for write error (WE should now be 0) */
-	if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0)
+	status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
+	if (status < 0)
 		goto exit;
 	if (status & STATUSREG_WE) {
 		ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_LINKINIT;
@@ -1583,7 +1610,8 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 		return -EINVAL;
 
 	/* wait for some data */
-	if ((status = dvb_ca_en50221_io_read_condition(ca, &result, &slot)) == 0) {
+	status = dvb_ca_en50221_io_read_condition(ca, &result, &slot);
+	if (status == 0) {
 
 		/* if we're in nonblocking mode, exit immediately */
 		if (file->f_flags & O_NONBLOCK)
@@ -1820,7 +1848,8 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 		return -EINVAL;
 
 	/* initialise the system data */
-	if ((ca = kzalloc(sizeof(struct dvb_ca_private), GFP_KERNEL)) == NULL) {
+	ca = kzalloc(sizeof(struct dvb_ca_private), GFP_KERNEL);
+	if (ca == NULL) {
 		ret = -ENOMEM;
 		goto exit;
 	}
@@ -1828,7 +1857,9 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 	ca->pub = pubca;
 	ca->flags = flags;
 	ca->slot_count = slot_count;
-	if ((ca->slot_info = kcalloc(slot_count, sizeof(struct dvb_ca_slot), GFP_KERNEL)) == NULL) {
+	ca->slot_info = kcalloc(slot_count, sizeof(struct dvb_ca_slot),
+				GFP_KERNEL);
+	if (ca->slot_info == NULL) {
 		ret = -ENOMEM;
 		goto free_ca;
 	}
-- 
2.7.4

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

* [PATCH V2 6/9] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (4 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 5/9] [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 7/9] [media] dvb-core/dvb_ca_en50221.c: Added line breaks Jasmin J.
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Used a helper variable "struct dvb_ca_slot *sl" instead of
"ca->slot_info[slot]". This reduces the line length and simplifies
code reading.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 187 ++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 81 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 02b8785..eb7f692 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -234,13 +234,14 @@ static char *findstr(char *haystack, int hlen, char *needle, int nlen)
  */
 static int dvb_ca_en50221_check_camstatus(struct dvb_ca_private *ca, int slot)
 {
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int slot_status;
 	int cam_present_now;
 	int cam_changed;
 
 	/* IRQ mode */
 	if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE) {
-		return (atomic_read(&ca->slot_info[slot].camchange_count) != 0);
+		return (atomic_read(&sl->camchange_count) != 0);
 	}
 
 	/* poll mode */
@@ -249,22 +250,23 @@ static int dvb_ca_en50221_check_camstatus(struct dvb_ca_private *ca, int slot)
 	cam_present_now = (slot_status & DVB_CA_EN50221_POLL_CAM_PRESENT) ? 1 : 0;
 	cam_changed = (slot_status & DVB_CA_EN50221_POLL_CAM_CHANGED) ? 1 : 0;
 	if (!cam_changed) {
-		int cam_present_old = (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_NONE);
+		int cam_present_old = (sl->slot_state != DVB_CA_SLOTSTATE_NONE);
+
 		cam_changed = (cam_present_now != cam_present_old);
 	}
 
 	if (cam_changed) {
 		if (!cam_present_now) {
-			ca->slot_info[slot].camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
+			sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
 		} else {
-			ca->slot_info[slot].camchange_type = DVB_CA_EN50221_CAMCHANGE_INSERTED;
+			sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_INSERTED;
 		}
-		atomic_set(&ca->slot_info[slot].camchange_count, 1);
+		atomic_set(&sl->camchange_count, 1);
 	} else {
-		if ((ca->slot_info[slot].slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
+		if ((sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
 		    (slot_status & DVB_CA_EN50221_POLL_CAM_READY)) {
 			// move to validate state if reset is completed
-			ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_VALIDATE;
+			sl->slot_state = DVB_CA_SLOTSTATE_VALIDATE;
 		}
 	}
 
@@ -333,6 +335,7 @@ static int dvb_ca_en50221_wait_if_status(struct dvb_ca_private *ca, int slot,
  */
 static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 {
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int ret;
 	int buf_size;
 	u8 buf[2];
@@ -340,12 +343,12 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	dprintk("%s\n", __func__);
 
 	/* we'll be determining these during this function */
-	ca->slot_info[slot].da_irq_supported = 0;
+	sl->da_irq_supported = 0;
 
 	/* set the host link buffer size temporarily. it will be overwritten with the
 	 * real negotiated size later.
 	 */
-	ca->slot_info[slot].link_buf_size = 2;
+	sl->link_buf_size = 2;
 
 	/* read the buffer size from the CAM */
 	ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
@@ -366,7 +369,7 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	buf_size = (buf[0] << 8) | buf[1];
 	if (buf_size > HOST_LINK_BUF_SIZE)
 		buf_size = HOST_LINK_BUF_SIZE;
-	ca->slot_info[slot].link_buf_size = buf_size;
+	sl->link_buf_size = buf_size;
 	buf[0] = buf_size >> 8;
 	buf[1] = buf_size & 0xff;
 	dprintk("Chosen link buffer size of %i\n", buf_size);
@@ -457,6 +460,7 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private *ca, int slot,
  */
 static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 {
+	struct dvb_ca_slot *sl;
 	int address = 0;
 	int tupleLength;
 	int tupleType;
@@ -529,10 +533,10 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 	rasz = tuple[0] & 3;
 	if (tupleLength < (3 + rasz + 14))
 		return -EINVAL;
-	ca->slot_info[slot].config_base = 0;
-	for (i = 0; i < rasz + 1; i++) {
-		ca->slot_info[slot].config_base |= (tuple[2 + i] << (8 * i));
-	}
+	sl = &ca->slot_info[slot];
+	sl->config_base = 0;
+	for (i = 0; i < rasz + 1; i++)
+		sl->config_base |= (tuple[2 + i] << (8 * i));
 
 	/* check it contains the correct DVB string */
 	dvb_str = findstr((char *)tuple, tupleLength, "DVB_CI_V", 8);
@@ -566,7 +570,7 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 				break;
 
 			/* get the config option */
-			ca->slot_info[slot].config_option = tuple[0] & 0x3f;
+			sl->config_option = tuple[0] & 0x3f;
 
 			/* OK, check it contains the correct strings */
 			if ((findstr((char *)tuple, tupleLength, "DVB_HOST", 8) == NULL) ||
@@ -594,8 +598,7 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 		return -EINVAL;
 
 	dprintk("Valid DVB CAM detected MANID:%x DEVID:%x CONFIGBASE:0x%x CONFIGOPTION:0x%x\n",
-		manfid, devid, ca->slot_info[slot].config_base,
-		ca->slot_info[slot].config_option);
+		manfid, devid, sl->config_base, sl->config_option);
 
 	// success!
 	return 0;
@@ -610,19 +613,20 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
  */
 static int dvb_ca_en50221_set_configoption(struct dvb_ca_private *ca, int slot)
 {
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int configoption;
 
 	dprintk("%s\n", __func__);
 
 	/* set the config option */
-	ca->pub->write_attribute_mem(ca->pub, slot,
-				     ca->slot_info[slot].config_base,
-				     ca->slot_info[slot].config_option);
+	ca->pub->write_attribute_mem(ca->pub, slot, sl->config_base,
+				     sl->config_option);
 
 	/* check it */
-	configoption = ca->pub->read_attribute_mem(ca->pub, slot, ca->slot_info[slot].config_base);
+	configoption = ca->pub->read_attribute_mem(ca->pub, slot,
+						   sl->config_base);
 	dprintk("Set configoption 0x%x, read configoption 0x%x\n",
-		ca->slot_info[slot].config_option, configoption & 0x3f);
+		sl->config_option, configoption & 0x3f);
 
 	/* fine! */
 	return 0;
@@ -647,6 +651,7 @@ static int dvb_ca_en50221_set_configoption(struct dvb_ca_private *ca, int slot)
 static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 				    u8 *ebuf, int ecount)
 {
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int bytes_read;
 	int status;
 	u8 buf[HOST_LINK_BUF_SIZE];
@@ -658,13 +663,13 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 	if (ebuf == NULL) {
 		int buf_free;
 
-		if (ca->slot_info[slot].rx_buffer.data == NULL) {
+		if (sl->rx_buffer.data == NULL) {
 			status = -EIO;
 			goto exit;
 		}
-		buf_free = dvb_ringbuffer_free(&ca->slot_info[slot].rx_buffer);
+		buf_free = dvb_ringbuffer_free(&sl->rx_buffer);
 
-		if (buf_free < (ca->slot_info[slot].link_buf_size +
+		if (buf_free < (sl->link_buf_size +
 				DVB_RINGBUFFER_PKTHDRSIZE)) {
 			status = -EAGAIN;
 			goto exit;
@@ -672,7 +677,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 	}
 
 	if (ca->pub->read_data &&
-	    (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_LINKINIT)) {
+	    (sl->slot_state != DVB_CA_SLOTSTATE_LINKINIT)) {
 		if (ebuf == NULL)
 			status = ca->pub->read_data(ca->pub, slot, buf,
 						    sizeof(buf));
@@ -710,20 +715,18 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 
 		/* check it will fit */
 		if (ebuf == NULL) {
-			if (bytes_read > ca->slot_info[slot].link_buf_size) {
+			if (bytes_read > sl->link_buf_size) {
 				pr_err("dvb_ca adapter %d: CAM tried to send a buffer larger than the link buffer size (%i > %i)!\n",
 				       ca->dvbdev->adapter->num, bytes_read,
-				       ca->slot_info[slot].link_buf_size);
-				ca->slot_info[slot].slot_state =
-						     DVB_CA_SLOTSTATE_LINKINIT;
+				       sl->link_buf_size);
+				sl->slot_state = DVB_CA_SLOTSTATE_LINKINIT;
 				status = -EIO;
 				goto exit;
 			}
 			if (bytes_read < 2) {
 				pr_err("dvb_ca adapter %d: CAM sent a buffer that was less than 2 bytes!\n",
 				       ca->dvbdev->adapter->num);
-				ca->slot_info[slot].slot_state =
-						     DVB_CA_SLOTSTATE_LINKINIT;
+				sl->slot_state = DVB_CA_SLOTSTATE_LINKINIT;
 				status = -EIO;
 				goto exit;
 			}
@@ -754,8 +757,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 		if (status < 0)
 			goto exit;
 		if (status & STATUSREG_RE) {
-			ca->slot_info[slot].slot_state =
-						     DVB_CA_SLOTSTATE_LINKINIT;
+			sl->slot_state = DVB_CA_SLOTSTATE_LINKINIT;
 			status = -EIO;
 			goto exit;
 		}
@@ -763,11 +765,11 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 
 	/* OK, add it to the receive buffer, or copy into external buffer if supplied */
 	if (ebuf == NULL) {
-		if (ca->slot_info[slot].rx_buffer.data == NULL) {
+		if (sl->rx_buffer.data == NULL) {
 			status = -EIO;
 			goto exit;
 		}
-		dvb_ringbuffer_pkt_write(&ca->slot_info[slot].rx_buffer, buf, bytes_read);
+		dvb_ringbuffer_pkt_write(&sl->rx_buffer, buf, bytes_read);
 	} else {
 		memcpy(ebuf, buf, bytes_read);
 	}
@@ -801,6 +803,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 				     u8 *buf, int bytes_write)
 {
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int status;
 	int i;
 
@@ -808,11 +811,11 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 
 
 	/* sanity check */
-	if (bytes_write > ca->slot_info[slot].link_buf_size)
+	if (bytes_write > sl->link_buf_size)
 		return -EINVAL;
 
 	if (ca->pub->write_data &&
-	    (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_LINKINIT))
+	    (sl->slot_state != DVB_CA_SLOTSTATE_LINKINIT))
 		return ca->pub->write_data(ca->pub, slot, buf, bytes_write);
 
 	/* it is possible we are dealing with a single buffer implementation,
@@ -893,7 +896,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 	if (status < 0)
 		goto exit;
 	if (status & STATUSREG_WE) {
-		ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_LINKINIT;
+		sl->slot_state = DVB_CA_SLOTSTATE_LINKINIT;
 		status = -EIO;
 		goto exit;
 	}
@@ -950,6 +953,7 @@ static int dvb_ca_en50221_slot_shutdown(struct dvb_ca_private *ca, int slot)
 void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot, int change_type)
 {
 	struct dvb_ca_private *ca = pubca->private;
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 
 	dprintk("CAMCHANGE IRQ slot:%i change_type:%i\n", slot, change_type);
 
@@ -962,8 +966,8 @@ void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot, int ch
 		return;
 	}
 
-	ca->slot_info[slot].camchange_type = change_type;
-	atomic_inc(&ca->slot_info[slot].camchange_count);
+	sl->camchange_type = change_type;
+	atomic_inc(&sl->camchange_count);
 	dvb_ca_en50221_thread_wakeup(ca);
 }
 EXPORT_SYMBOL(dvb_ca_en50221_camchange_irq);
@@ -978,11 +982,12 @@ EXPORT_SYMBOL(dvb_ca_en50221_camchange_irq);
 void dvb_ca_en50221_camready_irq(struct dvb_ca_en50221 *pubca, int slot)
 {
 	struct dvb_ca_private *ca = pubca->private;
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 
 	dprintk("CAMREADY IRQ slot:%i\n", slot);
 
-	if (ca->slot_info[slot].slot_state == DVB_CA_SLOTSTATE_WAITREADY) {
-		ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_VALIDATE;
+	if (sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) {
+		sl->slot_state = DVB_CA_SLOTSTATE_VALIDATE;
 		dvb_ca_en50221_thread_wakeup(ca);
 	}
 }
@@ -998,16 +1003,17 @@ EXPORT_SYMBOL(dvb_ca_en50221_camready_irq);
 void dvb_ca_en50221_frda_irq(struct dvb_ca_en50221 *pubca, int slot)
 {
 	struct dvb_ca_private *ca = pubca->private;
+	struct dvb_ca_slot *sl = &ca->slot_info[slot];
 	int flags;
 
 	dprintk("FR/DA IRQ slot:%i\n", slot);
 
-	switch (ca->slot_info[slot].slot_state) {
+	switch (sl->slot_state) {
 	case DVB_CA_SLOTSTATE_LINKINIT:
 		flags = ca->pub->read_cam_control(pubca, slot, CTRLIF_STATUS);
 		if (flags & STATUSREG_DA) {
 			dprintk("CAM supports DA IRQ\n");
-			ca->slot_info[slot].da_irq_supported = 1;
+			sl->da_irq_supported = 1;
 		}
 		break;
 
@@ -1053,7 +1059,9 @@ static void dvb_ca_en50221_thread_update_delay(struct dvb_ca_private *ca)
 	 * call might take several hundred milliseconds until timeout!
 	 */
 	for (slot = 0; slot < ca->slot_count; slot++) {
-		switch (ca->slot_info[slot].slot_state) {
+		struct dvb_ca_slot *sl = &ca->slot_info[slot];
+
+		switch (sl->slot_state) {
 		default:
 		case DVB_CA_SLOTSTATE_NONE:
 			delay = HZ * 60;  /* 60s */
@@ -1079,7 +1087,7 @@ static void dvb_ca_en50221_thread_update_delay(struct dvb_ca_private *ca)
 			if (!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
 				delay = HZ / 10;  /* 100ms */
 			if (ca->open) {
-				if ((!ca->slot_info[slot].da_irq_supported) ||
+				if ((!sl->da_irq_supported) ||
 				    (!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_DA)))
 					delay = HZ / 10;  /* 100ms */
 			}
@@ -1363,15 +1371,17 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
 	switch (cmd) {
 	case CA_RESET:
 		for (slot = 0; slot < ca->slot_count; slot++) {
-			mutex_lock(&ca->slot_info[slot].slot_lock);
-			if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_NONE) {
+			struct dvb_ca_slot *sl = &ca->slot_info[slot];
+
+			mutex_lock(&sl->slot_lock);
+			if (sl->slot_state != DVB_CA_SLOTSTATE_NONE) {
 				dvb_ca_en50221_slot_shutdown(ca, slot);
 				if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)
 					dvb_ca_en50221_camchange_irq(ca->pub,
 								     slot,
 								     DVB_CA_EN50221_CAMCHANGE_INSERTED);
 			}
-			mutex_unlock(&ca->slot_info[slot].slot_lock);
+			mutex_unlock(&sl->slot_lock);
 		}
 		ca->next_read_slot = 0;
 		dvb_ca_en50221_thread_wakeup(ca);
@@ -1389,21 +1399,23 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
 
 	case CA_GET_SLOT_INFO: {
 		struct ca_slot_info *info = parg;
+		struct dvb_ca_slot *sl;
 
-		if ((info->num > ca->slot_count) || (info->num < 0)) {
+		slot = info->num;
+		if ((slot > ca->slot_count) || (slot < 0)) {
 			err = -EINVAL;
 			goto out_unlock;
 		}
 
 		info->type = CA_CI_LINK;
 		info->flags = 0;
-		if ((ca->slot_info[info->num].slot_state != DVB_CA_SLOTSTATE_NONE)
-			&& (ca->slot_info[info->num].slot_state != DVB_CA_SLOTSTATE_INVALID)) {
+		sl = &ca->slot_info[slot];
+		if ((sl->slot_state != DVB_CA_SLOTSTATE_NONE)
+			&& (sl->slot_state != DVB_CA_SLOTSTATE_INVALID)) {
 			info->flags = CA_CI_MODULE_PRESENT;
 		}
-		if (ca->slot_info[info->num].slot_state == DVB_CA_SLOTSTATE_RUNNING) {
+		if (sl->slot_state == DVB_CA_SLOTSTATE_RUNNING)
 			info->flags |= CA_CI_MODULE_READY;
-		}
 		break;
 	}
 
@@ -1451,6 +1463,7 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_ca_private *ca = dvbdev->priv;
+	struct dvb_ca_slot *sl;
 	u8 slot, connection_id;
 	int status;
 	u8 fragbuf[HOST_LINK_BUF_SIZE];
@@ -1472,14 +1485,15 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 		return -EFAULT;
 	buf += 2;
 	count -= 2;
+	sl = &ca->slot_info[slot];
 
 	/* check if the slot is actually running */
-	if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING)
+	if (sl->slot_state != DVB_CA_SLOTSTATE_RUNNING)
 		return -EINVAL;
 
 	/* fragment the packets & store in the buffer */
 	while (fragpos < count) {
-		fraglen = ca->slot_info[slot].link_buf_size - 2;
+		fraglen = sl->link_buf_size - 2;
 		if (fraglen < 0)
 			break;
 		if (fraglen > HOST_LINK_BUF_SIZE - 2)
@@ -1499,14 +1513,14 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 		written = 0;
 		while (!time_after(jiffies, timeout)) {
 			/* check the CAM hasn't been removed/reset in the meantime */
-			if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING) {
+			if (sl->slot_state != DVB_CA_SLOTSTATE_RUNNING) {
 				status = -EIO;
 				goto exit;
 			}
 
-			mutex_lock(&ca->slot_info[slot].slot_lock);
+			mutex_lock(&sl->slot_lock);
 			status = dvb_ca_en50221_write_data(ca, slot, fragbuf, fraglen + 2);
-			mutex_unlock(&ca->slot_info[slot].slot_lock);
+			mutex_unlock(&sl->slot_lock);
 			if (status == (fraglen + 2)) {
 				written = 1;
 				break;
@@ -1546,16 +1560,17 @@ static int dvb_ca_en50221_io_read_condition(struct dvb_ca_private *ca,
 
 	slot = ca->next_read_slot;
 	while ((slot_count < ca->slot_count) && (!found)) {
-		if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING)
+		struct dvb_ca_slot *sl = &ca->slot_info[slot];
+
+		if (sl->slot_state != DVB_CA_SLOTSTATE_RUNNING)
 			goto nextslot;
 
-		if (ca->slot_info[slot].rx_buffer.data == NULL) {
+		if (sl->rx_buffer.data == NULL)
 			return 0;
-		}
 
-		idx = dvb_ringbuffer_pkt_next(&ca->slot_info[slot].rx_buffer, -1, &fraglen);
+		idx = dvb_ringbuffer_pkt_next(&sl->rx_buffer, -1, &fraglen);
 		while (idx != -1) {
-			dvb_ringbuffer_pkt_read(&ca->slot_info[slot].rx_buffer, idx, 0, hdr, 2);
+			dvb_ringbuffer_pkt_read(&sl->rx_buffer, idx, 0, hdr, 2);
 			if (connection_id == -1)
 				connection_id = hdr[0];
 			if ((hdr[0] == connection_id) && ((hdr[1] & 0x80) == 0)) {
@@ -1564,7 +1579,8 @@ static int dvb_ca_en50221_io_read_condition(struct dvb_ca_private *ca,
 				break;
 			}
 
-			idx = dvb_ringbuffer_pkt_next(&ca->slot_info[slot].rx_buffer, idx, &fraglen);
+			idx = dvb_ringbuffer_pkt_next(&sl->rx_buffer, idx,
+						      &fraglen);
 		}
 
 nextslot:
@@ -1592,6 +1608,7 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_ca_private *ca = dvbdev->priv;
+	struct dvb_ca_slot *sl;
 	int status;
 	int result = 0;
 	u8 hdr[2];
@@ -1628,7 +1645,8 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 		return status;
 	}
 
-	idx = dvb_ringbuffer_pkt_next(&ca->slot_info[slot].rx_buffer, -1, &fraglen);
+	sl = &ca->slot_info[slot];
+	idx = dvb_ringbuffer_pkt_next(&sl->rx_buffer, -1, &fraglen);
 	pktlen = 2;
 	do {
 		if (idx == -1) {
@@ -1638,7 +1656,7 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 			goto exit;
 		}
 
-		dvb_ringbuffer_pkt_read(&ca->slot_info[slot].rx_buffer, idx, 0, hdr, 2);
+		dvb_ringbuffer_pkt_read(&sl->rx_buffer, idx, 0, hdr, 2);
 		if (connection_id == -1)
 			connection_id = hdr[0];
 		if (hdr[0] == connection_id) {
@@ -1649,10 +1667,14 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 					fraglen -= 2;
 				}
 
-				if ((status = dvb_ringbuffer_pkt_read_user(&ca->slot_info[slot].rx_buffer, idx, 2,
-								      buf + pktlen, fraglen)) < 0) {
+				status =
+				   dvb_ringbuffer_pkt_read_user(&sl->rx_buffer,
+								idx, 2,
+								buf + pktlen,
+								fraglen);
+				if (status < 0)
 					goto exit;
-				}
+
 				pktlen += fraglen;
 			}
 
@@ -1661,9 +1683,9 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 			dispose = 1;
 		}
 
-		idx2 = dvb_ringbuffer_pkt_next(&ca->slot_info[slot].rx_buffer, idx, &fraglen);
+		idx2 = dvb_ringbuffer_pkt_next(&sl->rx_buffer, idx, &fraglen);
 		if (dispose)
-			dvb_ringbuffer_pkt_dispose(&ca->slot_info[slot].rx_buffer, idx);
+			dvb_ringbuffer_pkt_dispose(&sl->rx_buffer, idx);
 		idx = idx2;
 		dispose = 0;
 	} while (!last_fragment);
@@ -1709,14 +1731,15 @@ static int dvb_ca_en50221_io_open(struct inode *inode, struct file *file)
 	}
 
 	for (i = 0; i < ca->slot_count; i++) {
+		struct dvb_ca_slot *sl = &ca->slot_info[i];
 
-		if (ca->slot_info[i].slot_state == DVB_CA_SLOTSTATE_RUNNING) {
-			if (ca->slot_info[i].rx_buffer.data != NULL) {
+		if (sl->slot_state == DVB_CA_SLOTSTATE_RUNNING) {
+			if (sl->rx_buffer.data != NULL) {
 				/* it is safe to call this here without locks
 				 * because ca->open == 0. Data is not read in
 				 * this case
 				 */
-				dvb_ringbuffer_flush(&ca->slot_info[i].rx_buffer);
+				dvb_ringbuffer_flush(&sl->rx_buffer);
 			}
 		}
 	}
@@ -1876,11 +1899,13 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 
 	/* now initialise each slot */
 	for (i = 0; i < slot_count; i++) {
-		memset(&ca->slot_info[i], 0, sizeof(struct dvb_ca_slot));
-		ca->slot_info[i].slot_state = DVB_CA_SLOTSTATE_NONE;
-		atomic_set(&ca->slot_info[i].camchange_count, 0);
-		ca->slot_info[i].camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
-		mutex_init(&ca->slot_info[i].slot_lock);
+		struct dvb_ca_slot *sl = &ca->slot_info[i];
+
+		memset(sl, 0, sizeof(struct dvb_ca_slot));
+		sl->slot_state = DVB_CA_SLOTSTATE_NONE;
+		atomic_set(&sl->camchange_count, 0);
+		sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
+		mutex_init(&sl->slot_lock);
 	}
 
 	mutex_init(&ca->ioctl_mutex);
-- 
2.7.4

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

* [PATCH V2 7/9] [media] dvb-core/dvb_ca_en50221.c: Added line breaks
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (5 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 6/9] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 8/9] [media] dvb-core/dvb_ca_en50221.c: Removed useless braces Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 9/9] [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit Jasmin J.
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed all:
  WARNING: Missing a blank line after declarations

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index eb7f692..5a0b35d 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -178,7 +178,9 @@ static void dvb_ca_private_free(struct dvb_ca_private *ca)
 
 static void dvb_ca_private_release(struct kref *ref)
 {
-	struct dvb_ca_private *ca = container_of(ref, struct dvb_ca_private, refcount);
+	struct dvb_ca_private *ca = container_of(ref, struct dvb_ca_private,
+						 refcount);
+
 	dvb_ca_private_free(ca);
 }
 
@@ -298,7 +300,9 @@ static int dvb_ca_en50221_wait_if_status(struct dvb_ca_private *ca, int slot,
 	timeout = jiffies + timeout_hz;
 	while (1) {
 		/* read the status and check for error */
-		int res = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
+		int res = ca->pub->read_cam_control(ca->pub, slot,
+						    CTRLIF_STATUS);
+
 		if (res < 0)
 			return -EIO;
 
-- 
2.7.4

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

* [PATCH V2 8/9] [media] dvb-core/dvb_ca_en50221.c: Removed useless braces
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (6 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 7/9] [media] dvb-core/dvb_ca_en50221.c: Added line breaks Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  2017-07-12 23:00 ` [PATCH V2 9/9] [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit Jasmin J.
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed all:
  WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 5a0b35d..3e390a4 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -242,9 +242,8 @@ static int dvb_ca_en50221_check_camstatus(struct dvb_ca_private *ca, int slot)
 	int cam_changed;
 
 	/* IRQ mode */
-	if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE) {
+	if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)
 		return (atomic_read(&sl->camchange_count) != 0);
-	}
 
 	/* poll mode */
 	slot_status = ca->pub->poll_slot_status(ca->pub, slot, ca->open);
@@ -258,11 +257,10 @@ static int dvb_ca_en50221_check_camstatus(struct dvb_ca_private *ca, int slot)
 	}
 
 	if (cam_changed) {
-		if (!cam_present_now) {
+		if (!cam_present_now)
 			sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
-		} else {
+		else
 			sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_INSERTED;
-		}
 		atomic_set(&sl->camchange_count, 1);
 	} else {
 		if ((sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
@@ -314,9 +312,8 @@ static int dvb_ca_en50221_wait_if_status(struct dvb_ca_private *ca, int slot,
 		}
 
 		/* check for timeout */
-		if (time_after(jiffies, timeout)) {
+		if (time_after(jiffies, timeout))
 			break;
-		}
 
 		/* wait for a bit */
 		usleep_range(1000, 1100);
@@ -782,9 +779,9 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 		buf[0], (buf[1] & 0x80) == 0, bytes_read);
 
 	/* wake up readers when a last_fragment is received */
-	if ((buf[1] & 0x80) == 0x00) {
+	if ((buf[1] & 0x80) == 0x00)
 		wake_up_interruptible(&ca->wait_queue);
-	}
+
 	status = bytes_read;
 
 exit:
@@ -1665,11 +1662,10 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 			connection_id = hdr[0];
 		if (hdr[0] == connection_id) {
 			if (pktlen < count) {
-				if ((pktlen + fraglen - 2) > count) {
+				if ((pktlen + fraglen - 2) > count)
 					fraglen = count - pktlen;
-				} else {
+				else
 					fraglen -= 2;
-				}
 
 				status =
 				   dvb_ringbuffer_pkt_read_user(&sl->rx_buffer,
@@ -1806,9 +1802,8 @@ static unsigned int dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
 
 	dprintk("%s\n", __func__);
 
-	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1) {
+	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
 		mask |= POLLIN;
-	}
 
 	/* if there is something, return now */
 	if (mask)
@@ -1817,9 +1812,8 @@ static unsigned int dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
 	/* wait for something to happen */
 	poll_wait(file, &ca->wait_queue, wait);
 
-	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1) {
+	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
 		mask |= POLLIN;
-	}
 
 	return mask;
 }
@@ -1961,9 +1955,9 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 	/* shutdown the thread if there was one */
 	kthread_stop(ca->thread);
 
-	for (i = 0; i < ca->slot_count; i++) {
+	for (i = 0; i < ca->slot_count; i++)
 		dvb_ca_en50221_slot_shutdown(ca, i);
-	}
+
 	dvb_remove_device(ca->dvbdev);
 	dvb_ca_private_put(ca);
 	pubca->private = NULL;
-- 
2.7.4

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

* [PATCH V2 9/9] [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit
  2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
                   ` (7 preceding siblings ...)
  2017-07-12 23:00 ` [PATCH V2 8/9] [media] dvb-core/dvb_ca_en50221.c: Removed useless braces Jasmin J.
@ 2017-07-12 23:00 ` Jasmin J.
  8 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:00 UTC (permalink / raw)
  To: linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller, jasmin

From: Jasmin Jessich <jasmin@anw.at>

Fixed most of:
  WARNING: line over 80 characters
The remaining lines are printk strings, which should not be split and
lines where I thing they should stay as they are.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 57 +++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 3e390a4..947c95c 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -76,7 +76,7 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
 #define STATUSREG_WE     2	/* write error */
 #define STATUSREG_FR  0x40	/* module free */
 #define STATUSREG_DA  0x80	/* data available */
-#define STATUSREG_TXERR (STATUSREG_RE|STATUSREG_WE)	/* general transfer error */
+#define STATUSREG_TXERR (STATUSREG_RE|STATUSREG_WE) /* general transfer error */
 
 
 #define DVB_CA_SLOTSTATE_NONE           0
@@ -157,7 +157,9 @@ struct dvb_ca_private {
 	/* Delay the main thread should use */
 	unsigned long delay;
 
-	/* Slot to start looking for data to read from in the next user-space read operation */
+	/* Slot to start looking for data to read from in the next user-space
+	 * read operation
+	 */
 	int next_read_slot;
 
 	/* mutex serializing ioctls */
@@ -227,7 +229,7 @@ static char *findstr(char *haystack, int hlen, char *needle, int nlen)
 
 
 
-/* ******************************************************************************** */
+/* ************************************************************************** */
 /* EN50221 physical interface functions */
 
 
@@ -346,8 +348,8 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	/* we'll be determining these during this function */
 	sl->da_irq_supported = 0;
 
-	/* set the host link buffer size temporarily. it will be overwritten with the
-	 * real negotiated size later.
+	/* set the host link buffer size temporarily. it will be overwritten
+	 * with the real negotiated size later.
 	 */
 	sl->link_buf_size = 2;
 
@@ -366,7 +368,9 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 	if (ret != 0)
 		return ret;
 
-	/* store it, and choose the minimum of our buffer and the CAM's buffer size */
+	/* store it, and choose the minimum of our buffer and the CAM's buffer
+	 * size
+	 */
 	buf_size = (buf[0] << 8) | buf[1];
 	if (buf_size > HOST_LINK_BUF_SIZE)
 		buf_size = HOST_LINK_BUF_SIZE;
@@ -435,7 +439,8 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private *ca, int slot,
 
 	/* read in the whole tuple */
 	for (i = 0; i < _tupleLength; i++) {
-		tuple[i] = ca->pub->read_attribute_mem(ca->pub, slot, _address + (i * 2));
+		tuple[i] = ca->pub->read_attribute_mem(ca->pub, slot,
+						       _address + (i * 2));
 		dprintk("  0x%02x: 0x%02x %c\n",
 			i, tuple[i] & 0xff,
 			((tuple[i] > 31) && (tuple[i] < 127)) ? tuple[i] : '.');
@@ -588,7 +593,7 @@ static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 			end_chain = 1;
 			break;
 
-		default:	/* Unknown tuple type - just skip this tuple and move to the next one */
+		default:	/* Unknown tuple type - just skip this tuple */
 			dprintk("dvb_ca: Skipping unknown tuple type:0x%x length:0x%x\n",
 				tupleType, tupleLength);
 			break;
@@ -764,7 +769,9 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot,
 		}
 	}
 
-	/* OK, add it to the receive buffer, or copy into external buffer if supplied */
+	/* OK, add it to the receive buffer, or copy into external buffer if
+	 * supplied
+	 */
 	if (ebuf == NULL) {
 		if (sl->rx_buffer.data == NULL) {
 			status = -EIO;
@@ -915,7 +922,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 
 
 
-/* ******************************************************************************** */
+/* ************************************************************************** */
 /* EN50221 higher level functions */
 
 
@@ -951,7 +958,8 @@ static int dvb_ca_en50221_slot_shutdown(struct dvb_ca_private *ca, int slot)
  * @slot: Slot concerned.
  * @change_type: One of the DVB_CA_CAMCHANGE_* values.
  */
-void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot, int change_type)
+void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot,
+				  int change_type)
 {
 	struct dvb_ca_private *ca = pubca->private;
 	struct dvb_ca_slot *sl = &ca->slot_info[slot];
@@ -1027,7 +1035,7 @@ void dvb_ca_en50221_frda_irq(struct dvb_ca_en50221 *pubca, int slot)
 EXPORT_SYMBOL(dvb_ca_en50221_frda_irq);
 
 
-/* ******************************************************************************** */
+/* ************************************************************************** */
 /* EN50221 thread functions */
 
 /**
@@ -1342,7 +1350,7 @@ static int dvb_ca_en50221_thread(void *data)
 
 
 
-/* ******************************************************************************** */
+/* ************************************************************************** */
 /* EN50221 IO interface functions */
 
 /**
@@ -1475,7 +1483,9 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 
 	dprintk("%s\n", __func__);
 
-	/* Incoming packet has a 2 byte header. hdr[0] = slot_id, hdr[1] = connection_id */
+	/* Incoming packet has a 2 byte header.
+	 * hdr[0] = slot_id, hdr[1] = connection_id
+	 */
 	if (count < 2)
 		return -EINVAL;
 
@@ -1513,14 +1523,17 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 		timeout = jiffies + HZ / 2;
 		written = 0;
 		while (!time_after(jiffies, timeout)) {
-			/* check the CAM hasn't been removed/reset in the meantime */
+			/* check the CAM hasn't been removed/reset in the
+			 * meantime
+			 */
 			if (sl->slot_state != DVB_CA_SLOTSTATE_RUNNING) {
 				status = -EIO;
 				goto exit;
 			}
 
 			mutex_lock(&sl->slot_lock);
-			status = dvb_ca_en50221_write_data(ca, slot, fragbuf, fraglen + 2);
+			status = dvb_ca_en50221_write_data(ca, slot, fragbuf,
+							   fraglen + 2);
 			mutex_unlock(&sl->slot_lock);
 			if (status == (fraglen + 2)) {
 				written = 1;
@@ -1574,7 +1587,8 @@ static int dvb_ca_en50221_io_read_condition(struct dvb_ca_private *ca,
 			dvb_ringbuffer_pkt_read(&sl->rx_buffer, idx, 0, hdr, 2);
 			if (connection_id == -1)
 				connection_id = hdr[0];
-			if ((hdr[0] == connection_id) && ((hdr[1] & 0x80) == 0)) {
+			if ((hdr[0] == connection_id) &&
+			    ((hdr[1] & 0x80) == 0)) {
 				*_slot = slot;
 				found = 1;
 				break;
@@ -1623,7 +1637,9 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user *buf,
 
 	dprintk("%s\n", __func__);
 
-	/* Outgoing packet has a 2 byte header. hdr[0] = slot_id, hdr[1] = connection_id */
+	/* Outgoing packet has a 2 byte header.
+	 * hdr[0] = slot_id, hdr[1] = connection_id
+	 */
 	if (count < 2)
 		return -EINVAL;
 
@@ -1842,7 +1858,7 @@ static const struct dvb_device dvbdev_ca = {
 	.fops = &dvb_ca_fops,
 };
 
-/* ******************************************************************************** */
+/* ************************************************************************** */
 /* Initialisation/shutdown functions */
 
 
@@ -1891,7 +1907,8 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 	pubca->private = ca;
 
 	/* register the DVB device */
-	ret = dvb_register_device(dvb_adapter, &ca->dvbdev, &dvbdev_ca, ca, DVB_DEVICE_CA, 0);
+	ret = dvb_register_device(dvb_adapter, &ca->dvbdev, &dvbdev_ca, ca,
+				  DVB_DEVICE_CA, 0);
 	if (ret)
 		goto free_slot_info;
 
-- 
2.7.4

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:00 ` [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments Jasmin J.
@ 2017-07-12 23:17   ` Antti Palosaari
  2017-07-12 23:23     ` Jasmin J.
  0 siblings, 1 reply; 17+ messages in thread
From: Antti Palosaari @ 2017-07-12 23:17 UTC (permalink / raw)
  To: Jasmin J., linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

On 07/13/2017 02:00 AM, Jasmin J. wrote:
> From: Jasmin Jessich <jasmin@anw.at>
> 
> Fixed all:
>    WARNING: Block comments use * on subsequent lines

Also multiline comments should be written like this:
/*
  * Comment.
  */

Quickly looking this patch serie I noticed few other coding style 
mistakes. You should read kernel coding style documentation first, and 
then make changes according to doc.

regards
Antti

-- 
http://palosaari.fi/

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:17   ` Antti Palosaari
@ 2017-07-12 23:23     ` Jasmin J.
  2017-07-12 23:28       ` Antti Palosaari
  0 siblings, 1 reply; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:23 UTC (permalink / raw)
  To: Antti Palosaari, linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

Hello Antti!

> Quickly looking this patch serie I noticed few other coding style mistakes.
> You should read kernel coding style documentation first, and then make
> changes according to doc.
In fact I used checkpatch.pl to find the issues and fixed them. All the patches
are 100% checkpatch.pl tested and did not have one single error or warning.

So please can you point me to those issues you mean.

BR,
   Jasmin

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:23     ` Jasmin J.
@ 2017-07-12 23:28       ` Antti Palosaari
  2017-07-12 23:45         ` Jasmin J.
  0 siblings, 1 reply; 17+ messages in thread
From: Antti Palosaari @ 2017-07-12 23:28 UTC (permalink / raw)
  To: Jasmin J., linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

On 07/13/2017 02:23 AM, Jasmin J. wrote:
> Hello Antti!
> 
>> Quickly looking this patch serie I noticed few other coding style mistakes.
>> You should read kernel coding style documentation first, and then make
>> changes according to doc.
> In fact I used checkpatch.pl to find the issues and fixed them. All the patches
> are 100% checkpatch.pl tested and did not have one single error or warning.
> 
> So please can you point me to those issues you mean.

Have you ever looked that coding style doc? Maybe better to start 
reading it first. Checkpatch is only a tool, it is nothing which makes 
100% decision which is correct or not.

Multi-line comment style is explained on section 8 on kernel coding 
style doc.

Antti


-- 
http://palosaari.fi/

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:28       ` Antti Palosaari
@ 2017-07-12 23:45         ` Jasmin J.
  2017-07-13  0:04           ` Antti Palosaari
  0 siblings, 1 reply; 17+ messages in thread
From: Jasmin J. @ 2017-07-12 23:45 UTC (permalink / raw)
  To: Antti Palosaari, linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

Hello Antti!

> Have you ever looked that coding style doc?
Yes I read it several times already and used it in my daily work in my
previous company.

Beside the Multi-line comment style, which I will fix in a follow up,
you mentioned other issues.
Please can you tell me which one you mean, so that I can check the series
for those things.

BR,
   Jasmin

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-12 23:45         ` Jasmin J.
@ 2017-07-13  0:04           ` Antti Palosaari
  2017-07-13  0:07             ` Antti Palosaari
  0 siblings, 1 reply; 17+ messages in thread
From: Antti Palosaari @ 2017-07-13  0:04 UTC (permalink / raw)
  To: Jasmin J., linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

On 07/13/2017 02:45 AM, Jasmin J. wrote:
> Hello Antti!
> 
>> Have you ever looked that coding style doc?
> Yes I read it several times already and used it in my daily work in my
> previous company.
> 
> Beside the Multi-line comment style, which I will fix in a follow up,
> you mentioned other issues.
> Please can you tell me which one you mean, so that I can check the series
> for those things.

eh, OK, here short list from my head:
* you fixed comments, but left //-comments

* many cases where if (ret != 0), which generally should be written as 
if (ret). If you expect it is just error ret value, then prefer if 
(ret), but if ret has some other meaning like it returns number of bytes 
then if you expect 0-bytes returned (ret != 0) is also valid.

* unnecessary looking line split like that:
if (a
       & b)

* logical continuous line split wrong (I think I have seen checkpatch 
reported that kind of mistakes, dunno why not now)
if (a
     && b)
== >
if (a &&
     b)


Antti

-- 
http://palosaari.fi/

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-13  0:04           ` Antti Palosaari
@ 2017-07-13  0:07             ` Antti Palosaari
  2017-07-13  7:29               ` Jasmin J.
  0 siblings, 1 reply; 17+ messages in thread
From: Antti Palosaari @ 2017-07-13  0:07 UTC (permalink / raw)
  To: Jasmin J., linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller



On 07/13/2017 03:04 AM, Antti Palosaari wrote:
> On 07/13/2017 02:45 AM, Jasmin J. wrote:
>> Hello Antti!
>>
>>> Have you ever looked that coding style doc?
>> Yes I read it several times already and used it in my daily work in my
>> previous company.
>>
>> Beside the Multi-line comment style, which I will fix in a follow up,
>> you mentioned other issues.
>> Please can you tell me which one you mean, so that I can check the series
>> for those things.
> 
> eh, OK, here short list from my head:
> * you fixed comments, but left //-comments
> 
> * many cases where if (ret != 0), which generally should be written as 
> if (ret). If you expect it is just error ret value, then prefer if 
> (ret), but if ret has some other meaning like it returns number of bytes 
> then if you expect 0-bytes returned (ret != 0) is also valid.
> 
> * unnecessary looking line split like that:
> if (a
>        & b)
> 
> * logical continuous line split wrong (I think I have seen checkpatch 
> reported that kind of mistakes, dunno why not now)
> if (a
>      && b)
> == >
> if (a &&
>      b)

actually it reports, when run --strict mode:

+       if (a
+           && b) {
+               foo(a);
+               foo(b);
+       }
+

CHECK: Logical continuations should be on the previous line
#11: FILE: drivers/media/usb/dvb-usb-v2/af9035.c:2135:
+	if (a
+	    && b) {


Antti
-- 
http://palosaari.fi/

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

* Re: [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  2017-07-13  0:07             ` Antti Palosaari
@ 2017-07-13  7:29               ` Jasmin J.
  0 siblings, 0 replies; 17+ messages in thread
From: Jasmin J. @ 2017-07-13  7:29 UTC (permalink / raw)
  To: Antti Palosaari, linux-media; +Cc: mchehab, max.kellermann, rjkm, d.scheller

Hello Antti!

> actually it reports, when run --strict mode
I checked this once, but I thought this would be too aggressive changes.

>> * many cases where if (ret != 0), which generally should be written as if
>> (ret). If you expect it is just error ret value, then prefer if (ret), but
>> if> ret has some other meaning like it returns number of bytes then if you
>> expect 0-bytes returned (ret != 0) is also valid.
In fact I did no real code changes to keep the impact as little as possible.
But I agree fully with you and in my drivers I used always (ret) or (!ret).
Although this has been changed in my new company when it comes to certified
software ... .

I will try also to compile with GCC 7.1.1, if I get one for my system.

>> * unnecessary looking line split like that:
>> if (a
>>        & b)
I am sure I did this because of the 80 col limit, but I will look again.

THX for your review and the valuable input. I will add you the receiver list
next time.

BR,
   Jasmin

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

end of thread, other threads:[~2017-07-13  7:30 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-12 23:00 [PATCH V2 0/9] Fix coding style in en50221 CAM functions Jasmin J.
2017-07-12 23:00 ` [PATCH V2 1/9] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread Jasmin J.
2017-07-12 23:00 ` [PATCH V2 2/9] [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone Jasmin J.
2017-07-12 23:00 ` [PATCH V2 3/9] [media] dvb-core/dvb_ca_en50221.c: use usleep_range Jasmin J.
2017-07-12 23:00 ` [PATCH V2 4/9] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments Jasmin J.
2017-07-12 23:17   ` Antti Palosaari
2017-07-12 23:23     ` Jasmin J.
2017-07-12 23:28       ` Antti Palosaari
2017-07-12 23:45         ` Jasmin J.
2017-07-13  0:04           ` Antti Palosaari
2017-07-13  0:07             ` Antti Palosaari
2017-07-13  7:29               ` Jasmin J.
2017-07-12 23:00 ` [PATCH V2 5/9] [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs Jasmin J.
2017-07-12 23:00 ` [PATCH V2 6/9] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable Jasmin J.
2017-07-12 23:00 ` [PATCH V2 7/9] [media] dvb-core/dvb_ca_en50221.c: Added line breaks Jasmin J.
2017-07-12 23:00 ` [PATCH V2 8/9] [media] dvb-core/dvb_ca_en50221.c: Removed useless braces Jasmin J.
2017-07-12 23:00 ` [PATCH V2 9/9] [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit Jasmin J.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.