Factor out register bit twiddling in the Renesas Ethernet drivers

PATCH In review Sergei Shtylyov 2 participants 4 messages Feb 9, 2016 – Feb 10, 2016

Subsystems (4)

Networking Network drivers Renesas Ethernet avb driver Renesas SuperH Ethernet driver

lore

Share & tools

lore

Sergei Shtylyov

author

2016-02-09 22:36 UTC #0 lore raw

v2 cover Factor out register bit twiddling in the Renesas Ethernet drivers

Hello.

Here's a set of 2 patches against DaveM's 'net-next.git' repo. We factor out
the often repeated pattern of reading a register, AND'ing and/or OR'ing some bits, and then writing the value back.

[1/2] ravb: factor out register bit twiddling code [2/2] sh_eth: factor out register bit twiddling code

MBR, Sergei

Sergei Shtylyov

author

2016-02-09 22:37 UTC #1 lore raw

PATCH v2 1/2 ravb: factor out register bit twiddling code

The driver has often repeated pattern of reading a register, AND'ing and/or OR'ing some bits and writing the value back. Factor the pattern out into ravb_modify() -- this saves 260 bytes of code with ARM gcc 4.7.3.

While at it, update Cogent Embedded's copyrights.

Signed-off-by Sergei Shtylyov <[email protected]>

---
Changes in version 2:
- renamed ravb_modify()'s parameters.

 drivers/net/ethernet/renesas/ravb.h      |    4 +
 drivers/net/ethernet/renesas/ravb_main.c |   68 ++++++++++++-------------------
 drivers/net/ethernet/renesas/ravb_ptp.c  |   25 ++---------
 3 files changed, 36 insertions(+), 61 deletions(-)

drivers/net/ethernet/renesas/ravb.h +3 -1

Index: net-next/drivers/net/ethernet/renesas/ravb.h
===================================================================
@@ -2,7 +2,7 @@
  *
  * Copyright (C) 2014-2015 Renesas Electronics Corporation
  * Copyright (C) 2015 Renesas Solutions Corp.
- * Copyright (C) 2015 Cogent Embedded, Inc. <[email protected]>
+ * Copyright (C) 2015-2016 Cogent Embedded, Inc. <[email protected]>
  *
  * Based on the SuperH Ethernet driver
  *

@@ -837,6 +837,8 @@ static inline void ravb_write(struct net
 	iowrite32(data, priv->addr + reg);
 }
 
+void ravb_modify(struct net_device *ndev, enum ravb_reg reg, u32 clear,
+		 u32 set);
 int ravb_wait(struct net_device *ndev, enum ravb_reg reg, u32 mask, u32 value);
 
 irqreturn_t ravb_ptp_interrupt(struct net_device *ndev);

drivers/net/ethernet/renesas/ravb_main.c +27 ~~-41~~

170 lines not on this page · raw message

drivers/net/ethernet/renesas/ravb_ptp.c +6 ~~-19~~

Index: net-next/drivers/net/ethernet/renesas/ravb_ptp.c
===================================================================
@@ -2,7 +2,7 @@
  *
  * Copyright (C) 2013-2015 Renesas Electronics Corporation
  * Copyright (C) 2015 Renesas Solutions Corp.
- * Copyright (C) 2015 Cogent Embedded, Inc. <[email protected]>
+ * Copyright (C) 2015-2016 Cogent Embedded, Inc. <[email protected]>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by

@@ -21,7 +21,7 @@ static int ravb_ptp_tcr_request(struct r
 	if (error)
 		return error;
 
-	ravb_write(ndev, ravb_read(ndev, GCCR) | request, GCCR);
+	ravb_modify(ndev, GCCR, request, request);
 	return ravb_wait(ndev, GCCR, GCCR_TCR, GCCR_TCR_NOREQ);
 }

@@ -185,7 +185,6 @@ static int ravb_ptp_extts(struct ptp_clo
 						 ptp.info);
 	struct net_device *ndev = priv->ndev;
 	unsigned long flags;
-	u32 gic;
 
 	if (req->index)
 		return -EINVAL;

@@ -195,12 +194,7 @@ static int ravb_ptp_extts(struct ptp_clo
 	priv->ptp.extts[req->index] = on;
 
 	spin_lock_irqsave(&priv->lock, flags);
-	gic = ravb_read(ndev, GIC);
-	if (on)
-		gic |= GIC_PTCE;
-	else
-		gic &= ~GIC_PTCE;
-	ravb_write(ndev, gic, GIC);
+	ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0);
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);

