lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 9 Jul 2010 11:40:59 +0900 From: Simon Horman <horms@...ge.net.au> To: "Choi, David" <David.Choi@...rel.Com> Cc: netdev@...r.kernel.org, "Li, Charles" <Charles.Li@...rel.Com> Subject: Re: [PATCH linux-2.6.35-rc3] ks8842 driver 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@...ge.net.au] > Sent: Thursday, July 08, 2010 12:53 AM > To: Choi, David > Cc: netdev@...r.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@...rel.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@...rel.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@...paq.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@...r.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@...r.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@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists