Re: [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit

On Thu, Oct 11, 2012 at 11:26:29PM +0200, Francois Romieu wrote:

Thomas Petazzoni [off-list ref] :
[...]

quoted

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
new file mode 100644
index 0000000..4f7fe08
--- /dev/null
+++ b/drivers/net/ethernet/marvell/mvneta.c

[...]

quoted

+static void mvneta_rxq_desc_num_update(struct mvneta_port *pp,
+				       struct mvneta_rx_queue *rxq,
+				       int rx_done, int rx_filled)
+{
+	u32 val;
+
+	if ((rx_done <= 0xff) && (rx_filled <= 0xff)) {
+		val = rx_done |
+		  (rx_filled << MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT);
+		mvreg_write(pp, MVNETA_RXQ_STATUS_UPDATE_REG(rxq->id), val);
+		return;
+	}
+
+	/* Only 255 descriptors can be added at once */
+	while ((rx_done > 0) || (rx_filled > 0)) {
+		if (rx_done <= 0xff) {
+			val = rx_done;
+			rx_done = 0;
+		} else {
+			val = 0xff;
+			rx_done -= 0xff;
+		}
+		if (rx_filled <= 0xff) {
+			val |= rx_filled
+				<< MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT;

			val |= rx_filled << MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT;

I agree with Francois on most of these.  I prefer readability over hard
80 column limits.  Although, 80 columns is still sound guidance.  For
example, a majority of the broken lines are due to long macro and
constant names.  I did a 'git grep NETA' and didn't see anything
alarming.  So, above could become

			val |= rx_filled << NETA_RXQ_ADD_NONOCC_SHIFT;

thx,

Jason.