[U-Boot] [PATCH 0/3] net: tsec: Fixes for mcast() in the tsec driver

[U-Boot] [PATCH 0/3] net: tsec: Fixes for mcast() in the tsec driver

[Claudiu Manoil]

Though this patchset fixes tsec's driver mcast() instance primarily,
the fix from net.h was propagated to other drivers too, as appropriate.
Notably, these fixes also end up in removal of some unwanted global
static vars from the tsec driver.

Thanks.
Claudiu

Cc: Joe Hershberger <joe.hershberger@gmail.com>

Claudiu Manoil (3):
  net: Fix mcast function pointer prototype
  net: tsec: Fix and cleanup tsec_mcast_addr()
  net: tsec: Fix priv pointer in tsec_mcast_addr()

 drivers/net/rtl8139.c |  2 +-
 drivers/net/tsec.c    | 47 ++++++++++++++++++++---------------------------
 include/net.h         |  2 +-
 3 files changed, 22 insertions(+), 29 deletions(-)

-- 
1.7.11.7

[U-Boot] [PATCH 1/3] net: Fix mcast function pointer prototype

[Claudiu Manoil]

This fixes the following compiler warnings when activating
CONFIG_MCAST_TFTP:

tsec.c: In function 'tsec_mcast_addr':
tsec.c:130:2: warning: passing argument 2 of 'ether_crc' makes pointer
from integer without a cast [enabled by default]
In file included from /work/u-boot-net/include/common.h:874:0,
                 from tsec.c:15:
/work/u-boot-net/include/net.h:189:5: note: expected 'const unsigned
char *' but argument is of type 'u8'
tsec.c: In function 'tsec_initialize':
tsec.c:646:13: warning: assignment from incompatible pointer type
[enabled by default]
eth.c: In function 'eth_mcast_join':
eth.c:358:2: warning: passing argument 2 of 'eth_current->mcast' makes
integer from pointer without a cast [enabled by default]
eth.c:358:2: note: expected 'u32' but argument is of type 'u8 *'

In the eth_mcast_join() implementation, eth_current->mcast()
takes a u8 pointer to the multicast mac address and not a ip
address value as implied by its prototype.

Fix parameter type mismatch for tsec_macst_addr() (tsec.c):
ether_crc() takes a u8 pointer not a u8 value.
mcast() is given a u8 pointer to the multicats mac address.
Update parameter type for the rest of mcast() instances.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/rtl8139.c | 2 +-
 drivers/net/tsec.c    | 4 ++--
 include/net.h         | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
index 4186699..208ce5c 100644
--- a/drivers/net/rtl8139.c
+++ b/drivers/net/rtl8139.c
@@ -188,7 +188,7 @@ static int rtl_transmit(struct eth_device *dev, void *packet, int length);
 static int rtl_poll(struct eth_device *dev);
 static void rtl_disable(struct eth_device *dev);
 #ifdef CONFIG_MCAST_TFTP/*  This driver already accepts all b/mcast */
-static int rtl_bcast_addr (struct eth_device *dev, u8 bcast_mac, u8 set)
+static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac, u8 set)
 {
 	return (0);
 }
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index f5e314b..3428dd0 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -120,14 +120,14 @@ static void tsec_configure_serdes(struct tsec_private *priv)
  * for PowerPC (tm) is usually the case) in the tregister holds
  * the entry. */
 static int
