netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] tg3: add support for Ethernet core in BCM4785
@ 2013-01-12 20:01 Hauke Mehrtens
  2013-01-12 20:01 ` [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr Hauke Mehrtens
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-12 20:01 UTC (permalink / raw)
  To: davem; +Cc: mcarlson, mchan, netdev, m, Hauke Mehrtens

These patches are adding support for the Ethernet core found in the BCM4705/BCM4785 SoC.

Hauke Mehrtens (3):
  ssb: add missing method ssb_gige_get_macaddr
  tg3: make it possible to provide phy_id in ioctl
  tg3: add support for Ethernet core in bcm4785

 drivers/net/ethernet/broadcom/tg3.c |  137 +++++++++++++++++++++++++++++++----
 drivers/net/ethernet/broadcom/tg3.h |    5 ++
 include/linux/pci_ids.h             |    1 +
 include/linux/ssb/ssb_driver_gige.h |    3 +
 4 files changed, 133 insertions(+), 13 deletions(-)

-- 
1.7.10.4

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

* [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr
  2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
@ 2013-01-12 20:01 ` Hauke Mehrtens
  2013-01-12 23:32   ` David Miller
  2013-01-12 20:01 ` [PATCH 2/3] tg3: make it possible to provide phy_id in ioctl Hauke Mehrtens
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-12 20:01 UTC (permalink / raw)
  To: davem; +Cc: mcarlson, mchan, netdev, m, Hauke Mehrtens

When CONFIG_SSB_DRIVER_GIGE is not set the header does not provide the
needed method.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 include/linux/ssb/ssb_driver_gige.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/ssb/ssb_driver_gige.h b/include/linux/ssb/ssb_driver_gige.h
index 6b05dcd..1438523 100644
--- a/include/linux/ssb/ssb_driver_gige.h
+++ b/include/linux/ssb/ssb_driver_gige.h
@@ -175,6 +175,9 @@ static inline bool ssb_gige_must_flush_posted_writes(struct pci_dev *pdev)
 {
 	return 0;
 }
+static inline void ssb_gige_get_macaddr(struct pci_dev *pdev, u8 *macaddr)
+{
+}
 
 #endif /* CONFIG_SSB_DRIVER_GIGE */
 #endif /* LINUX_SSB_DRIVER_GIGE_H_ */
-- 
1.7.10.4

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

