[PATCH 1/5] bcma: export bcma_find_core_unit()

[PATCH 1/5] bcma: export bcma_find_core_unit()

[Hauke Mehrtens]

This function is used to get a specific core when there is more than
one core of that specific type. This is used in bgmac to reset all GMAC
cores.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/bcma/bcma_private.h |    2 --
 drivers/bcma/main.c         |   13 +------------
 include/linux/bcma/bcma.h   |    9 ++++++++-
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
index 0215f9a..09b632a 100644
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -33,8 +33,6 @@ int __init bcma_bus_early_register(struct bcma_bus *bus,
 int bcma_bus_suspend(struct bcma_bus *bus);
 int bcma_bus_resume(struct bcma_bus *bus);
 #endif
-struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
-					u8 unit);
 
 /* scan.c */
 int bcma_bus_scan(struct bcma_bus *bus);
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 5a9f6bd..34ea4c5 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -78,18 +78,6 @@ static u16 bcma_cc_core_id(struct bcma_bus *bus)
 	return BCMA_CORE_CHIPCOMMON;
 }
 
-struct bcma_device *bcma_find_core(struct bcma_bus *bus, u16 coreid)
-{
-	struct bcma_device *core;
-
-	list_for_each_entry(core, &bus->cores, list) {
-		if (core->id.id == coreid)
-			return core;
-	}
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(bcma_find_core);
-
 struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
 					u8 unit)
 {
@@ -101,6 +89,7 @@ struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
 	}
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(bcma_find_core_unit);
 
 bool bcma_wait_value(struct bcma_device *core, u16 reg, u32 mask, u32 value,
 		     int timeout)
diff --git a/include/linux/bcma/bcma.h b/include/linux/bcma/bcma.h
index 4d043c3..0b3bb16 100644
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -418,7 +418,14 @@ static inline void bcma_maskset16(struct bcma_device *cc,
 	bcma_write16(cc, offset, (bcma_read16(cc, offset) & mask) | set);
 }
 
-extern struct bcma_device *bcma_find_core(struct bcma_bus *bus, u16 coreid);
+extern struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
+					       u8 unit);
+static inline struct bcma_device *bcma_find_core(struct bcma_bus *bus,
+						 u16 coreid)
+{
+	return bcma_find_core_unit(bus, coreid, 0);
+}
+
 extern bool bcma_core_is_enabled(struct bcma_device *core);
 extern void bcma_core_disable(struct bcma_device *core, u32 flags);
 extern int bcma_core_enable(struct bcma_device *core, u32 flags);
-- 
1.7.10.4

[PATCH 2/5] bgmac: initialize the DMA controller of core rev >= 4

[Hauke Mehrtens]

The DMA controller used in the device supported by GMAC with core rev
>= 4 has some new options which are now set to the default values used
in the Broadcom SDK.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |   23 +++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.h |   50 +++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 39efb86..aa5e289 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -96,6 +96,19 @@ static void bgmac_dma_tx_enable(struct bgmac *bgmac,
 	u32 ctl;
 
 	ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL);
+	if (bgmac->core->id.rev >= 4) {
+		ctl &= ~BGMAC_DMA_TX_BL_MASK;
+		ctl |= BGMAC_DMA_TX_BL_128 << BGMAC_DMA_TX_BL_SHIFT;
+
+		ctl &= ~BGMAC_DMA_TX_MR_MASK;
+		ctl |= BGMAC_DMA_TX_MR_2 << BGMAC_DMA_TX_MR_SHIFT;
+
+		ctl &= ~BGMAC_DMA_TX_PC_MASK;
+		ctl |= BGMAC_DMA_TX_PC_16 << BGMAC_DMA_TX_PC_SHIFT;
+
+		ctl &= ~BGMAC_DMA_TX_PT_MASK;
+		ctl |= BGMAC_DMA_TX_PT_8 << BGMAC_DMA_TX_PT_SHIFT;
+	}
 	ctl |= BGMAC_DMA_TX_ENABLE;
 	ctl |= BGMAC_DMA_TX_PARITY_DISABLE;
 	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, ctl);
@@ -240,6 +253,16 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 	u32 ctl;
 
 	ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL);
