| Message ID | C43529A246480145B0A6D0234BDB0F0D0212A4@MELANITE.micrel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jul 07, 2010 at 08:24:33AM -0700, Choi, David wrote: > To whom it may have concerned, > > It seems that there are differences between Micrel ks8842 device and Timberdale FPGA based device with generic bus interface. In order to check the differences, I have sent several times but have not received any response. This patch is to support ks8841/ks8842 device from Micrel. Is it desirable that the code can never be compiled to work with both devices? Is this code likely to be included in a generic/distro kernel? > > From: David J. Choi <david.choi@micrel.com> > > Body of the explanation: > -support 16bit and 32bit bus width. > -add device reset for ks8842/8841 Micrel device. > -set 100Mbps as a default for Micrel device. > -set MAC address in both MAC/Switch layer with different sequence for Micrel device, > as mentioned in data sheet. > > Signed-off-by: David J. Choi <david.choi@micrel.com> > > --- > --- linux-2.6.35-rc3/drivers/net/ks8842.c.orig 2010-07-01 16:26:50.000000000 -0700 > +++ linux-2.6.35-rc3/drivers/net/ks8842.c 2010-07-07 07:41:03.000000000 -0700 > @@ -18,6 +18,7 @@ > > /* Supports: > * The Micrel KS8842 behind the timberdale FPGA > + * The genuine Micrel KS8841/42 device with ISA 16/32bit bus interface > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > @@ -191,6 +192,12 @@ static inline u32 ks8842_read32(struct k > > static void ks8842_reset(struct ks8842_adapter *adapter) > { > +#ifdef CONFIG_MICREL_KS884X > + ks8842_write16(adapter, 3, 1, REG_GRR); > + msleep(10); > + iowrite16(0, adapter->hw_addr + REG_GRR); > + > +#else > /* The KS8842 goes haywire when doing softare reset > * a work around in the timberdale IP is implemented to > * do a hardware reset instead > @@ -201,6 +208,7 @@ static void ks8842_reset(struct ks8842_a > iowrite16(32, adapter->hw_addr + REG_SELECT_BANK); > iowrite32(0x1, adapter->hw_addr + REG_TIMB_RST); > msleep(20); > +#endif > } > > static void ks8842_update_link_status(struct net_device *netdev, > @@ -269,8 +277,11 @@ static void ks8842_reset_hw(struct ks884 > > /* restart port auto-negotiation */ > ks8842_enable_bits(adapter, 49, 1 << 13, REG_P1CR4); > + > +#ifndef CONFIG_MICREL_KS884X > /* only advertise 10Mbps */ > ks8842_clear_bits(adapter, 49, 3 << 2, REG_P1CR4); > +#endif > > /* Enable the transmitter */ > ks8842_enable_tx(adapter); > @@ -296,6 +307,20 @@ static void ks8842_read_mac_addr(struct > for (i = 0; i < ETH_ALEN; i++) > dest[ETH_ALEN - i - 1] = ks8842_read8(adapter, 2, REG_MARL + i); > > +#ifdef CONFIG_MICREL_KS884X > + /* > + the sequence of saving mac addr between MAC and Switch is > + different. > + */ > + > + mac = ks8842_read16(adapter, 2, REG_MARL); > + ks8842_write16(adapter, 39, mac, REG_MACAR3); > + mac = ks8842_read16(adapter, 2, REG_MARM); > + ks8842_write16(adapter, 39, mac, REG_MACAR2); > + mac = ks8842_read16(adapter, 2, REG_MARH); > + ks8842_write16(adapter, 39, mac, REG_MACAR1); > +#else > + > /* make sure the switch port uses the same MAC as the QMU */ > mac = ks8842_read16(adapter, 2, REG_MARL); > ks8842_write16(adapter, 39, mac, REG_MACAR1); > @@ -303,6 +328,7 @@ static void ks8842_read_mac_addr(struct > ks8842_write16(adapter, 39, mac, REG_MACAR2); > mac = ks8842_read16(adapter, 2, REG_MARH); > ks8842_write16(adapter, 39, mac, REG_MACAR3); > +#endif > } > > static void ks8842_write_mac_addr(struct ks8842_adapter *adapter, u8 *mac) > @@ -313,8 +339,13 @@ static void ks8842_write_mac_addr(struct > spin_lock_irqsave(&adapter->lock, flags); > for (i = 0; i < ETH_ALEN; i++) { > ks8842_write8(adapter, 2, mac[ETH_ALEN - i - 1], REG_MARL + i); > +#ifdef CONFIG_MICREL_KS884X > + ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > + REG_MACAR3 + 1 - i); > +#else > ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > REG_MACAR1 + i); > +#endif > } > spin_unlock_irqrestore(&adapter->lock, flags); > } > @@ -328,8 +359,12 @@ static int ks8842_tx_frame(struct sk_buf > { > struct ks8842_adapter *adapter = netdev_priv(netdev); > int len = skb->len; > +#ifdef CONFIG_KS884X_16BIT > + u16 *ptr16 = (u16 *)skb->data; > +#else > u32 *ptr = (u32 *)skb->data; > u32 ctrl; > +#endif > > dev_dbg(&adapter->pdev->dev, > "%s: len %u head %p data %p tail %p end %p\n", > @@ -340,6 +375,18 @@ static int ks8842_tx_frame(struct sk_buf > if (ks8842_tx_fifo_space(adapter) < len + 8) > return NETDEV_TX_BUSY; > > +#ifdef CONFIG_KS884X_16BIT > + ks8842_write16(adapter, 17, 0x8000 | 0x100, REG_QMU_DATA_LO); > + ks8842_write16(adapter, 17, (u16)len, REG_QMU_DATA_HI); > + netdev->stats.tx_bytes += len; > + > + /* copy buffer */ > + while (len > 0) { > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_LO); > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_HI); > + len -= sizeof(u32); > + } > +#else > /* the control word, enable IRQ, port 1 and the length */ > ctrl = 0x8000 | 0x100 | (len << 16); > ks8842_write32(adapter, 17, ctrl, REG_QMU_DATA_LO); > @@ -352,6 +399,7 @@ static int ks8842_tx_frame(struct sk_buf > len -= sizeof(u32); > ptr++; > } > +#endif > > /* enqueue packet */ > ks8842_write16(adapter, 17, 1, REG_TXQCR); > @@ -364,10 +412,15 @@ static int ks8842_tx_frame(struct sk_buf > static void ks8842_rx_frame(struct net_device *netdev, > struct ks8842_adapter *adapter) > { > +#ifdef CONFIG_KS884X_16BIT > + u16 status = ks8842_read16(adapter, 17, REG_QMU_DATA_LO); > + int len = (int)ks8842_read16(adapter, 17, REG_QMU_DATA_HI) & 0xffff; > +#else > u32 status = ks8842_read32(adapter, 17, REG_QMU_DATA_LO); > int len = (status >> 16) & 0x7ff; > > status &= 0xffff; > +#endif > > dev_dbg(&adapter->pdev->dev, "%s - rx_data: status: %x\n", > __func__, status); > @@ -379,13 +432,28 @@ static void ks8842_rx_frame(struct net_d > dev_dbg(&adapter->pdev->dev, "%s, got package, len: %d\n", > __func__, len); > if (skb) { > +#ifdef CONFIG_KS884X_16BIT > + u16 *data16; > +#else > u32 *data; > +#endif > > netdev->stats.rx_packets++; > netdev->stats.rx_bytes += len; > if (status & RXSR_MULTICAST) > netdev->stats.multicast++; > > +#ifdef CONFIG_KS884X_16BIT > + data16 = (u16 *)skb_put(skb, len); > + ks8842_select_bank(adapter, 17); > + while (len > 0) { > + *data16++ = ioread16(adapter->hw_addr + > + REG_QMU_DATA_LO); > + *data16++ = ioread16(adapter->hw_addr + > + REG_QMU_DATA_HI); > + len -= sizeof(u32); > + } > +#else > data = (u32 *)skb_put(skb, len); > > ks8842_select_bank(adapter, 17); > @@ -397,6 +465,7 @@ static void ks8842_rx_frame(struct net_d > > skb->protocol = eth_type_trans(skb, netdev); > netif_rx(skb); > +#endif > } else > netdev->stats.rx_dropped++; > } else { > --- linux-2.6.35-rc3/drivers/net/Kconfig.orig 2010-07-02 15:52:41.000000000 -0700 > +++ linux-2.6.35-rc3/drivers/net/Kconfig 2010-07-07 07:45:47.000000000 -0700 > @@ -1748,11 +1748,29 @@ config TLAN > Please email feedback to <torben.mathiasen@compaq.com>. > > config KS8842 > - tristate "Micrel KSZ8842" > + tristate "Micrel KSZ8841/42 with generic bus interface" > depends on HAS_IOMEM > help > - This platform driver is for Micrel KSZ8842 / KS8842 > - 2-port ethernet switch chip (managed, VLAN, QoS). > + This platform driver is for KSZ8841(1-port) / KS8842(2-port) > + ethernet switch chip (managed, VLAN, QoS) from Micrel or > + Timberdale(FPGA). > + > +if KS8842 > +config MICREL_KS884X > + boolean "KSZ8841/42 device from Micrel" > + default n > + help > + Say Y to use Micrel device. Otherwise Timberdale(FPGA) device is > + selected. > + > +config KS884X_16BIT > + boolean "16bit bus width" > + default y > + help > + This option specifies 16bit or 32bit bus interface. Say Y to use > + 16bit bus. Otherwise 32bit bus is selected. > + > +endif # KS8842 > > config KS8851 > tristate "Micrel KS8851 SPI" > --- > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Simon, Thank you for your comments. The original ks8842 driver is designed to work on the customized bus interface based on an FPGA. This patch is intended to address the more commonly used generic bus interface available on the majority of SoC in the market. It is unlikely that for a system to use both FPGA based and generic bus interface for ks8842, I am quite certain that those 2 devices are used mutual exclusively. Regards, David J. Choi -----Original Message----- From: Simon Horman [mailto:horms@verge.net.au] Sent: Thursday, July 08, 2010 12:53 AM To: Choi, David Cc: netdev@vger.kernel.org; Li, Charles Subject: Re: [PATCH linux-2.6.35-rc3] ks8842 driver On Wed, Jul 07, 2010 at 08:24:33AM -0700, Choi, David wrote: > To whom it may have concerned, > > It seems that there are differences between Micrel ks8842 device and Timberdale FPGA based device with generic bus interface. In order to check the differences, I have sent several times but have not received any response. This patch is to support ks8841/ks8842 device from Micrel. Is it desirable that the code can never be compiled to work with both devices? Is this code likely to be included in a generic/distro kernel? > > From: David J. Choi <david.choi@micrel.com> > > Body of the explanation: > -support 16bit and 32bit bus width. > -add device reset for ks8842/8841 Micrel device. > -set 100Mbps as a default for Micrel device. > -set MAC address in both MAC/Switch layer with different sequence for Micrel device, > as mentioned in data sheet. > > Signed-off-by: David J. Choi <david.choi@micrel.com> > > --- > --- linux-2.6.35-rc3/drivers/net/ks8842.c.orig 2010-07-01 16:26:50.000000000 -0700 > +++ linux-2.6.35-rc3/drivers/net/ks8842.c 2010-07-07 07:41:03.000000000 -0700 > @@ -18,6 +18,7 @@ > > /* Supports: > * The Micrel KS8842 behind the timberdale FPGA > + * The genuine Micrel KS8841/42 device with ISA 16/32bit bus interface > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > @@ -191,6 +192,12 @@ static inline u32 ks8842_read32(struct k > > static void ks8842_reset(struct ks8842_adapter *adapter) > { > +#ifdef CONFIG_MICREL_KS884X > + ks8842_write16(adapter, 3, 1, REG_GRR); > + msleep(10); > + iowrite16(0, adapter->hw_addr + REG_GRR); > + > +#else > /* The KS8842 goes haywire when doing softare reset > * a work around in the timberdale IP is implemented to > * do a hardware reset instead > @@ -201,6 +208,7 @@ static void ks8842_reset(struct ks8842_a > iowrite16(32, adapter->hw_addr + REG_SELECT_BANK); > iowrite32(0x1, adapter->hw_addr + REG_TIMB_RST); > msleep(20); > +#endif > } > > static void ks8842_update_link_status(struct net_device *netdev, > @@ -269,8 +277,11 @@ static void ks8842_reset_hw(struct ks884 > > /* restart port auto-negotiation */ > ks8842_enable_bits(adapter, 49, 1 << 13, REG_P1CR4); > + > +#ifndef CONFIG_MICREL_KS884X > /* only advertise 10Mbps */ > ks8842_clear_bits(adapter, 49, 3 << 2, REG_P1CR4); > +#endif > > /* Enable the transmitter */ > ks8842_enable_tx(adapter); > @@ -296,6 +307,20 @@ static void ks8842_read_mac_addr(struct > for (i = 0; i < ETH_ALEN; i++) > dest[ETH_ALEN - i - 1] = ks8842_read8(adapter, 2, REG_MARL + i); > > +#ifdef CONFIG_MICREL_KS884X > + /* > + the sequence of saving mac addr between MAC and Switch is > + different. > + */ > + > + mac = ks8842_read16(adapter, 2, REG_MARL); > + ks8842_write16(adapter, 39, mac, REG_MACAR3); > + mac = ks8842_read16(adapter, 2, REG_MARM); > + ks8842_write16(adapter, 39, mac, REG_MACAR2); > + mac = ks8842_read16(adapter, 2, REG_MARH); > + ks8842_write16(adapter, 39, mac, REG_MACAR1); > +#else > + > /* make sure the switch port uses the same MAC as the QMU */ > mac = ks8842_read16(adapter, 2, REG_MARL); > ks8842_write16(adapter, 39, mac, REG_MACAR1); > @@ -303,6 +328,7 @@ static void ks8842_read_mac_addr(struct > ks8842_write16(adapter, 39, mac, REG_MACAR2); > mac = ks8842_read16(adapter, 2, REG_MARH); > ks8842_write16(adapter, 39, mac, REG_MACAR3); > +#endif > } > > static void ks8842_write_mac_addr(struct ks8842_adapter *adapter, u8 *mac) > @@ -313,8 +339,13 @@ static void ks8842_write_mac_addr(struct > spin_lock_irqsave(&adapter->lock, flags); > for (i = 0; i < ETH_ALEN; i++) { > ks8842_write8(adapter, 2, mac[ETH_ALEN - i - 1], REG_MARL + i); > +#ifdef CONFIG_MICREL_KS884X > + ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > + REG_MACAR3 + 1 - i); > +#else > ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > REG_MACAR1 + i); > +#endif > } > spin_unlock_irqrestore(&adapter->lock, flags); > } > @@ -328,8 +359,12 @@ static int ks8842_tx_frame(struct sk_buf > { > struct ks8842_adapter *adapter = netdev_priv(netdev); > int len = skb->len; > +#ifdef CONFIG_KS884X_16BIT > + u16 *ptr16 = (u16 *)skb->data; > +#else > u32 *ptr = (u32 *)skb->data; > u32 ctrl; > +#endif > > dev_dbg(&adapter->pdev->dev, > "%s: len %u head %p data %p tail %p end %p\n", > @@ -340,6 +375,18 @@ static int ks8842_tx_frame(struct sk_buf > if (ks8842_tx_fifo_space(adapter) < len + 8) > return NETDEV_TX_BUSY; > > +#ifdef CONFIG_KS884X_16BIT > + ks8842_write16(adapter, 17, 0x8000 | 0x100, REG_QMU_DATA_LO); > + ks8842_write16(adapter, 17, (u16)len, REG_QMU_DATA_HI); > + netdev->stats.tx_bytes += len; > + > + /* copy buffer */ > + while (len > 0) { > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_LO); > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_HI); > + len -= sizeof(u32); > + } > +#else > /* the control word, enable IRQ, port 1 and the length */ > ctrl = 0x8000 | 0x100 | (len << 16); > ks8842_write32(adapter, 17, ctrl, REG_QMU_DATA_LO); > @@ -352,6 +399,7 @@ static int ks8842_tx_frame(struct sk_buf > len -= sizeof(u32); > ptr++; > } > +#endif > > /* enqueue packet */ > ks8842_write16(adapter, 17, 1, REG_TXQCR); > @@ -364,10 +412,15 @@ static int ks8842_tx_frame(struct sk_buf > static void ks8842_rx_frame(struct net_device *netdev, > struct ks8842_adapter *adapter) > { > +#ifdef CONFIG_KS884X_16BIT > + u16 status = ks8842_read16(adapter, 17, REG_QMU_DATA_LO); > + int len = (int)ks8842_read16(adapter, 17, REG_QMU_DATA_HI) & 0xffff; > +#else > u32 status = ks8842_read32(adapter, 17, REG_QMU_DATA_LO); > int len = (status >> 16) & 0x7ff; > > status &= 0xffff; > +#endif > > dev_dbg(&adapter->pdev->dev, "%s - rx_data: status: %x\n", > __func__, status); > @@ -379,13 +432,28 @@ static void ks8842_rx_frame(struct net_d > dev_dbg(&adapter->pdev->dev, "%s, got package, len: %d\n", > __func__, len); > if (skb) { > +#ifdef CONFIG_KS884X_16BIT > + u16 *data16; > +#else > u32 *data; > +#endif > > netdev->stats.rx_packets++; > netdev->stats.rx_bytes += len; > if (status & RXSR_MULTICAST) > netdev->stats.multicast++; > > +#ifdef CONFIG_KS884X_16BIT > + data16 = (u16 *)skb_put(skb, len); > + ks8842_select_bank(adapter, 17); > + while (len > 0) { > + *data16++ = ioread16(adapter->hw_addr + > + REG_QMU_DATA_LO); > + *data16++ = ioread16(adapter->hw_addr + > + REG_QMU_DATA_HI); > + len -= sizeof(u32); > + } > +#else > data = (u32 *)skb_put(skb, len); > > ks8842_select_bank(adapter, 17); > @@ -397,6 +465,7 @@ static void ks8842_rx_frame(struct net_d > > skb->protocol = eth_type_trans(skb, netdev); > netif_rx(skb); > +#endif > } else > netdev->stats.rx_dropped++; > } else { > --- linux-2.6.35-rc3/drivers/net/Kconfig.orig 2010-07-02 15:52:41.000000000 -0700 > +++ linux-2.6.35-rc3/drivers/net/Kconfig 2010-07-07 07:45:47.000000000 -0700 > @@ -1748,11 +1748,29 @@ config TLAN > Please email feedback to <torben.mathiasen@compaq.com>. > > config KS8842 > - tristate "Micrel KSZ8842" > + tristate "Micrel KSZ8841/42 with generic bus interface" > depends on HAS_IOMEM > help > - This platform driver is for Micrel KSZ8842 / KS8842 > - 2-port ethernet switch chip (managed, VLAN, QoS). > + This platform driver is for KSZ8841(1-port) / KS8842(2-port) > + ethernet switch chip (managed, VLAN, QoS) from Micrel or > + Timberdale(FPGA). > + > +if KS8842 > +config MICREL_KS884X > + boolean "KSZ8841/42 device from Micrel" > + default n > + help > + Say Y to use Micrel device. Otherwise Timberdale(FPGA) device is > + selected. > + > +config KS884X_16BIT > + boolean "16bit bus width" > + default y > + help > + This option specifies 16bit or 32bit bus interface. Say Y to use > + 16bit bus. Otherwise 32bit bus is selected. > + > +endif # KS8842 > > config KS8851 > tristate "Micrel KS8851 SPI" > --- > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 08, 2010 at 12:01:51PM -0700, Choi, David wrote: > Hi Simon, > > Thank you for your comments. > > The original ks8842 driver is designed to work on the customized bus > interface based on an FPGA. This patch is intended to address the more > commonly used generic bus interface available on the majority of SoC in > the market. > > It is unlikely that for a system to use both FPGA based and generic bus > interface for ks8842, I am quite certain that those 2 devices are used > mutual exclusively. Hi David, Understood, in that case I think that using #ifdef is reasonable. Though to my mind it would be nicer to try and abstract it out, as you have in the first hunk, to something of the form: foo() { A #ifdef CONFIG_MICREL_KS884X B #else C #endif D } And then call foo() unconditionally. I think this is cleaner than, for example ks8842_rx_frame() where there are several instances of the same #ifdef condition. > > Regards, > David J. Choi > > > -----Original Message----- > From: Simon Horman [mailto:horms@verge.net.au] > Sent: Thursday, July 08, 2010 12:53 AM > To: Choi, David > Cc: netdev@vger.kernel.org; Li, Charles > Subject: Re: [PATCH linux-2.6.35-rc3] ks8842 driver > > On Wed, Jul 07, 2010 at 08:24:33AM -0700, Choi, David wrote: > > To whom it may have concerned, > > > > It seems that there are differences between Micrel ks8842 device and > Timberdale FPGA based device with generic bus interface. In order to > check the differences, I have sent several times but have not received > any response. This patch is to support ks8841/ks8842 device from Micrel. > > Is it desirable that the code can never be compiled to work with both > devices? > Is this code likely to be included in a generic/distro kernel? > > > > > From: David J. Choi <david.choi@micrel.com> > > > > Body of the explanation: > > -support 16bit and 32bit bus width. > > -add device reset for ks8842/8841 Micrel device. > > -set 100Mbps as a default for Micrel device. > > -set MAC address in both MAC/Switch layer with different sequence > for Micrel device, > > as mentioned in data sheet. > > > > Signed-off-by: David J. Choi <david.choi@micrel.com> > > > > --- > > --- linux-2.6.35-rc3/drivers/net/ks8842.c.orig 2010-07-01 > 16:26:50.000000000 -0700 > > +++ linux-2.6.35-rc3/drivers/net/ks8842.c 2010-07-07 > 07:41:03.000000000 -0700 > > @@ -18,6 +18,7 @@ > > > > /* Supports: > > * The Micrel KS8842 behind the timberdale FPGA > > + * The genuine Micrel KS8841/42 device with ISA 16/32bit bus > interface > > */ > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > @@ -191,6 +192,12 @@ static inline u32 ks8842_read32(struct k > > > > static void ks8842_reset(struct ks8842_adapter *adapter) > > { > > +#ifdef CONFIG_MICREL_KS884X > > + ks8842_write16(adapter, 3, 1, REG_GRR); > > + msleep(10); > > + iowrite16(0, adapter->hw_addr + REG_GRR); > > + > > +#else > > /* The KS8842 goes haywire when doing softare reset > > * a work around in the timberdale IP is implemented to > > * do a hardware reset instead > > @@ -201,6 +208,7 @@ static void ks8842_reset(struct ks8842_a > > iowrite16(32, adapter->hw_addr + REG_SELECT_BANK); > > iowrite32(0x1, adapter->hw_addr + REG_TIMB_RST); > > msleep(20); > > +#endif > > } > > > > static void ks8842_update_link_status(struct net_device *netdev, > > @@ -269,8 +277,11 @@ static void ks8842_reset_hw(struct ks884 > > > > /* restart port auto-negotiation */ > > ks8842_enable_bits(adapter, 49, 1 << 13, REG_P1CR4); > > + > > +#ifndef CONFIG_MICREL_KS884X > > /* only advertise 10Mbps */ > > ks8842_clear_bits(adapter, 49, 3 << 2, REG_P1CR4); > > +#endif > > > > /* Enable the transmitter */ > > ks8842_enable_tx(adapter); > > @@ -296,6 +307,20 @@ static void ks8842_read_mac_addr(struct > > for (i = 0; i < ETH_ALEN; i++) > > dest[ETH_ALEN - i - 1] = ks8842_read8(adapter, 2, > REG_MARL + i); > > > > +#ifdef CONFIG_MICREL_KS884X > > + /* > > + the sequence of saving mac addr between MAC and Switch > is > > + different. > > + */ > > + > > + mac = ks8842_read16(adapter, 2, REG_MARL); > > + ks8842_write16(adapter, 39, mac, REG_MACAR3); > > + mac = ks8842_read16(adapter, 2, REG_MARM); > > + ks8842_write16(adapter, 39, mac, REG_MACAR2); > > + mac = ks8842_read16(adapter, 2, REG_MARH); > > + ks8842_write16(adapter, 39, mac, REG_MACAR1); > > +#else > > + > > /* make sure the switch port uses the same MAC as the QMU */ > > mac = ks8842_read16(adapter, 2, REG_MARL); > > ks8842_write16(adapter, 39, mac, REG_MACAR1); > > @@ -303,6 +328,7 @@ static void ks8842_read_mac_addr(struct > > ks8842_write16(adapter, 39, mac, REG_MACAR2); > > mac = ks8842_read16(adapter, 2, REG_MARH); > > ks8842_write16(adapter, 39, mac, REG_MACAR3); > > +#endif > > } > > > > static void ks8842_write_mac_addr(struct ks8842_adapter *adapter, u8 > *mac) > > @@ -313,8 +339,13 @@ static void ks8842_write_mac_addr(struct > > spin_lock_irqsave(&adapter->lock, flags); > > for (i = 0; i < ETH_ALEN; i++) { > > ks8842_write8(adapter, 2, mac[ETH_ALEN - i - 1], > REG_MARL + i); > > +#ifdef CONFIG_MICREL_KS884X > > + ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > > + REG_MACAR3 + 1 - i); > > +#else > > ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], > > REG_MACAR1 + i); > > +#endif > > } > > spin_unlock_irqrestore(&adapter->lock, flags); > > } > > @@ -328,8 +359,12 @@ static int ks8842_tx_frame(struct sk_buf > > { > > struct ks8842_adapter *adapter = netdev_priv(netdev); > > int len = skb->len; > > +#ifdef CONFIG_KS884X_16BIT > > + u16 *ptr16 = (u16 *)skb->data; > > +#else > > u32 *ptr = (u32 *)skb->data; > > u32 ctrl; > > +#endif > > > > dev_dbg(&adapter->pdev->dev, > > "%s: len %u head %p data %p tail %p end %p\n", > > @@ -340,6 +375,18 @@ static int ks8842_tx_frame(struct sk_buf > > if (ks8842_tx_fifo_space(adapter) < len + 8) > > return NETDEV_TX_BUSY; > > > > +#ifdef CONFIG_KS884X_16BIT > > + ks8842_write16(adapter, 17, 0x8000 | 0x100, REG_QMU_DATA_LO); > > + ks8842_write16(adapter, 17, (u16)len, REG_QMU_DATA_HI); > > + netdev->stats.tx_bytes += len; > > + > > + /* copy buffer */ > > + while (len > 0) { > > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_LO); > > + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_HI); > > + len -= sizeof(u32); > > + } > > +#else > > /* the control word, enable IRQ, port 1 and the length */ > > ctrl = 0x8000 | 0x100 | (len << 16); > > ks8842_write32(adapter, 17, ctrl, REG_QMU_DATA_LO); > > @@ -352,6 +399,7 @@ static int ks8842_tx_frame(struct sk_buf > > len -= sizeof(u32); > > ptr++; > > } > > +#endif > > > > /* enqueue packet */ > > ks8842_write16(adapter, 17, 1, REG_TXQCR); > > @@ -364,10 +412,15 @@ static int ks8842_tx_frame(struct sk_buf > > static void ks8842_rx_frame(struct net_device *netdev, > > struct ks8842_adapter *adapter) > > { > > +#ifdef CONFIG_KS884X_16BIT > > + u16 status = ks8842_read16(adapter, 17, REG_QMU_DATA_LO); > > + int len = (int)ks8842_read16(adapter, 17, REG_QMU_DATA_HI) & > 0xffff; > > +#else > > u32 status = ks8842_read32(adapter, 17, REG_QMU_DATA_LO); > > int len = (status >> 16) & 0x7ff; > > > > status &= 0xffff; > > +#endif > > > > dev_dbg(&adapter->pdev->dev, "%s - rx_data: status: %x\n", > > __func__, status); > > @@ -379,13 +432,28 @@ static void ks8842_rx_frame(struct net_d > > dev_dbg(&adapter->pdev->dev, "%s, got package, len: > %d\n", > > __func__, len); > > if (skb) { > > +#ifdef CONFIG_KS884X_16BIT > > + u16 *data16; > > +#else > > u32 *data; > > +#endif > > > > netdev->stats.rx_packets++; > > netdev->stats.rx_bytes += len; > > if (status & RXSR_MULTICAST) > > netdev->stats.multicast++; > > > > +#ifdef CONFIG_KS884X_16BIT > > + data16 = (u16 *)skb_put(skb, len); > > + ks8842_select_bank(adapter, 17); > > + while (len > 0) { > > + *data16++ = ioread16(adapter->hw_addr + > > + REG_QMU_DATA_LO); > > + *data16++ = ioread16(adapter->hw_addr + > > + REG_QMU_DATA_HI); > > + len -= sizeof(u32); > > + } > > +#else > > data = (u32 *)skb_put(skb, len); > > > > ks8842_select_bank(adapter, 17); > > @@ -397,6 +465,7 @@ static void ks8842_rx_frame(struct net_d > > > > skb->protocol = eth_type_trans(skb, netdev); > > netif_rx(skb); > > +#endif > > } else > > netdev->stats.rx_dropped++; > > } else { > > --- linux-2.6.35-rc3/drivers/net/Kconfig.orig 2010-07-02 > 15:52:41.000000000 -0700 > > +++ linux-2.6.35-rc3/drivers/net/Kconfig 2010-07-07 > 07:45:47.000000000 -0700 > > @@ -1748,11 +1748,29 @@ config TLAN > > Please email feedback to <torben.mathiasen@compaq.com>. > > > > config KS8842 > > - tristate "Micrel KSZ8842" > > + tristate "Micrel KSZ8841/42 with generic bus interface" > > depends on HAS_IOMEM > > help > > - This platform driver is for Micrel KSZ8842 / KS8842 > > - 2-port ethernet switch chip (managed, VLAN, QoS). > > + This platform driver is for KSZ8841(1-port) / KS8842(2-port) > > + ethernet switch chip (managed, VLAN, QoS) from Micrel or > > + Timberdale(FPGA). > > + > > +if KS8842 > > +config MICREL_KS884X > > + boolean "KSZ8841/42 device from Micrel" > > + default n > > + help > > + Say Y to use Micrel device. Otherwise Timberdale(FPGA) device > is > > + selected. > > + > > +config KS884X_16BIT > > + boolean "16bit bus width" > > + default y > > + help > > + This option specifies 16bit or 32bit bus interface. Say Y to > use > > + 16bit bus. Otherwise 32bit bus is selected. > > + > > +endif # KS8842 > > > > config KS8851 > > tristate "Micrel KS8851 SPI" > > --- > > -- > > To unsubscribe from this list: send the line "unsubscribe netdev" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Choi, David" <David.Choi@Micrel.Com> Date: Thu, 8 Jul 2010 12:01:51 -0700 > The original ks8842 driver is designed to work on the customized bus > interface based on an FPGA. This patch is intended to address the more > commonly used generic bus interface available on the majority of SoC in > the market. > > It is unlikely that for a system to use both FPGA based and generic bus > interface for ks8842, I am quite certain that those 2 devices are used > mutual exclusively. Like Simon, I'm not to thrilled with this approach. Any flag bit test you'd need to add to the driver to handle both cases will have zero performance impact since the cost of the MMIO accesses will dominate such tests entirely. Add a boolean flag bit to the driver software state, set it based upon some platform_device private setting, and test it in these paths to device what to do. As a bonus, anyone who enables this driver at all in their build will test the compilation of both code paths. And to me, that extra compilation testing trumps whatever arguments you may make for not making this support dynamic. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 08, 2010 at 09:41:01PM -0700, David Miller wrote: > From: "Choi, David" <David.Choi@Micrel.Com> > Date: Thu, 8 Jul 2010 12:01:51 -0700 > > > The original ks8842 driver is designed to work on the customized bus > > interface based on an FPGA. This patch is intended to address the more > > commonly used generic bus interface available on the majority of SoC in > > the market. > > > > It is unlikely that for a system to use both FPGA based and generic bus > > interface for ks8842, I am quite certain that those 2 devices are used > > mutual exclusively. > > Like Simon, I'm not to thrilled with this approach. > > Any flag bit test you'd need to add to the driver to handle both cases > will have zero performance impact since the cost of the MMIO accesses > will dominate such tests entirely. > > Add a boolean flag bit to the driver software state, set it based upon > some platform_device private setting, and test it in these paths to > device what to do. > > As a bonus, anyone who enables this driver at all in their build will > test the compilation of both code paths. And to me, that extra > compilation testing trumps whatever arguments you may make for not > making this support dynamic. I was thinking more in terms of needing fewer kernels, but yes build coverage is a big win. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-2.6.35-rc3/drivers/net/ks8842.c.orig 2010-07-01 16:26:50.000000000 -0700 +++ linux-2.6.35-rc3/drivers/net/ks8842.c 2010-07-07 07:41:03.000000000 -0700 @@ -18,6 +18,7 @@ /* Supports: * The Micrel KS8842 behind the timberdale FPGA + * The genuine Micrel KS8841/42 device with ISA 16/32bit bus interface */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt @@ -191,6 +192,12 @@ static inline u32 ks8842_read32(struct k static void ks8842_reset(struct ks8842_adapter *adapter) { +#ifdef CONFIG_MICREL_KS884X + ks8842_write16(adapter, 3, 1, REG_GRR); + msleep(10); + iowrite16(0, adapter->hw_addr + REG_GRR); + +#else /* The KS8842 goes haywire when doing softare reset * a work around in the timberdale IP is implemented to * do a hardware reset instead @@ -201,6 +208,7 @@ static void ks8842_reset(struct ks8842_a iowrite16(32, adapter->hw_addr + REG_SELECT_BANK); iowrite32(0x1, adapter->hw_addr + REG_TIMB_RST); msleep(20); +#endif } static void ks8842_update_link_status(struct net_device *netdev, @@ -269,8 +277,11 @@ static void ks8842_reset_hw(struct ks884 /* restart port auto-negotiation */ ks8842_enable_bits(adapter, 49, 1 << 13, REG_P1CR4); + +#ifndef CONFIG_MICREL_KS884X /* only advertise 10Mbps */ ks8842_clear_bits(adapter, 49, 3 << 2, REG_P1CR4); +#endif /* Enable the transmitter */ ks8842_enable_tx(adapter); @@ -296,6 +307,20 @@ static void ks8842_read_mac_addr(struct for (i = 0; i < ETH_ALEN; i++) dest[ETH_ALEN - i - 1] = ks8842_read8(adapter, 2, REG_MARL + i); +#ifdef CONFIG_MICREL_KS884X + /* + the sequence of saving mac addr between MAC and Switch is + different. + */ + + mac = ks8842_read16(adapter, 2, REG_MARL); + ks8842_write16(adapter, 39, mac, REG_MACAR3); + mac = ks8842_read16(adapter, 2, REG_MARM); + ks8842_write16(adapter, 39, mac, REG_MACAR2); + mac = ks8842_read16(adapter, 2, REG_MARH); + ks8842_write16(adapter, 39, mac, REG_MACAR1); +#else + /* make sure the switch port uses the same MAC as the QMU */ mac = ks8842_read16(adapter, 2, REG_MARL); ks8842_write16(adapter, 39, mac, REG_MACAR1); @@ -303,6 +328,7 @@ static void ks8842_read_mac_addr(struct ks8842_write16(adapter, 39, mac, REG_MACAR2); mac = ks8842_read16(adapter, 2, REG_MARH); ks8842_write16(adapter, 39, mac, REG_MACAR3); +#endif } static void ks8842_write_mac_addr(struct ks8842_adapter *adapter, u8 *mac) @@ -313,8 +339,13 @@ static void ks8842_write_mac_addr(struct spin_lock_irqsave(&adapter->lock, flags); for (i = 0; i < ETH_ALEN; i++) { ks8842_write8(adapter, 2, mac[ETH_ALEN - i - 1], REG_MARL + i); +#ifdef CONFIG_MICREL_KS884X + ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], + REG_MACAR3 + 1 - i); +#else ks8842_write8(adapter, 39, mac[ETH_ALEN - i - 1], REG_MACAR1 + i); +#endif } spin_unlock_irqrestore(&adapter->lock, flags); } @@ -328,8 +359,12 @@ static int ks8842_tx_frame(struct sk_buf { struct ks8842_adapter *adapter = netdev_priv(netdev); int len = skb->len; +#ifdef CONFIG_KS884X_16BIT + u16 *ptr16 = (u16 *)skb->data; +#else u32 *ptr = (u32 *)skb->data; u32 ctrl; +#endif dev_dbg(&adapter->pdev->dev, "%s: len %u head %p data %p tail %p end %p\n", @@ -340,6 +375,18 @@ static int ks8842_tx_frame(struct sk_buf if (ks8842_tx_fifo_space(adapter) < len + 8) return NETDEV_TX_BUSY; +#ifdef CONFIG_KS884X_16BIT + ks8842_write16(adapter, 17, 0x8000 | 0x100, REG_QMU_DATA_LO); + ks8842_write16(adapter, 17, (u16)len, REG_QMU_DATA_HI); + netdev->stats.tx_bytes += len; + + /* copy buffer */ + while (len > 0) { + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_LO); + iowrite16(*ptr16++, adapter->hw_addr + REG_QMU_DATA_HI); + len -= sizeof(u32); + } +#else /* the control word, enable IRQ, port 1 and the length */ ctrl = 0x8000 | 0x100 | (len << 16); ks8842_write32(adapter, 17, ctrl, REG_QMU_DATA_LO); @@ -352,6 +399,7 @@ static int ks8842_tx_frame(struct sk_buf len -= sizeof(u32); ptr++; } +#endif /* enqueue packet */ ks8842_write16(adapter, 17, 1, REG_TXQCR); @@ -364,10 +412,15 @@ static int ks8842_tx_frame(struct sk_buf static void ks8842_rx_frame(struct net_device *netdev, struct ks8842_adapter *adapter) { +#ifdef CONFIG_KS884X_16BIT + u16 status = ks8842_read16(adapter, 17, REG_QMU_DATA_LO); + int len = (int)ks8842_read16(adapter, 17, REG_QMU_DATA_HI) & 0xffff; +#else u32 status = ks8842_read32(adapter, 17, REG_QMU_DATA_LO); int len = (status >> 16) & 0x7ff; status &= 0xffff; +#endif dev_dbg(&adapter->pdev->dev, "%s - rx_data: status: %x\n", __func__, status); @@ -379,13 +432,28 @@ static void ks8842_rx_frame(struct net_d dev_dbg(&adapter->pdev->dev, "%s, got package, len: %d\n", __func__, len); if (skb) { +#ifdef CONFIG_KS884X_16BIT + u16 *data16; +#else u32 *data; +#endif netdev->stats.rx_packets++; netdev->stats.rx_bytes += len; if (status & RXSR_MULTICAST) netdev->stats.multicast++; +#ifdef CONFIG_KS884X_16BIT + data16 = (u16 *)skb_put(skb, len); + ks8842_select_bank(adapter, 17); + while (len > 0) { + *data16++ = ioread16(adapter->hw_addr + + REG_QMU_DATA_LO); + *data16++ = ioread16(adapter->hw_addr + + REG_QMU_DATA_HI); + len -= sizeof(u32); + } +#else data = (u32 *)skb_put(skb, len); ks8842_select_bank(adapter, 17); @@ -397,6 +465,7 @@ static void ks8842_rx_frame(struct net_d skb->protocol = eth_type_trans(skb, netdev); netif_rx(skb); +#endif } else netdev->stats.rx_dropped++; } else { --- linux-2.6.35-rc3/drivers/net/Kconfig.orig 2010-07-02 15:52:41.000000000 -0700 +++ linux-2.6.35-rc3/drivers/net/Kconfig 2010-07-07 07:45:47.000000000 -0700 @@ -1748,11 +1748,29 @@ config TLAN Please email feedback to <torben.mathiasen@compaq.com>. config KS8842 - tristate "Micrel KSZ8842" + tristate "Micrel KSZ8841/42 with generic bus interface" depends on HAS_IOMEM help - This platform driver is for Micrel KSZ8842 / KS8842 - 2-port ethernet switch chip (managed, VLAN, QoS). + This platform driver is for KSZ8841(1-port) / KS8842(2-port) + ethernet switch chip (managed, VLAN, QoS) from Micrel or + Timberdale(FPGA). + +if KS8842 +config MICREL_KS884X + boolean "KSZ8841/42 device from Micrel" + default n + help + Say Y to use Micrel device. Otherwise Timberdale(FPGA) device is + selected. + +config KS884X_16BIT + boolean "16bit bus width" + default y + help + This option specifies 16bit or 32bit bus interface. Say Y to use + 16bit bus. Otherwise 32bit bus is selected. + +endif # KS8842 config KS8851 tristate "Micrel KS8851 SPI"