* [PATCH 2/3] tg3: make it possible to provide phy_id in ioctl
  2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
  2013-01-12 20:01 ` [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr Hauke Mehrtens
@ 2013-01-12 20:01 ` Hauke Mehrtens
  2013-01-12 20:01 ` [PATCH 3/3] tg3: add support for Ethernet core in bcm4785 Hauke Mehrtens
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-12 20:01 UTC (permalink / raw)
  To: davem; +Cc: mcarlson, mchan, netdev, m, Hauke Mehrtens

In OpenWrt we currently use a switch driver which uses the ioctls to
configure the switch in the phy. We have to provide the phy_id to do
so, but without this patch this is not possible.

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

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 88f2d41..6c397d7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -1091,7 +1091,8 @@ static void tg3_switch_clocks(struct tg3 *tp)
 
 #define PHY_BUSY_LOOPS	5000
 
-static int tg3_readphy(struct tg3 *tp, int reg, u32 *val)
+static int __tg3_readphy(struct tg3 *tp, unsigned int phy_addr, int reg,
+			 u32 *val)
 {
 	u32 frame_val;
 	unsigned int loops;
@@ -1107,7 +1108,7 @@ static int tg3_readphy(struct tg3 *tp, int reg, u32 *val)
 
 	*val = 0x0;
 
-	frame_val  = ((tp->phy_addr << MI_COM_PHY_ADDR_SHIFT) &
+	frame_val  = ((phy_addr << MI_COM_PHY_ADDR_SHIFT) &
 		      MI_COM_PHY_ADDR_MASK);
 	frame_val |= ((reg << MI_COM_REG_ADDR_SHIFT) &
 		      MI_COM_REG_ADDR_MASK);
@@ -1144,7 +1145,13 @@ static int tg3_readphy(struct tg3 *tp, int reg, u32 *val)
 	return ret;
 }
 
-static int tg3_writephy(struct tg3 *tp, int reg, u32 val)
+static int tg3_readphy(struct tg3 *tp, int reg, u32 *val)
+{
+	return __tg3_readphy(tp, tp->phy_addr, reg, val);
+}
+
+static int __tg3_writephy(struct tg3 *tp, unsigned int phy_addr, int reg,
+			  u32 val)
 {
 	u32 frame_val;
 	unsigned int loops;
@@ -1162,7 +1169,7 @@ static int tg3_writephy(struct tg3 *tp, int reg, u32 val)
 
 	tg3_ape_lock(tp, tp->phy_ape_lock);
 
-	frame_val  = ((tp->phy_addr << MI_COM_PHY_ADDR_SHIFT) &
+	frame_val  = ((phy_addr << MI_COM_PHY_ADDR_SHIFT) &
 		      MI_COM_PHY_ADDR_MASK);
 	frame_val |= ((reg << MI_COM_REG_ADDR_SHIFT) &
 		      MI_COM_REG_ADDR_MASK);
@@ -1197,6 +1204,11 @@ static int tg3_writephy(struct tg3 *tp, int reg, u32 val)
 	return ret;
 }
 
+static int tg3_writephy(struct tg3 *tp, int reg, u32 val)
+{
+	return __tg3_writephy(tp, tp->phy_addr, reg, val);
+}
+
 static int tg3_phy_cl45_write(struct tg3 *tp, u32 devad, u32 addr, u32 val)
 {
 	int err;
@@ -12956,7 +12968,8 @@ static int tg3_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 			return -EAGAIN;
 
 		spin_lock_bh(&tp->lock);
-		err = tg3_readphy(tp, data->reg_num & 0x1f, &mii_regval);
+		err = __tg3_readphy(tp, data->phy_id & 0x1f,
+				    data->reg_num & 0x1f, &mii_regval);
 		spin_unlock_bh(&tp->lock);
 
 		data->val_out = mii_regval;
@@ -12972,7 +12985,8 @@ static int tg3_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 			return -EAGAIN;
 
 		spin_lock_bh(&tp->lock);
-		err = tg3_writephy(tp, data->reg_num & 0x1f, data->val_in);
+		err = __tg3_writephy(tp, data->phy_id & 0x1f,
+				     data->reg_num & 0x1f, data->val_in);
 		spin_unlock_bh(&tp->lock);
 
 		return err;
-- 
1.7.10.4

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

* [PATCH 3/3] tg3: add support for Ethernet core in bcm4785
  2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
  2013-01-12 20:01 ` [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr Hauke Mehrtens
  2013-01-12 20:01 ` [PATCH 2/3] tg3: make it possible to provide phy_id in ioctl Hauke Mehrtens
@ 2013-01-12 20:01 ` Hauke Mehrtens
  2013-01-12 23:32 ` [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 David Miller
  2013-01-24 17:21 ` Hauke Mehrtens
  4 siblings, 0 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-12 20:01 UTC (permalink / raw)
  To: davem; +Cc: mcarlson, mchan, netdev, m, Hauke Mehrtens

The BCM4785 or sometimes named BMC4705 is a Broadcom SoC which a
Gigabit 5750 Ethernet core. The core is connected via PCI with the rest
of the SoC, but it uses some extension.

This core does not use a firmware or an eeprom.

Some devices only have a switch which supports 100MBit/s, this
currently does not work with this driver.

This patch was original written by Michael Buesch <m@bues.ch> and is in
OpenWrt for some years now.

This was tested on a Linksys WRT610N V1 and older version by other
people on different devices.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/tg3.c |  111 ++++++++++++++++++++++++++++++++---
 drivers/net/ethernet/broadcom/tg3.h |    5 ++
 include/linux/pci_ids.h             |    1 +
 3 files changed, 110 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 6c397d7..788f0e3 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -44,6 +44,7 @@
 #include <linux/prefetch.h>
 #include <linux/dma-mapping.h>
 #include <linux/firmware.h>
+#include <linux/ssb/ssb_driver_gige.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
 
@@ -263,6 +264,7 @@ static DEFINE_PCI_DEVICE_TABLE(tg3_pci_tbl) = {
 			TG3_DRV_DATA_FLAG_5705_10_100},
 	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5721)},
 	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5722)},
+	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5750)},
 	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5751)},
 	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5751M)},
 	{PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5751F),