+	if (bgmac->core->id.rev >= 4) {
+		ctl &= ~BGMAC_DMA_RX_BL_MASK;
+		ctl |= BGMAC_DMA_RX_BL_128 << BGMAC_DMA_RX_BL_SHIFT;
+
+		ctl &= ~BGMAC_DMA_RX_PC_MASK;
+		ctl |= BGMAC_DMA_RX_PC_8 << BGMAC_DMA_RX_PC_SHIFT;
+
+		ctl &= ~BGMAC_DMA_RX_PT_MASK;
+		ctl |= BGMAC_DMA_RX_PT_1 << BGMAC_DMA_RX_PT_SHIFT;
+	}
 	ctl &= BGMAC_DMA_RX_ADDREXT_MASK;
 	ctl |= BGMAC_DMA_RX_ENABLE;
 	ctl |= BGMAC_DMA_RX_PARITY_DISABLE;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 130b16b..71adac9 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -237,9 +237,34 @@
 #define  BGMAC_DMA_TX_SUSPEND			0x00000002
 #define  BGMAC_DMA_TX_LOOPBACK			0x00000004
 #define  BGMAC_DMA_TX_FLUSH			0x00000010
+#define  BGMAC_DMA_TX_MR_MASK			0x000000C0	/* Multiple outstanding reads */
+#define  BGMAC_DMA_TX_MR_SHIFT			6
+#define   BGMAC_DMA_TX_MR_1			0
+#define   BGMAC_DMA_TX_MR_2			1
 #define  BGMAC_DMA_TX_PARITY_DISABLE		0x00000800
 #define  BGMAC_DMA_TX_ADDREXT_MASK		0x00030000
 #define  BGMAC_DMA_TX_ADDREXT_SHIFT		16
+#define  BGMAC_DMA_TX_BL_MASK			0x001C0000	/* BurstLen bits */
+#define  BGMAC_DMA_TX_BL_SHIFT			18
+#define   BGMAC_DMA_TX_BL_16			0
+#define   BGMAC_DMA_TX_BL_32			1
+#define   BGMAC_DMA_TX_BL_64			2
+#define   BGMAC_DMA_TX_BL_128			3
+#define   BGMAC_DMA_TX_BL_256			4
+#define   BGMAC_DMA_TX_BL_512			5
+#define   BGMAC_DMA_TX_BL_1024			6
+#define  BGMAC_DMA_TX_PC_MASK			0x00E00000	/* Prefetch control */
+#define  BGMAC_DMA_TX_PC_SHIFT			21
+#define   BGMAC_DMA_TX_PC_0			0
+#define   BGMAC_DMA_TX_PC_4			1
+#define   BGMAC_DMA_TX_PC_8			2
+#define   BGMAC_DMA_TX_PC_16			3
+#define  BGMAC_DMA_TX_PT_MASK			0x03000000	/* Prefetch threshold */
+#define  BGMAC_DMA_TX_PT_SHIFT			24
+#define   BGMAC_DMA_TX_PT_1			0
+#define   BGMAC_DMA_TX_PT_2			1
+#define   BGMAC_DMA_TX_PT_4			2
+#define   BGMAC_DMA_TX_PT_8			3
 #define BGMAC_DMA_TX_INDEX			0x04
 #define BGMAC_DMA_TX_RINGLO			0x08
 #define BGMAC_DMA_TX_RINGHI			0x0C
@@ -267,8 +292,33 @@
 #define  BGMAC_DMA_RX_DIRECT_FIFO		0x00000100
 #define  BGMAC_DMA_RX_OVERFLOW_CONT		0x00000400
 #define  BGMAC_DMA_RX_PARITY_DISABLE		0x00000800
+#define  BGMAC_DMA_RX_MR_MASK			0x000000C0	/* Multiple outstanding reads */
+#define  BGMAC_DMA_RX_MR_SHIFT			6
+#define   BGMAC_DMA_TX_MR_1			0
+#define   BGMAC_DMA_TX_MR_2			1
 #define  BGMAC_DMA_RX_ADDREXT_MASK		0x00030000
 #define  BGMAC_DMA_RX_ADDREXT_SHIFT		16
