[PATCH] r8168: r8168 fixes

[PATCH] r8168: r8168 fixes

[Nishit Shah]

Hi,

The following patch fixes three r8168 bugs:

1) vlan tag missing in transmit path. Due to incorrect assignment of
transmit vlan tag option, driver is not able to transmit data on vlan
interface.

2) compilation warning with rtl8168_powerup_pll if CONFIG_PM isn’t defined.

3) compilation warning with unused variable dev in rtl8168_tx_clear.

Signed-off-by: Rajesh Modhwadiya <rajesh.modhwadiya@elitecore.com>
Signed-off-by: Nishit Shah <nishit.shah@elitecore.com>

---
r8168_n.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- r8168-8.011.00/src/r8168_n.c        2009-02-18 09:23:21.000000000 +0530
+++ r8168-8.011.00-tmp/src/r8168_n.c    2009-05-15 01:40:57.000000000 +0530
@@ -808,6 +808,8 @@
        }
 }

+#ifdef CONFIG_PM
+
 static void
 rtl8168_powerup_pll(struct net_device *dev)  { @@ -825,6 +827,8 @@
        rtl8168_set_speed(dev, tp->autoneg, tp->speed, tp->duplex);  }

+#endif /* CONFIG_PM */
+
 static void
 rtl8168_get_wol(struct net_device *dev,
                struct ethtool_wolinfo *wol) @@ -4151,7 +4155,6 @@
rtl8168_tx_clear(struct rtl8168_private *tp)  {
        unsigned int i;
-       struct net_device *dev = tp->dev;

        for (i = tp->dirty_tx; i < tp->dirty_tx + NUM_TX_DESC; i++) {
                unsigned int entry = i % NUM_TX_DESC; @@ -4436,7 +4439,7 @@

        tp->tx_skb[entry].len = len;
        txd->addr = cpu_to_le64(mapping);
-       txd->opts2 = cpu_to_le32(rtl8168_tx_vlan_tag(tp, skb));
+       opts2 |= cpu_to_le32(rtl8168_tx_vlan_tag(tp, skb));

        wmb();


Rgds,
Nishit Shah.

Re: [PATCH] r8168: r8168 fixes

[Francois Romieu]

Nishit Shah <nsshah.82@gmail.com> :
[...]
> The following patch fixes three r8168 bugs:

Please Cc: edward_hsu@realtek.com when you send patches for the
out-of-tree r8168 driver.

> 1) vlan tag missing in transmit path. Due to incorrect assignment of
> transmit vlan tag option, driver is not able to transmit data on vlan
> interface.

The fix does not seem completely right either, see below.

> 2) compilation warning with rtl8168_powerup_pll if CONFIG_PM isn’t defined.
> 
> 3) compilation warning with unused variable dev in rtl8168_tx_clear.
> 
> Signed-off-by: Rajesh Modhwadiya <rajesh.modhwadiya@elitecore.com>
> Signed-off-by: Nishit Shah <nishit.shah@elitecore.com>
[...]
> --- r8168-8.011.00/src/r8168_n.c        2009-02-18 09:23:21.000000000 +0530
> +++ r8168-8.011.00-tmp/src/r8168_n.c    2009-05-15 01:40:57.000000000 +0530
> @@ -808,6 +808,8 @@
>         }
>  }
> 
> +#ifdef CONFIG_PM
> +
>  static void
>  rtl8168_powerup_pll(struct net_device *dev)  { @@ -825,6 +827,8 @@
>         rtl8168_set_speed(dev, tp->autoneg, tp->speed, tp->duplex);  }
> 
> +#endif /* CONFIG_PM */
> +
>  static void
>  rtl8168_get_wol(struct net_device *dev,
>                 struct ethtool_wolinfo *wol) @@ -4151,7 +4155,6 @@

The patch is garbled.

> rtl8168_tx_clear(struct rtl8168_private *tp)  {
>         unsigned int i;
> -       struct net_device *dev = tp->dev;
> 
>         for (i = tp->dirty_tx; i < tp->dirty_tx + NUM_TX_DESC; i++) {
>                 unsigned int entry = i % NUM_TX_DESC; @@ -4436,7 +4439,7 @@
> 
>         tp->tx_skb[entry].len = len;
>         txd->addr = cpu_to_le64(mapping);
> -       txd->opts2 = cpu_to_le32(rtl8168_tx_vlan_tag(tp, skb));
> +       opts2 |= cpu_to_le32(rtl8168_tx_vlan_tag(tp, skb));
> 
>         wmb();
[...]
        status2 = opts2;
[...]
        txd->opts2 = cpu_to_le32(status2);

txd->opts2 = cpu_to_le32( ... | cpu_to_le32(rtl8168_tx_vlan_tag(tp, skb)));

-> cpu_to_le32 twice ?

-- 
Ueimor

Re: [PATCH] r8168: r8168 fixes

[David Miller]

From: Nishit Shah <nsshah.82@gmail.com>
Date: Wed, 3 Jun 2009 14:14:11 +0530

> Hi,
> 
> The following patch fixes three r8168 bugs:
> 
> 1) vlan tag missing in transmit path. Due to incorrect assignment of
> transmit vlan tag option, driver is not able to transmit data on vlan
> interface.
> 
> 2) compilation warning with rtl8168_powerup_pll if CONFIG_PM isn’t defined.
> 
> 3) compilation warning with unused variable dev in rtl8168_tx_clear.
> 
> Signed-off-by: Rajesh Modhwadiya <rajesh.modhwadiya@elitecore.com>
> Signed-off-by: Nishit Shah <nishit.shah@elitecore.com>

There is no file named r8168*, or anything like it, in the upstream
kernel tree.

You should not be submitting patches for out-of-tree drivers
to this mailing list, thank you.

Re: [PATCH] r8168: r8168 fixes

[nishit.shah]

Hi Dave,

>
> There is no file named r8168*, or anything like it, in the upstream
> kernel tree.
>
> You should not be submitting patches for out-of-tree drivers
> to this mailing list, thank you.
>

I am aware that r8168 is not a part of upstream kernel but I have
submitted the patch due to modinfo output of the driver,

[root@kernel-dev r8168]# modinfo r8168.ko
filename:       r8168.ko
version:        8.011.00-NAPI
license:        GPL
description:    RealTek RTL-8168 Gigabit Ethernet driver
author:         Realtek and the Linux r8168 crew <netdev@vger.kernel.org>
srcversion:     053AD75C2DC4A728C4477D9
alias:          pci:v000010ECd00008168sv*sd*bc*sc*i*

In author field, mail address in netdev@vger.kernel.org.

Rgds,
Nishit Shah.

Re: [PATCH] r8168: r8168 fixes

[David Miller]

From: nishit.shah@elitecore.com
Date: Thu, 4 Jun 2009 12:13:25 +0530 (IST)

> description:    RealTek RTL-8168 Gigabit Ethernet driver
> author:         Realtek and the Linux r8168 crew <netdev@vger.kernel.org>

They should have used a real developer address, and if not
definitely anything other than netdev until this driver
is actually merged :-/