@@ -573,7 +575,9 @@ static void _tw32_flush(struct tg3 *tp, u32 off, u32 val, u32 usec_wait)
 static inline void tw32_mailbox_flush(struct tg3 *tp, u32 off, u32 val)
 {
 	tp->write32_mbox(tp, off, val);
-	if (!tg3_flag(tp, MBOX_WRITE_REORDER) && !tg3_flag(tp, ICH_WORKAROUND))
+	if (tg3_flag(tp, FLUSH_POSTED_WRITES) ||
+	    (!tg3_flag(tp, MBOX_WRITE_REORDER) &&
+	     !tg3_flag(tp, ICH_WORKAROUND)))
 		tp->read32_mbox(tp, off);
 }
 
@@ -583,7 +587,8 @@ static void tg3_write32_tx_mbox(struct tg3 *tp, u32 off, u32 val)
 	writel(val, mbox);
 	if (tg3_flag(tp, TXD_MBOX_HWBUG))
 		writel(val, mbox);
-	if (tg3_flag(tp, MBOX_WRITE_REORDER))
+	if (tg3_flag(tp, MBOX_WRITE_REORDER) ||
+	    tg3_flag(tp, FLUSH_POSTED_WRITES))
 		readl(mbox);
 }
 
@@ -1781,6 +1786,11 @@ static int tg3_poll_fw(struct tg3 *tp)
 	int i;
 	u32 val;
 