+#define  BGMAC_DMA_RX_BL_MASK			0x001C0000	/* BurstLen bits */
+#define  BGMAC_DMA_RX_BL_SHIFT			18
+#define   BGMAC_DMA_RX_BL_16			0
+#define   BGMAC_DMA_RX_BL_32			1
+#define   BGMAC_DMA_RX_BL_64			2
+#define   BGMAC_DMA_RX_BL_128			3
+#define   BGMAC_DMA_RX_BL_256			4
+#define   BGMAC_DMA_RX_BL_512			5
+#define   BGMAC_DMA_RX_BL_1024			6
+#define  BGMAC_DMA_RX_PC_MASK			0x00E00000	/* Prefetch control */
+#define  BGMAC_DMA_RX_PC_SHIFT			21
+#define   BGMAC_DMA_RX_PC_0			0
+#define   BGMAC_DMA_RX_PC_4			1
+#define   BGMAC_DMA_RX_PC_8			2
+#define   BGMAC_DMA_RX_PC_16			3
+#define  BGMAC_DMA_RX_PT_MASK			0x03000000	/* Prefetch threshold */
+#define  BGMAC_DMA_RX_PT_SHIFT			24
+#define   BGMAC_DMA_RX_PT_1			0
+#define   BGMAC_DMA_RX_PT_2			1
+#define   BGMAC_DMA_RX_PT_4			2
+#define   BGMAC_DMA_RX_PT_8			3
 #define BGMAC_DMA_RX_INDEX			0x24
 #define BGMAC_DMA_RX_RINGLO			0x28
 #define BGMAC_DMA_RX_RINGHI			0x2C
-- 
1.7.10.4

[PATCH 3/5] bgmac: add support for new BGMAC_CMDCFG_SR position on core rev >= 4

[Hauke Mehrtens]

The BGMAC_CMDCFG_SR register is at a different position on core rev >= 4

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |    8 ++++----
 drivers/net/ethernet/broadcom/bgmac.h |    4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index aa5e289..88ec69e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -768,13 +768,13 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 mask, u32 set,
 	u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
 	u32 new_val = (cmdcfg & mask) | set;
 
-	bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR);
+	bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR(bgmac->core->id.rev));
 	udelay(2);
 
 	if (new_val != cmdcfg || force)
 		bgmac_write(bgmac, BGMAC_CMDCFG, new_val);
 
-	bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR);
+	bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR(bgmac->core->id.rev));
 	udelay(2);
 }
 
@@ -977,7 +977,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			     BGMAC_CMDCFG_PROM |
 			     BGMAC_CMDCFG_NLC |
 			     BGMAC_CMDCFG_CFE |
-			     BGMAC_CMDCFG_SR,
+			     BGMAC_CMDCFG_SR(core->id.rev),
 			     false);
 	bgmac->mac_speed = SPEED_UNKNOWN;
 	bgmac->mac_duplex = DUPLEX_UNKNOWN;
@@ -1020,7 +1020,7 @@ static void bgmac_enable(struct bgmac *bgmac)
 
 	cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
 	bgmac_cmdcfg_maskset(bgmac, ~(BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE),
-			     BGMAC_CMDCFG_SR, true);
+			     BGMAC_CMDCFG_SR(bgmac->core->id.rev), true);
 	udelay(2);
 	cmdcfg |= BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE;
 	bgmac_write(bgmac, BGMAC_CMDCFG, cmdcfg);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 71adac9..ceeaac6 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -197,7 +197,9 @@
 #define  BGMAC_CMDCFG_TAI			0x00000200
 #define  BGMAC_CMDCFG_HD			0x00000400	/* Set if in half duplex mode */
 #define  BGMAC_CMDCFG_HD_SHIFT			10
-#define  BGMAC_CMDCFG_SR			0x00000800	/* Set to reset mode */
+#define  BGMAC_CMDCFG_SR_REVO			0x00000800	/* Set to reset mode, for other revs */
+#define  BGMAC_CMDCFG_SR_REV4			0x00002000	/* Set to reset mode, only for core rev 4 */
+#define  BGMAC_CMDCFG_SR(rev)  ((rev >= 4) ? BGMAC_CMDCFG_SR_REV4 : BGMAC_CMDCFG_SR_REVO)
 #define  BGMAC_CMDCFG_ML			0x00008000	/* Set to activate mac loopback mode */
 #define  BGMAC_CMDCFG_AE			0x00400000
 #define  BGMAC_CMDCFG_CFE			0x00800000
-- 
1.7.10.4

[PATCH 4/5] bgmac: reset all cores on Northstar SoC