-tsec_mcast_addr (struct eth_device *dev, u8 mcast_mac, u8 set)
+tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
 {
 	struct tsec_private *priv = privlist[1];
 	volatile tsec_t *regs = priv->regs;
 	volatile u32  *reg_array, value;
 	u8 result, whichbit, whichreg;
 
-	result = (u8)((ether_crc(MAC_ADDR_LEN,mcast_mac) >> 24) & 0xff);
+	result = (u8)((ether_crc(MAC_ADDR_LEN, mcast_mac) >> 24) & 0xff);
 	whichbit = result & 0x1f;	/* the 5 LSB = which bit to set */
 	whichreg = result >> 5;		/* the 3 MSB = which reg to set it in */
 	value = (1 << (31-whichbit));
diff --git a/include/net.h b/include/net.h
index 5aedc17..0802fad 100644
--- a/include/net.h
+++ b/include/net.h
@@ -89,7 +89,7 @@ struct eth_device {
 	int  (*recv) (struct eth_device *);
 	void (*halt) (struct eth_device *);
 #ifdef CONFIG_MCAST_TFTP
-	int (*mcast) (struct eth_device *, u32 ip, u8 set);
+	int (*mcast) (struct eth_device *, const u8 *enetaddr, u8 set);
 #endif
 	int  (*write_hwaddr) (struct eth_device *);
 	struct eth_device *next;
-- 
1.7.11.7

[U-Boot] [PATCH 2/3] net: tsec: Fix and cleanup tsec_mcast_addr()

[Claudiu Manoil]

There are several implementation issues for tsec_mcast_addr()
addressed by this patch:
* unmanaged, not portable r/w access to registers; fixed with
setbits_be32()/ clrbits_be32()
* use of volatile pointers
* unnecessary forced cast to u8 for the ether_crc() result
* removed redundant parens
* corrected some comment slips

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/tsec.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 3428dd0..9ffc801 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -113,32 +113,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
  * result.
  * 2) Use the 8 most significant bits as a hash into a 256-entry
  * table.  The table is controlled through 8 32-bit registers:
- * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is
- * gaddr7.  This means that the 3 most significant bits in the
+ * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is entry
+ * 255.  This means that the 3 most significant bits in the
  * hash index which gaddr register to use, and the 5 other bits
  * indicate which bit (assuming an IBM numbering scheme, which
- * for PowerPC (tm) is usually the case) in the tregister holds
+ * for PowerPC (tm) is usually the case) in the register holds
  * the entry. */
 static int
 tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
 {
 	struct tsec_private *priv = privlist[1];
-	volatile tsec_t *regs = priv->regs;
-	volatile u32  *reg_array, value;
-	u8 result, whichbit, whichreg;
+	struct tsec __iomem *regs = priv->regs;
+	u32 result, value;
+	u8 whichbit, whichreg;
 
-	result = (u8)((ether_crc(MAC_ADDR_LEN, mcast_mac) >> 24) & 0xff);
-	whichbit = result & 0x1f;	/* the 5 LSB = which bit to set */
-	whichreg = result >> 5;		/* the 3 MSB = which reg to set it in */
-	value = (1 << (31-whichbit));
+	result = ether_crc(MAC_ADDR_LEN, mcast_mac);
+	whichbit = (result >> 24) & 0x1f; /* the 5 LSB = which bit to set */
+	whichreg = result >> 29; /* the 3 MSB = which reg to set it in */
 
-	reg_array = &(regs->hash.gaddr0);
+	value = 1 << (31-whichbit);
+
+	if (set)
+		setbits_be32(&regs->hash.gaddr0 + whichreg, value);
+	else
+		clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
 
-	if (set) {
-		reg_array[whichreg] |= value;
-	} else {
-		reg_array[whichreg] &= ~value;
-	}
 	return 0;
 }
 #endif /* Multicast TFTP ? */
-- 
1.7.11.7

[U-Boot] [PATCH 3/3] net: tsec: Fix priv pointer in tsec_mcast_addr()

[Claudiu Manoil]

Access to privlist[1] (hardcoded referece to the 2nd tsec's
priv area) is neither correct nor does it make sense in the
current context.  Each tsec dev has access to its own priv
instance only, and hence to its own set of group address
registers (GADDR) to filter multicast addresses.

This fix leads to removal of the unused (faulty) privlist[]
and related global static vars.  Note that mcast() can be
called only after eth_device allocation and init, and hence
after priv area allocation, so dev->priv is correctly
initialized upon mcast() call.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/tsec.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 9ffc801..9371ffa 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -33,11 +33,6 @@ typedef volatile struct rtxbd {
 	rxbd8_t rxbd[PKTBUFSRX];
 } RTXBD;
 
-#define MAXCONTROLLERS	(8)
-
-static struct tsec_private *privlist[MAXCONTROLLERS];
-static int num_tsecs = 0;
-
 #ifdef __GNUC__
 static RTXBD rtx __attribute__ ((aligned(8)));
 #else
@@ -122,7 +117,7 @@ static void tsec_configure_serdes(struct tsec_private *priv)
 static int
 tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
 {
-	struct tsec_private *priv = privlist[1];
+	struct tsec_private *priv = (struct tsec_private *)dev->priv;
 	struct tsec __iomem *regs = priv->regs;
 	u32 result, value;
 	u8 whichbit, whichreg;
@@ -625,7 +620,6 @@ static int tsec_initialize(bd_t *bis, struct tsec_info_struct *tsec_info)
 	if (NULL == priv)
 		return 0;
 
-	privlist[num_tsecs++] = priv;
 	priv->regs = tsec_info->regs;
 	priv->phyregs_sgmii = tsec_info->miiregs_sgmii;
 
-- 
1.7.11.7