+	if (tg3_flag(tp, IS_SSB_CORE)) {
+		/* We don't use firmware. */
+		return 0;
+	}
+
 	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
 		/* Wait up to 20ms for init done. */
 		for (i = 0; i < 200; i++) {
@@ -3453,6 +3463,11 @@ static int tg3_halt_cpu(struct tg3 *tp, u32 offset)
 		tw32_f(offset + CPU_MODE,  CPU_MODE_HALT);
 		udelay(10);
 	} else {
+		/* There is only an Rx CPU for the 5750 derivative in the
+		 * BCM4785. */
+		if (tg3_flag(tp, IS_SSB_CORE))
+			return 0;
+
 		for (i = 0; i < 10000; i++) {
 			tw32(offset + CPU_STATE, 0xffffffff);
 			tw32(offset + CPU_MODE,  CPU_MODE_HALT);
@@ -3920,8 +3935,9 @@ static int tg3_power_down_prepare(struct tg3 *tp)
 	tg3_frob_aux_power(tp, true);
 
 	/* Workaround for unstable PLL clock */
-	if ((GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5750_AX) ||
-	    (GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5750_BX)) {
+	if ((!tg3_flag(tp, IS_SSB_CORE)) &&
+	    ((GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5750_AX) ||
+	     (GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5750_BX))) {
 		u32 val = tr32(0x7d00);
 
 		val &= ~((1 << 16) | (1 << 4) | (1 << 2) | (1 << 1) | 1);
@@ -4442,6 +4458,15 @@ relink:
 	if (current_link_up == 0 || (tp->phy_flags & TG3_PHYFLG_IS_LOW_POWER)) {
 		tg3_phy_copper_begin(tp);
 
+		if (tg3_flag(tp, ROBOSWITCH)) {
+			current_link_up = 1;
+			/* FIXME: when BCM5325 switch is used use 100 MBit/s */
+			current_speed = SPEED_1000;
+			current_duplex = DUPLEX_FULL;
+			tp->link_config.active_speed = current_speed;
+			tp->link_config.active_duplex = current_duplex;
+		}
+
 		tg3_readphy(tp, MII_BMSR, &bmsr);
 		if ((!tg3_readphy(tp, MII_BMSR, &bmsr) && (bmsr & BMSR_LSTATUS)) ||
 		    (tp->mac_mode & MAC_MODE_PORT_INT_LPBACK))
@@ -4460,6 +4485,26 @@ relink:
 	else
 		tp->mac_mode |= MAC_MODE_PORT_MODE_GMII;
 
+	/* In order for the 5750 core in BCM4785 chip to work properly
+	 * in RGMII mode, the Led Control Register must be set up.
+	 */
+	if (tg3_flag(tp, RGMII_MODE)) {
+		u32 led_ctrl = tr32(MAC_LED_CTRL);
+		led_ctrl &= ~(LED_CTRL_1000MBPS_ON | LED_CTRL_100MBPS_ON);
+
+		if (tp->link_config.active_speed == SPEED_10)
+			led_ctrl |= LED_CTRL_LNKLED_OVERRIDE;
+		else if (tp->link_config.active_speed == SPEED_100)
+			led_ctrl |= (LED_CTRL_LNKLED_OVERRIDE |
+				     LED_CTRL_100MBPS_ON);
+		else if (tp->link_config.active_speed == SPEED_1000)
+			led_ctrl |= (LED_CTRL_LNKLED_OVERRIDE |
+				     LED_CTRL_1000MBPS_ON);
+
+		tw32(MAC_LED_CTRL, led_ctrl);
+		udelay(40);
+	}
+
 	tp->mac_mode &= ~MAC_MODE_HALF_DUPLEX;
 	if (tp->link_config.active_duplex == DUPLEX_HALF)
 		tp->mac_mode |= MAC_MODE_HALF_DUPLEX;
@@ -8436,6 +8481,14 @@ static int tg3_chip_reset(struct tg3 *tp)
 		tw32(0x5000, 0x400);
 	}
 
+	if (tg3_flag(tp, IS_SSB_CORE)) {
+		/* BCM4785: In order to avoid repercussions from using
+		 * potentially defective internal ROM, stop the Rx RISC CPU,
+		 * which is not required. */
+		tg3_stop_fw(tp);
+		tg3_halt_cpu(tp, RX_CPU_BASE);
+	}
+
 	tw32(GRC_MODE, tp->grc_mode);
 
 	if (tp->pci_chip_rev_id == CHIPREV_ID_5705_A0) {
@@ -10094,6 +10147,11 @@ static void tg3_timer(unsigned long __opaque)
 	    tg3_flag(tp, 57765_CLASS))
 		tg3_chk_missed_msi(tp);
 
+	if (tg3_flag(tp, FLUSH_POSTED_WRITES)) {
+		/* BCM4785: Flush posted writes from GbE to host memory. */
+		tr32(HOSTCC_MODE);
+	}
+
 	if (!tg3_flag(tp, TAGGED_STATUS)) {
 		/* All of this garbage is because when using non-tagged
 		 * IRQ status the mailbox/status_block protocol the chip
@@ -13866,6 +13924,14 @@ static void tg3_get_5720_nvram_info(struct tg3 *tp)
 /* Chips other than 5700/5701 use the NVRAM for fetching info. */
 static void tg3_nvram_init(struct tg3 *tp)
 {
+	if (tg3_flag(tp, IS_SSB_CORE)) {
+		/* No NVRAM and EEPROM on the SSB Broadcom GigE core. */
+		tg3_flag_clear(tp, NVRAM);
+		tg3_flag_clear(tp, NVRAM_BUFFERED);
+		tg3_flag_set(tp, NO_NVRAM);
+		return;
+	}
+
 	tw32_f(GRC_EEPROM_ADDR,
 	     (EEPROM_ADDR_FSM_RESET |
 	      (EEPROM_DEFAULT_CLOCK_PERIOD <<
@@ -14392,10 +14458,19 @@ static int tg3_phy_probe(struct tg3 *tp)
 			 * subsys device table.
 			 */
 			p = tg3_lookup_by_subsys(tp);
-			if (!p)
+			if (p) {
+				tp->phy_id = p->phy_id;
+			} else if (!tg3_flag(tp, IS_SSB_CORE)) {
+				/* For now we saw the IDs 0xbc050cd0,
+				 * 0xbc050f80 and 0xbc050c30 on devices
+				 * connected to an BCM4785 and there are
+				 * probably more. Just assume that the phy is
+				 * supported when it is connected to a SSB core
+				 * for now.
+				 */
 				return -ENODEV;
+			}
 
-			tp->phy_id = p->phy_id;
 			if (!tp->phy_id ||
 			    tp->phy_id == TG3_PHY_ID_BCM8002)
 				tp->phy_flags |= TG3_PHYFLG_PHY_SERDES;
@@ -15471,6 +15546,11 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 				     TG3_CPMU_STATUS_FSHFT_5719;
 	}
 
+	if (tg3_flag(tp, FLUSH_POSTED_WRITES)) {
+		tp->write32_tx_mbox = tg3_write_flush_reg32;
+		tp->write32_rx_mbox = tg3_write_flush_reg32;
+	}
+
 	/* Get eeprom hw config before calling tg3_set_power_state().
 	 * In particular, the TG3_FLAG_IS_NIC flag must be
 	 * determined before calling tg3_set_power_state() so that
@@ -15868,6 +15948,10 @@ static int tg3_get_device_address(struct tg3 *tp)
 	}
 
 	if (!is_valid_ether_addr(&dev->dev_addr[0])) {
+		if (tg3_flag(tp, IS_SSB_CORE))
+			ssb_gige_get_macaddr(tp->pdev, &dev->dev_addr[0]);
+	}
+	if (!is_valid_ether_addr(&dev->dev_addr[0])) {
 #ifdef CONFIG_SPARC
 		if (!tg3_get_default_macaddr_sparc(tp))
 			return 0;
@@ -16152,7 +16236,8 @@ static int tg3_test_dma(struct tg3 *tp)
 			if (tg3_flag(tp, 40BIT_DMA_BUG) &&
 			    GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5704)
 				tp->dma_rwctrl |= 0x8000;
-			else if (ccval == 0x6 || ccval == 0x7)
+			else if ((ccval == 0x6 || ccval == 0x7) ||
+				 tg3_flag(tp, ONE_DMA_AT_ONCE))
 				tp->dma_rwctrl |= DMA_RWCTRL_ONE_DMA;
 
 			if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5703)
@@ -16516,6 +16601,18 @@ static int tg3_init_one(struct pci_dev *pdev,
 	else
 		tp->msg_enable = TG3_DEF_MSG_ENABLE;
 
+	if (pdev_is_ssb_gige_core(pdev)) {
+		tg3_flag_set(tp, IS_SSB_CORE);
+		if (ssb_gige_must_flush_posted_writes(pdev))
+			tg3_flag_set(tp, FLUSH_POSTED_WRITES);
+		if (ssb_gige_one_dma_at_once(pdev))
+			tg3_flag_set(tp, ONE_DMA_AT_ONCE);
+		if (ssb_gige_have_roboswitch(pdev))
+			tg3_flag_set(tp, ROBOSWITCH);
+		if (ssb_gige_is_rgmii(pdev))
+			tg3_flag_set(tp, RGMII_MODE);
+	}
+
 	/* The word/byte swap controls here control register access byte
 	 * swapping.  DMA data byte swapping is controlled in the GRC_MODE
 	 * setting below.
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 9cd88a4..ef6ced2 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3056,6 +3056,11 @@ enum TG3_FLAGS {
 	TG3_FLAG_57765_PLUS,
 	TG3_FLAG_57765_CLASS,
 	TG3_FLAG_5717_PLUS,
+	TG3_FLAG_IS_SSB_CORE,
+	TG3_FLAG_FLUSH_POSTED_WRITES,
+	TG3_FLAG_ROBOSWITCH,
+	TG3_FLAG_ONE_DMA_AT_ONCE,
+	TG3_FLAG_RGMII_MODE,
 
 	/* Add new flags before this comment and TG3_FLAG_NUMBER_OF_FLAGS */
 	TG3_FLAG_NUMBER_OF_FLAGS,	/* Last entry in enum TG3_FLAGS */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 0f84473..badb429 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2126,6 +2126,7 @@
 #define PCI_DEVICE_ID_TIGON3_5754M	0x1672
 #define PCI_DEVICE_ID_TIGON3_5755M	0x1673
 #define PCI_DEVICE_ID_TIGON3_5756	0x1674
+#define PCI_DEVICE_ID_TIGON3_5750	0x1676
 #define PCI_DEVICE_ID_TIGON3_5751	0x1677
 #define PCI_DEVICE_ID_TIGON3_5715	0x1678
 #define PCI_DEVICE_ID_TIGON3_5715S	0x1679
-- 
1.7.10.4

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

* Re: [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr
  2013-01-12 20:01 ` [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr Hauke Mehrtens
@ 2013-01-12 23:32   ` David Miller
  2013-01-13  9:47     ` Michael Büsch
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2013-01-12 23:32 UTC (permalink / raw)
  To: hauke; +Cc: mcarlson, mchan, netdev, m

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 12 Jan 2013 21:01:43 +0100

> When CONFIG_SSB_DRIVER_GIGE is not set the header does not provide the
> needed method.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

This isn't right.

You can't implement this function in a way that the caller cannot
determine that it didn't actually do anything.

You either have to protect calls with ifdefs or make this routine
return an error indication, in response to which the caller can
set a random ethernet address or take some other corrective action.

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

* Re: [PATCH 0/3] tg3: add support for Ethernet core in BCM4785
  2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
                   ` (2 preceding siblings ...)
  2013-01-12 20:01 ` [PATCH 3/3] tg3: add support for Ethernet core in bcm4785 Hauke Mehrtens
@ 2013-01-12 23:32 ` David Miller
  2013-01-13 22:26   ` Hauke Mehrtens
  2013-01-24 17:21 ` Hauke Mehrtens
  4 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2013-01-12 23:32 UTC (permalink / raw)
  To: hauke; +Cc: mcarlson, mchan, netdev, m

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 12 Jan 2013 21:01:42 +0100

> These patches are adding support for the Ethernet core found in the BCM4705/BCM4785 SoC.
> 

You must always indicate what tree you are targetting your changes
at, rather than have me guess.

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

* Re: [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr
  2013-01-12 23:32   ` David Miller
@ 2013-01-13  9:47     ` Michael Büsch
  2013-01-13 22:31       ` Hauke Mehrtens
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Büsch @ 2013-01-13  9:47 UTC (permalink / raw)
  To: David Miller; +Cc: hauke, mcarlson, mchan, netdev

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

On Sat, 12 Jan 2013 15:32:11 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Hauke Mehrtens <hauke@hauke-m.de>
> Date: Sat, 12 Jan 2013 21:01:43 +0100
> 
> > When CONFIG_SSB_DRIVER_GIGE is not set the header does not provide the
> > needed method.
> > 
> > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> 
> This isn't right.
> 
> You can't implement this function in a way that the caller cannot
> determine that it didn't actually do anything.
> 
> You either have to protect calls with ifdefs or make this routine
> return an error indication, in response to which the caller can
> set a random ethernet address or take some other corrective action.

tg3 uses is_valid_ether_addr() afterwards anyway. An error code return value
could be added, but it didn't seem necessary.

-- 
Greetings, Michael.

PGP: 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 0/3] tg3: add support for Ethernet core in BCM4785
  2013-01-12 23:32 ` [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 David Miller
@ 2013-01-13 22:26   ` Hauke Mehrtens
  0 siblings, 0 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-13 22:26 UTC (permalink / raw)
  To: David Miller; +Cc: mcarlson, mchan, netdev, m

On 01/13/2013 12:32 AM, David Miller wrote:
> From: Hauke Mehrtens <hauke@hauke-m.de>
> Date: Sat, 12 Jan 2013 21:01:42 +0100
> 
>> These patches are adding support for the Ethernet core found in the BCM4705/BCM4785 SoC.
>>
> 
> You must always indicate what tree you are targetting your changes
> at, rather than have me guess.

Sorry, I missed that in the cover letter, this is based your net-next  tree.

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

* Re: [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr
  2013-01-13  9:47     ` Michael Büsch
@ 2013-01-13 22:31       ` Hauke Mehrtens
  0 siblings, 0 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-13 22:31 UTC (permalink / raw)
  To: Michael Büsch; +Cc: David Miller, mcarlson, mchan, netdev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/13/2013 10:47 AM, Michael Büsch wrote:
> On Sat, 12 Jan 2013 15:32:11 -0800 (PST) David Miller
> <davem@davemloft.net> wrote:
> 
>> From: Hauke Mehrtens <hauke@hauke-m.de> Date: Sat, 12 Jan 2013
>> 21:01:43 +0100
>> 
>>> When CONFIG_SSB_DRIVER_GIGE is not set the header does not
>>> provide the needed method.
>>> 
>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> 
>> This isn't right.
>> 
>> You can't implement this function in a way that the caller
>> cannot determine that it didn't actually do anything.
>> 
>> You either have to protect calls with ifdefs or make this
>> routine return an error indication, in response to which the
>> caller can set a random ethernet address or take some other
>> corrective action.
> 
> tg3 uses is_valid_ether_addr() afterwards anyway. An error code
> return value could be added, but it didn't seem necessary.

David, I could change that function to return an error code in this
case, but this stub should never get called, because a caller should
check if this device is a ssb gige core with pdev_is_ssb_gige_core()
before and this always returns false if CONFIG_SSB_DRIVER_GIGE is not set.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBAgAGBQJQ8zXWAAoJEIZ0px9YPRMyFgEP/Rz1BOXwbHawvupAWKCLdhm5
UquCnygVZrsbskN63c5R4OfVXGZ6UFihbf1xKHq4IRv9UXkNdymoiCmv6sUVfkjR
FemyqRZ4QOz08cbVhb0k0nEIpVguyf16063kGo4JHIGAZc54xruMZv54uZYOfvvR
GYbTsVbkzNvW2ArVesGGYjbKHyEyOmjqW5GvjyyH72cQK+vZI8PBRC8tO9l8d3D1
pq3KnHSeOEXBbODgWC36QmcMV8EEQ4BAxJQjsABXO7KRyu0HpAu7zSEn8aGhV4ln
X1q6WMzPQIOjJhmxJUGbnJEo1s05/AjT2lIU7J7QcHN21YQqIUjWzG9lYCWcbhX/
RloFjEk/Gf80CfzJj97RCLKiJScfa8tdbvFvisDZaQVbet7Ql5nHeRSyiMTzMFKi
oivkh3gmaN/2CE8rgj5QIiL3DSzy0MY2cbTZMx3QmYAlTQ52cTT3NLRaT5RKxv3c
gAUq0EvkOZhHKo/WkBWbI/V4p4CWNhJI/wNphHuOM7lJ3kvfITXw17+/29rywkZY
pB5TwdlQBtuRUjAbXvUJmfIAZE1p4bd9+0liW+qPlpoa1d7ZOaTWPhxUg+74kvgP
rItNpGf9RAb7/A/n9XjciemihcMpEH4BjShgJVNN66+LwfbQ2ZRNGsJfyJZGcYEO
n7qAWVoPmgKihES59NtA
=1JMZ
-----END PGP SIGNATURE-----

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

* Re: [PATCH 0/3] tg3: add support for Ethernet core in BCM4785
  2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
                   ` (3 preceding siblings ...)
  2013-01-12 23:32 ` [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 David Miller
@ 2013-01-24 17:21 ` Hauke Mehrtens
  4 siblings, 0 replies; 10+ messages in thread
From: Hauke Mehrtens @ 2013-01-24 17:21 UTC (permalink / raw)
  To: davem; +Cc: mcarlson, mchan, netdev, m

On 01/12/2013 09:01 PM, Hauke Mehrtens wrote:
> These patches are adding support for the Ethernet core found in the BCM4705/BCM4785 SoC.
> 
> Hauke Mehrtens (3):
>   ssb: add missing method ssb_gige_get_macaddr
>   tg3: make it possible to provide phy_id in ioctl
>   tg3: add support for Ethernet core in bcm4785
> 
>  drivers/net/ethernet/broadcom/tg3.c |  137 +++++++++++++++++++++++++++++++----
>  drivers/net/ethernet/broadcom/tg3.h |    5 ++
>  include/linux/pci_ids.h             |    1 +
>  include/linux/ssb/ssb_driver_gige.h |    3 +
>  4 files changed, 133 insertions(+), 13 deletions(-)
> 
Hi David,

Should I resend these patches with error handling if the mac address
could not be set?

Hauke

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

end of thread, other threads:[~2013-01-24 17:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-12 20:01 [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 Hauke Mehrtens
2013-01-12 20:01 ` [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr Hauke Mehrtens
2013-01-12 23:32   ` David Miller
2013-01-13  9:47     ` Michael Büsch
2013-01-13 22:31       ` Hauke Mehrtens
2013-01-12 20:01 ` [PATCH 2/3] tg3: make it possible to provide phy_id in ioctl Hauke Mehrtens
2013-01-12 20:01 ` [PATCH 3/3] tg3: add support for Ethernet core in bcm4785 Hauke Mehrtens
2013-01-12 23:32 ` [PATCH 0/3] tg3: add support for Ethernet core in BCM4785 David Miller
2013-01-13 22:26   ` Hauke Mehrtens
2013-01-24 17:21 ` Hauke Mehrtens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).