[Hauke Mehrtens]

On the Northstar SoC (BCM4707 and BCM53018) we have to enable all GMAC
cores when we just want to use on. We iterate over all the cores and
activate them.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 88ec69e..2cdacb8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1446,6 +1446,27 @@ static int bgmac_probe(struct bcma_device *core)
 
 	bgmac_chip_reset(bgmac);
 
+	/* For Northstar, we have to take all GMAC core out of reset */
+	if (core->id.id == BCMA_CHIP_ID_BCM4707 ||
+	    core->id.id == BCMA_CHIP_ID_BCM53018) {
+		struct bcma_device *ns_core;
+		int ns_gmac;
+
+		/* Northstar has 4 GMAC cores */
+		for (ns_gmac = 0; ns_gmac < 4; ns_gmac++) {
+			/* As northstar requirement, we have to reset all GAMCs
+			 * before accessing one. bgmac_chip_reset() call
+			 * bcma_core_enable() for this core. Then the other
+			 * three GAMCs didn't reset.  We do it here.
+			 */
+			ns_core = bcma_find_core_unit(core->bus,
+						      BCMA_CORE_MAC_GBIT,
+						      ns_gmac);
+			if (ns_core && !bcma_core_is_enabled(ns_core))
+				bcma_core_enable(ns_core, 0);
+		}
+	}
+
 	err = bgmac_dma_alloc(bgmac);
 	if (err) {
 		bgmac_err(bgmac, "Unable to alloc memory for DMA\n");
-- 
1.7.10.4

[PATCH 5/5] bgmac: add support for Northstar SoC (BCM4707, BCM53018)

[Hauke Mehrtens]

This adds support for the Northstar SoC. This SoC does not have a PMU in
bcma and no register on it should be called. In addition it support 2.5
GBit/s Ethernet to the PHY.

This GMAC core is not fully working there are still problems with the
DMA controller.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |   46 +++++++++++++++++++++++----------
 drivers/net/ethernet/broadcom/bgmac.h |    1 +
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 2cdacb8..cf7e1ff 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -848,6 +848,9 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 	case SPEED_1000:
 		set |= BGMAC_CMDCFG_ES_1000;
 		break;
+	case SPEED_2500:
+		set |= BGMAC_CMDCFG_ES_2500;
+		break;
 	default:
 		bgmac_err(bgmac, "Unsupported speed: %d\n", bgmac->mac_speed);
 	}
@@ -860,12 +863,24 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
-	u8 imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >>
-			BGMAC_DS_MM_SHIFT;
-	if (imode == 0 || imode == 1) {
-		bgmac->mac_speed = SPEED_100;
-		bgmac->mac_duplex = DUPLEX_FULL;
-		bgmac_mac_speed(bgmac);
+	struct bcma_device *core = bgmac->core;
+	struct bcma_chipinfo *ci = &core->bus->chipinfo;
+
+	if (ci->id == BCMA_CHIP_ID_BCM4707 ||
+	    ci->id == BCMA_CHIP_ID_BCM53018) {
+		bcma_awrite32(core, BCMA_IOCTL,
+			      bcma_aread32(core, BCMA_IOCTL) | 0x44);
+			bgmac->mac_speed = SPEED_2500;
+			bgmac->mac_duplex = DUPLEX_FULL;
+			bgmac_mac_speed(bgmac);
+	} else {
+		u8 imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) &
+			    BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT;
+		if (imode == 0 || imode == 1) {
+			bgmac->mac_speed = SPEED_100;
+			bgmac->mac_duplex = DUPLEX_FULL;
+			bgmac_mac_speed(bgmac);
+		}
 	}
 }
 
@@ -911,7 +926,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 
 	bcma_core_enable(core, flags);
 