@@ -216,7 +210,6 @@ static int ravb_ptp_perout(struct ptp_cl
 	struct ravb_ptp_perout *perout;
 	unsigned long flags;
 	int error = 0;
-	u32 gic;
 
 	if (req->index)
 		return -EINVAL;

@@ -248,9 +241,7 @@ static int ravb_ptp_perout(struct ptp_cl
 		error = ravb_ptp_update_compare(priv, (u32)start_ns);
 		if (!error) {
 			/* Unmask interrupt */
-			gic = ravb_read(ndev, GIC);
-			gic |= GIC_PTME;
-			ravb_write(ndev, gic, GIC);
+			ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME);
 		}
 	} else	{
 		spin_lock_irqsave(&priv->lock, flags);

@@ -259,9 +250,7 @@ static int ravb_ptp_perout(struct ptp_cl
 		perout->period = 0;
 
 		/* Mask interrupt */
-		gic = ravb_read(ndev, GIC);
-		gic &= ~GIC_PTME;
-		ravb_write(ndev, gic, GIC);
+		ravb_modify(ndev, GIC, GIC_PTME, 0);
 	}
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);

@@ -331,7 +320,6 @@ void ravb_ptp_init(struct net_device *nd
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	unsigned long flags;
-	u32 gccr;
 
 	priv->ptp.info = ravb_ptp_info;

@@ -340,8 +328,7 @@ void ravb_ptp_init(struct net_device *nd
 
 	spin_lock_irqsave(&priv->lock, flags);
 	ravb_wait(ndev, GCCR, GCCR_TCR, GCCR_TCR_NOREQ);
-	gccr = ravb_read(ndev, GCCR) & ~GCCR_TCSS;
-	ravb_write(ndev, gccr | GCCR_TCSS_ADJGPTP, GCCR);
+	ravb_modify(ndev, GCCR, GCCR_TCSS, GCCR_TCSS_ADJGPTP);
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);

Sergei Shtylyov

author

2016-02-09 22:38 UTC #2 lore raw

PATCH v2 2/2 sh_eth: factor out register bit twiddling code

In reply to Sergei Shtylyov

The driver has often repeated pattern of reading a register, AND'ing and/or OR'ing some bits and writing the value back. Factor the pattern out into sh_eth_modify() -- this saves 84 bytes of code with ARM gcc 4.7.3.

While at it, update Cogent Embedded's copyright.

Signed-off-by Sergei Shtylyov <[email protected]>

---
Changes in version 2:
- renamed sh_eth_modify()'s parameters.

 drivers/net/ethernet/renesas/sh_eth.c |   53 +++++++++++++++-------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

drivers/net/ethernet/renesas/sh_eth.c +24 ~~-29~~

Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
@@ -3,7 +3,7 @@
  *  Copyright (C) 2014  Renesas Electronics Corporation
  *  Copyright (C) 2006-2012 Nobuhiro Iwamatsu
  *  Copyright (C) 2008-2014 Renesas Solutions Corp.
- *  Copyright (C) 2013-2014 Cogent Embedded, Inc.
+ *  Copyright (C) 2013-2016 Cogent Embedded, Inc.
  *  Copyright (C) 2014 Codethink Limited
  *
  *  This program is free software; you can redistribute it and/or modify it

@@ -428,6 +428,13 @@ static u32 sh_eth_read(struct net_device
 	return ioread32(mdp->addr + offset);
 }
 
+static void sh_eth_modify(struct net_device *ndev, int enum_index, u32 clear,
+			  u32 set)
+{
+	sh_eth_write(ndev, (sh_eth_read(ndev, enum_index) & ~clear) | set,
+		     enum_index);
+}
+
 static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 {
 	return mdp->reg_offset == sh_eth_offset_gigabit;

@@ -467,10 +474,7 @@ static void sh_eth_set_duplex(struct net
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
+	sh_eth_modify(ndev, ECMR, ECMR_DM, mdp->duplex ? ECMR_DM : 0);
 }
 
 static void sh_eth_chip_reset(struct net_device *ndev)

@@ -583,10 +587,10 @@ static void sh_eth_set_rate_r8a777x(stru
 
 	switch (mdp->speed) {
 	case 10: /* 10BASE */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_ELB, ECMR);
+		sh_eth_modify(ndev, ECMR, ECMR_ELB, 0);
 		break;
 	case 100:/* 100BASE */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_ELB, ECMR);
+		sh_eth_modify(ndev, ECMR, ECMR_ELB, ECMR_ELB);
 		break;
 	default:
 		break;

@@ -649,10 +653,10 @@ static void sh_eth_set_rate_sh7724(struc
 
 	switch (mdp->speed) {
 	case 10: /* 10BASE */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_RTM, ECMR);
+		sh_eth_modify(ndev, ECMR, ECMR_RTM, 0);
 		break;
 	case 100:/* 100BASE */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_RTM, ECMR);
+		sh_eth_modify(ndev, ECMR, ECMR_RTM, ECMR_RTM);
 		break;
 	default:
 		break;

@@ -924,8 +928,7 @@ static int sh_eth_reset(struct net_devic
 
 	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp)) {
 		sh_eth_write(ndev, EDSR_ENALL, EDSR);
-		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER,
-			     EDMR);
+		sh_eth_modify(ndev, EDMR, EDMR_SRST_GETHER, EDMR_SRST_GETHER);
 
 		ret = sh_eth_check_reset(ndev);
 		if (ret)

@@ -949,11 +952,9 @@ static int sh_eth_reset(struct net_devic
 		if (mdp->cd->select_mii)
 			sh_eth_select_mii(ndev);
 	} else {
-		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_ETHER,
-			     EDMR);
+		sh_eth_modify(ndev, EDMR, EDMR_SRST_ETHER, EDMR_SRST_ETHER);
 		mdelay(3);
-		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) & ~EDMR_SRST_ETHER,
-			     EDMR);
+		sh_eth_modify(ndev, EDMR, EDMR_SRST_ETHER, 0);
 	}
 
 	return ret;