-	if (core->id.rev > 2) {
+	if (core->id.rev > 2 && ci->id != BCMA_CHIP_ID_BCM4707 &&
+	    ci->id != BCMA_CHIP_ID_BCM53018) {
 		bgmac_set(bgmac, BCMA_CLKCTLST,
 			  BGMAC_BCMA_CLKCTLST_MISC_PLL_REQ);
 		bgmac_wait_value(bgmac->core, BCMA_CLKCTLST,
@@ -1049,12 +1065,16 @@ static void bgmac_enable(struct bgmac *bgmac)
 		break;
 	}
 
-	rxq_ctl = bgmac_read(bgmac, BGMAC_RXQ_CTL);
-	rxq_ctl &= ~BGMAC_RXQ_CTL_MDP_MASK;
-	bp_clk = bcma_pmu_get_bus_clock(&bgmac->core->bus->drv_cc) / 1000000;
-	mdp = (bp_clk * 128 / 1000) - 3;
-	rxq_ctl |= (mdp << BGMAC_RXQ_CTL_MDP_SHIFT);
-	bgmac_write(bgmac, BGMAC_RXQ_CTL, rxq_ctl);
+	if (ci->id != BCMA_CHIP_ID_BCM4707 &&
+	    ci->id != BCMA_CHIP_ID_BCM53018) {
+		rxq_ctl = bgmac_read(bgmac, BGMAC_RXQ_CTL);
+		rxq_ctl &= ~BGMAC_RXQ_CTL_MDP_MASK;
+		bp_clk = bcma_pmu_get_bus_clock(&bgmac->core->bus->drv_cc) /
+				1000000;
+		mdp = (bp_clk * 128 / 1000) - 3;
+		rxq_ctl |= (mdp << BGMAC_RXQ_CTL_MDP_SHIFT);
+		bgmac_write(bgmac, BGMAC_RXQ_CTL, rxq_ctl);
+	}
 }
 
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index ceeaac6..8f71056 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -189,6 +189,7 @@
 #define   BGMAC_CMDCFG_ES_10			0x00000000
 #define   BGMAC_CMDCFG_ES_100			0x00000004
 #define   BGMAC_CMDCFG_ES_1000			0x00000008
+#define   BGMAC_CMDCFG_ES_2500			0x0000000C
 #define  BGMAC_CMDCFG_PROM			0x00000010	/* Set to activate promiscuous mode */
 #define  BGMAC_CMDCFG_PAD_EN			0x00000020
 #define  BGMAC_CMDCFG_CF			0x00000040
-- 
1.7.10.4

Re: [PATCH 3/5] bgmac: add support for new BGMAC_CMDCFG_SR position on core rev >= 4

[Rafał Miłecki]

2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
> The BGMAC_CMDCFG_SR register is at a different position on core rev >= 4
> -#define  BGMAC_CMDCFG_SR                       0x00000800      /* Set to reset mode */
> +#define  BGMAC_CMDCFG_SR_REVO                  0x00000800      /* Set to reset mode, for other revs */
> +#define  BGMAC_CMDCFG_SR_REV4                  0x00002000      /* Set to reset mode, only for core rev 4 */
> +#define  BGMAC_CMDCFG_SR(rev)  ((rev >= 4) ? BGMAC_CMDCFG_SR_REV4 : BGMAC_CMDCFG_SR_REVO)

If you use _REV4 for rev 4+, then what about using _REV0 for rev 0+?

I guess REVO stands for OTHERS, but others doesn't actually mean rev != 4 here.

Unless it was supposed to be 0 from the beginning and it's just a typo?

In future you may need to use _REV7 or whatever number.

-- 
Rafał

Re: [PATCH 5/5] bgmac: add support for Northstar SoC (BCM4707, BCM53018)

[Rafał Miłecki]

 > +       if (ci->id == BCMA_CHIP_ID_BCM4707 ||
> +           ci->id == BCMA_CHIP_ID_BCM53018) {

Please add a missing
if (bgmac->phyaddr != BGMAC_PHY_NOREGS)
return;
at the beginning of this 4707/53018 block


> +               bcma_awrite32(core, BCMA_IOCTL,
> +                             bcma_aread32(core, BCMA_IOCTL) | 0x44);

Please use
BGMAC_BCMA_IOCTL_SW_CLKEN | 0x40
(unless you know 0x40, then replace it too).


> +                       bgmac->mac_speed = SPEED_2500;
> +                       bgmac->mac_duplex = DUPLEX_FULL;
> +                       bgmac_mac_speed(bgmac);

Wrong indent above.


> @@ -911,7 +926,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
>
>         bcma_core_enable(core, flags);

Look closer. Don't calculate flags and don't call bcma_core_enable for
4707 (but call it for 53018).


> -       if (core->id.rev > 2) {
> +       if (core->id.rev > 2 && ci->id != BCMA_CHIP_ID_BCM4707 &&
> +           ci->id != BCMA_CHIP_ID_BCM53018) {

Could you put 4707 check on the separated line? I'm not pushing however.

-- 
Rafał

Re: [PATCH 5/5] bgmac: add support for Northstar SoC (BCM4707, BCM53018)

[Hauke Mehrtens]

On 01/02/2014 09:40 PM, Rafał Miłecki wrote:
>  > +       if (ci->id == BCMA_CHIP_ID_BCM4707 ||
>> +           ci->id == BCMA_CHIP_ID_BCM53018) {
> 
> Please add a missing
> if (bgmac->phyaddr != BGMAC_PHY_NOREGS)
> return;
> at the beginning of this 4707/53018 block

Why should I add that there?

> 
>> +               bcma_awrite32(core, BCMA_IOCTL,
>> +                             bcma_aread32(core, BCMA_IOCTL) | 0x44);
> 
> Please use
> BGMAC_BCMA_IOCTL_SW_CLKEN | 0x40
> (unless you know 0x40, then replace it too).

The Documentation says this is:
* Bit 2 : TX_CLK_OUT_INVERT_EN - If set, this will invert the TX clock
out of AMAC.
And the other is:
* Bit 12:8  "interface_mode"  This field is programmed through IDM
control bits [6:2]

see this:
https://github.com/RMerl/asuswrt-merlin/blob/master/release/src-rt-6.x.4708/et/sys/etcgmac.c#L1039

> 
> 
>> +                       bgmac->mac_speed = SPEED_2500;
>> +                       bgmac->mac_duplex = DUPLEX_FULL;
>> +                       bgmac_mac_speed(bgmac);
> 
> Wrong indent above.

fixed

> 
>> @@ -911,7 +926,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
>>
>>         bcma_core_enable(core, flags);
> 
> Look closer. Don't calculate flags and don't call bcma_core_enable for
> 4707 (but call it for 53018).

I haven't seen that condition, strange.
> 
> 
>> -       if (core->id.rev > 2) {
>> +       if (core->id.rev > 2 && ci->id != BCMA_CHIP_ID_BCM4707 &&
>> +           ci->id != BCMA_CHIP_ID_BCM53018) {
> 
> Could you put 4707 check on the separated line? I'm not pushing however.

changed

Hauke

Re: [PATCH 3/5] bgmac: add support for new BGMAC_CMDCFG_SR position on core rev >= 4

[Hauke Mehrtens]

On 01/02/2014 09:28 PM, Rafał Miłecki wrote:
> 2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
>> The BGMAC_CMDCFG_SR register is at a different position on core rev >= 4
>> -#define  BGMAC_CMDCFG_SR                       0x00000800      /* Set to reset mode */
>> +#define  BGMAC_CMDCFG_SR_REVO                  0x00000800      /* Set to reset mode, for other revs */
>> +#define  BGMAC_CMDCFG_SR_REV4                  0x00002000      /* Set to reset mode, only for core rev 4 */
>> +#define  BGMAC_CMDCFG_SR(rev)  ((rev >= 4) ? BGMAC_CMDCFG_SR_REV4 : BGMAC_CMDCFG_SR_REVO)
> 
> If you use _REV4 for rev 4+, then what about using _REV0 for rev 0+?
> 
> I guess REVO stands for OTHERS, but others doesn't actually mean rev != 4 here.
> 
> Unless it was supposed to be 0 from the beginning and it's just a typo?
> 
> In future you may need to use _REV7 or whatever number.
> 
This should be REV0, 0 and O are just so similar.

I will also change this from rev >= 4 to rev == 4, as the Broadcom
driver does the same.

Hauke

Re: [PATCH 3/5] bgmac: add support for new BGMAC_CMDCFG_SR position on core rev >= 4

[Rafał Miłecki]

2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
> On 01/02/2014 09:28 PM, Rafał Miłecki wrote:
>> 2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
>>> The BGMAC_CMDCFG_SR register is at a different position on core rev >= 4
>>> -#define  BGMAC_CMDCFG_SR                       0x00000800      /* Set to reset mode */
>>> +#define  BGMAC_CMDCFG_SR_REVO                  0x00000800      /* Set to reset mode, for other revs */
>>> +#define  BGMAC_CMDCFG_SR_REV4                  0x00002000      /* Set to reset mode, only for core rev 4 */
>>> +#define  BGMAC_CMDCFG_SR(rev)  ((rev >= 4) ? BGMAC_CMDCFG_SR_REV4 : BGMAC_CMDCFG_SR_REVO)
>>
>> If you use _REV4 for rev 4+, then what about using _REV0 for rev 0+?
>>
>> I guess REVO stands for OTHERS, but others doesn't actually mean rev != 4 here.
>>
>> Unless it was supposed to be 0 from the beginning and it's just a typo?
>>
>> In future you may need to use _REV7 or whatever number.
>>
> This should be REV0, 0 and O are just so similar.
>
> I will also change this from rev >= 4 to rev == 4, as the Broadcom
> driver does the same.

Ahh, if this is only for rev == 4, then maybe BGMAC_CMDCFG_SR or
BGMAC_CMDCFG_SR_REVO aren't bad after all.

It's up to you in this situation :)

-- 
Rafał

Re: [PATCH 5/5] bgmac: add support for Northstar SoC (BCM4707, BCM53018)

[Rafał Miłecki]

2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
> On 01/02/2014 09:40 PM, Rafał Miłecki wrote:
>>  > +       if (ci->id == BCMA_CHIP_ID_BCM4707 ||
>>> +           ci->id == BCMA_CHIP_ID_BCM53018) {
>>
>> Please add a missing
>> if (bgmac->phyaddr != BGMAC_PHY_NOREGS)
>> return;
>> at the beginning of this 4707/53018 block
>
> Why should I add that there?

This is what I can see in etcgmac.c version:
* $Id: etcgmac.c 414031 2013-07-23 10:54:51Z $


>>> +               bcma_awrite32(core, BCMA_IOCTL,
>>> +                             bcma_aread32(core, BCMA_IOCTL) | 0x44);
>>
>> Please use
>> BGMAC_BCMA_IOCTL_SW_CLKEN | 0x40
>> (unless you know 0x40, then replace it too).
>
> The Documentation says this is:
> * Bit 2 : TX_CLK_OUT_INVERT_EN - If set, this will invert the TX clock
> out of AMAC.
> And the other is:
> * Bit 12:8  "interface_mode"  This field is programmed through IDM
> control bits [6:2]
>
> see this:
> https://github.com/RMerl/asuswrt-merlin/blob/master/release/src-rt-6.x.4708/et/sys/etcgmac.c#L1039

Documentation is about "BCM4707 GMAC DevStatus register" which is:
#define BGMAC_DEV_CTL
That explains why you can't execute:
devstatus = R_REG(ch->osh, &regs->devstatus);
mode = ((devstatus & DS_MM_MASK) >> DS_MM_SHIFT);
code/path on BCM4707 family.

si_core_cflags -> ai_core_cflags operates on ai->ioctrl register (and
so does bcma_awrite32), so these documented bits don't apply to it.

-- 
Rafał

Re: [PATCH 5/5] bgmac: add support for Northstar SoC (BCM4707, BCM53018)

[Rafał Miłecki]

2014/1/3 Rafał Miłecki <zajec5@gmail.com>:
> 2014/1/2 Hauke Mehrtens <hauke@hauke-m.de>:
>> On 01/02/2014 09:40 PM, Rafał Miłecki wrote:
>>>  > +       if (ci->id == BCMA_CHIP_ID_BCM4707 ||
>>>> +           ci->id == BCMA_CHIP_ID_BCM53018) {
>>>
>>> Please add a missing
>>> if (bgmac->phyaddr != BGMAC_PHY_NOREGS)
>>> return;
>>> at the beginning of this 4707/53018 block
>>
>> Why should I add that there?
>
> This is what I can see in etcgmac.c version:
> * $Id: etcgmac.c 414031 2013-07-23 10:54:51Z $

Hm, this is weird. I think this
* $Id: etcgmac.c 414031 2013-07-23 10:54:51Z $
is not any version, but just a timestamp of executing some cleaning-script.

Anyway, your reference code is 6.37.14.62:
https://github.com/RMerl/asuswrt-merlin/blob/master/release/src-rt-6.x.4708/include/epivers.h

And mine is 6.37.14.34 (GPL_RT_AC68U_30004374205.zip).

So it seems your is newer one...

OK, leave this code like it is, ignore that my comment for now.