@@ -1285,7 +1286,7 @@ static int sh_eth_dev_init(struct net_de
 	sh_eth_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN,
 		     RFLR);
 
-	sh_eth_write(ndev, sh_eth_read(ndev, EESR), EESR);
+	sh_eth_modify(ndev, EESR, 0, 0);
 	if (start) {
 		mdp->irq_enabled = true;
 		sh_eth_write(ndev, mdp->cd->eesipr_value, EESIPR);

@@ -1532,15 +1533,13 @@ static int sh_eth_rx(struct net_device *
 static void sh_eth_rcv_snd_disable(struct net_device *ndev)
 {
 	/* disable tx and rx */
-	sh_eth_write(ndev, sh_eth_read(ndev, ECMR) &
-		~(ECMR_RE | ECMR_TE), ECMR);
+	sh_eth_modify(ndev, ECMR, ECMR_RE | ECMR_TE, 0);
 }
 
 static void sh_eth_rcv_snd_enable(struct net_device *ndev)
 {
 	/* enable tx and rx */
-	sh_eth_write(ndev, sh_eth_read(ndev, ECMR) |
-		(ECMR_RE | ECMR_TE), ECMR);
+	sh_eth_modify(ndev, ECMR, ECMR_RE | ECMR_TE, ECMR_RE | ECMR_TE);
 }
 
 /* error control function */

@@ -1569,13 +1568,11 @@ static void sh_eth_error(struct net_devi
 				sh_eth_rcv_snd_disable(ndev);
 			} else {
 				/* Link Up */
-				sh_eth_write(ndev, sh_eth_read(ndev, EESIPR) &
-						   ~DMAC_M_ECI, EESIPR);
+				sh_eth_modify(ndev, EESIPR, DMAC_M_ECI, 0);
 				/* clear int */
-				sh_eth_write(ndev, sh_eth_read(ndev, ECSR),
-					     ECSR);
-				sh_eth_write(ndev, sh_eth_read(ndev, EESIPR) |
-						   DMAC_M_ECI, EESIPR);
+				sh_eth_modify(ndev, ECSR, 0, 0);
+				sh_eth_modify(ndev, EESIPR, DMAC_M_ECI,
+					      DMAC_M_ECI);
 				/* enable tx and rx */
 				sh_eth_rcv_snd_enable(ndev);
 			}

@@ -1765,9 +1762,7 @@ static void sh_eth_adjust_link(struct ne
 				mdp->cd->set_rate(ndev);
 		}
 		if (!mdp->link) {
-			sh_eth_write(ndev,
-				     sh_eth_read(ndev, ECMR) & ~ECMR_TXF,
-				     ECMR);
+			sh_eth_modify(ndev, ECMR, ECMR_TXF, 0);
 			new_state = 1;
 			mdp->link = phydev->link;
 			if (mdp->cd->no_psr || mdp->no_ether_link)

David Miller

maintainer

2016-02-10 10:38 UTC #3 lore raw

In reply to Sergei Shtylyov

From: Sergei Shtylyov <[email protected]>
Date: Wed, 10 Feb 2016 01:36:55 +0300

Quoted 6 lines

Here's a set of 2 patches against DaveM's 'net-next.git' repo. We factor out
the often repeated pattern of reading a register, AND'ing and/or OR'ing some bits, and then writing the value back.
[1/2] ravb: factor out register bit twiddling code [2/2] sh_eth: factor out register bit twiddling code

Series applied, thanks.