[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

The rtl8169 driver uses a global variable to store the register address
of the adapter being operated upon. This is updated to point at the
correct adapter when sending or receiving a packet, or shutting down the
adapter, but not when initializing the adapter. Consequently, switching
between different adapters within the same U-Boot runtime does not work
correctly since the driver programs the wrong registers during
rtl8169_eth_start() -> rtl8169_common_start() -> rtl8169_hw_start().

Note that since rtl8169_eth_stop() does set the global variable, the
second consecutive attempt to use the "new" adapter did work even before
this patch, because each time network usage is shut down, the network
core calls stop, which sets the variable so that the next start does
actually initialize the hardware, and the adapter works.

Equally, rtl8169_eth_probe() calls rtl_init() which sets the global, so
if using only a single device, or if picking the "right" device (based on
probe order) when multiple devices are present, ioaddr will already be set
correctly from the get-go, so the issue does not occur.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note that as with almost any patch, I believe NVIDIA copyright applies to
the submission. However, I don't want to block all my work based on that
other conversion, so I'm just going to deliberately ignore our policy of
adding/updating copyright headers to files when editing them. Hopefully
doing so deliberately (rather than the usual case of just forgetting)
doesn't get me fired.
---
 drivers/net/rtl8169.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 163b9df55c9b..843b083f8f90 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -850,9 +850,11 @@ static void rtl8169_init_ring(pci_dev_t dev)
 }
 
 #ifdef CONFIG_DM_ETH
-static void rtl8169_common_start(struct udevice *dev, unsigned char *enetaddr)
+static void rtl8169_common_start(struct udevice *dev, unsigned char *enetaddr,
+				 unsigned long dev_iobase)
 #else
-static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr)
+static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr,
+				 unsigned long dev_iobase)
 #endif
 {
 	int i;
@@ -862,6 +864,8 @@ static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr)
 	printf ("%s\n", __FUNCTION__);
 #endif
 
+	ioaddr = dev_iobase;
+
 	rtl8169_init_ring(dev);
 	rtl8169_hw_start(dev);
 	/* Construct a perfect filter frame with the mac address as first match
@@ -885,8 +889,9 @@ static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr)
 static int rtl8169_eth_start(struct udevice *dev)
 {
 	struct eth_pdata *plat = dev_get_platdata(dev);
+	struct rtl8169_private *priv = dev_get_priv(dev);
 
-	rtl8169_common_start(dev, plat->enetaddr);
+	rtl8169_common_start(dev, plat->enetaddr, priv->iobase);
 
 	return 0;
 }
@@ -897,7 +902,7 @@ RESET - Finish setting up the ethernet interface
 static int rtl_reset(struct eth_device *dev, bd_t *bis)
 {
 	rtl8169_common_start((pci_dev_t)(unsigned long)dev->priv,
-			     dev->enetaddr);
+			     dev->enetaddr, dev->iobase);
 
 	return 0;
 }
-- 
2.8.1

[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[Joe Hershberger]

On Tue, Apr 26, 2016 at 4:29 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The rtl8169 driver uses a global variable to store the register address
> of the adapter being operated upon. This is updated to point at the
> correct adapter when sending or receiving a packet, or shutting down the
> adapter, but not when initializing the adapter. Consequently, switching
> between different adapters within the same U-Boot runtime does not work
> correctly since the driver programs the wrong registers during
> rtl8169_eth_start() -> rtl8169_common_start() -> rtl8169_hw_start().
>
> Note that since rtl8169_eth_stop() does set the global variable, the
> second consecutive attempt to use the "new" adapter did work even before
> this patch, because each time network usage is shut down, the network
> core calls stop, which sets the variable so that the next start does
> actually initialize the hardware, and the adapter works.
>
> Equally, rtl8169_eth_probe() calls rtl_init() which sets the global, so
> if using only a single device, or if picking the "right" device (based on
> probe order) when multiple devices are present, ioaddr will already be set
> correctly from the get-go, so the issue does not occur.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Looks reasonable to me.
Acked-by: Joe Hershberger <joe.hershberger@ni.com>

> ---
> Note that as with almost any patch, I believe NVIDIA copyright applies to
> the submission. However, I don't want to block all my work based on that
> other conversion, so I'm just going to deliberately ignore our policy of
> adding/updating copyright headers to files when editing them. Hopefully
> doing so deliberately (rather than the usual case of just forgetting)
> doesn't get me fired.

Hope you keep your job... at least if you like it.

-Joe

[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[Stephen Warren]

On 04/26/2016 04:05 PM, Joe Hershberger wrote:
> On Tue, Apr 26, 2016 at 4:29 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The rtl8169 driver uses a global variable to store the register address
>> of the adapter being operated upon. This is updated to point at the
>> correct adapter when sending or receiving a packet, or shutting down the
>> adapter, but not when initializing the adapter. Consequently, switching
>> between different adapters within the same U-Boot runtime does not work
>> correctly since the driver programs the wrong registers during
>> rtl8169_eth_start() -> rtl8169_common_start() -> rtl8169_hw_start().
>>
>> Note that since rtl8169_eth_stop() does set the global variable, the
>> second consecutive attempt to use the "new" adapter did work even before
>> this patch, because each time network usage is shut down, the network
>> core calls stop, which sets the variable so that the next start does
>> actually initialize the hardware, and the adapter works.
>>
>> Equally, rtl8169_eth_probe() calls rtl_init() which sets the global, so
>> if using only a single device, or if picking the "right" device (based on
>> probe order) when multiple devices are present, ioaddr will already be set
>> correctly from the get-go, so the issue does not occur.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> Looks reasonable to me.
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Joe,

I assume you'll be applying this?

[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[Joe Hershberger]

On Wed, Jun 1, 2016 at 11:40 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/26/2016 04:05 PM, Joe Hershberger wrote:
>>
>> On Tue, Apr 26, 2016 at 4:29 PM, Stephen Warren <swarren@wwwdotorg.org>
>> wrote:
>>>
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> The rtl8169 driver uses a global variable to store the register address
>>> of the adapter being operated upon. This is updated to point at the
>>> correct adapter when sending or receiving a packet, or shutting down the
>>> adapter, but not when initializing the adapter. Consequently, switching
>>> between different adapters within the same U-Boot runtime does not work
>>> correctly since the driver programs the wrong registers during
>>> rtl8169_eth_start() -> rtl8169_common_start() -> rtl8169_hw_start().
>>>
>>> Note that since rtl8169_eth_stop() does set the global variable, the
>>> second consecutive attempt to use the "new" adapter did work even before
>>> this patch, because each time network usage is shut down, the network
>>> core calls stop, which sets the variable so that the next start does
>>> actually initialize the hardware, and the adapter works.
>>>
>>> Equally, rtl8169_eth_probe() calls rtl_init() which sets the global, so
>>> if using only a single device, or if picking the "right" device (based on
>>> probe order) when multiple devices are present, ioaddr will already be
>>> set
>>> correctly from the get-go, so the issue does not occur.
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>>
>> Looks reasonable to me.
>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>
>
> Joe,
>
> I assume you'll be applying this?

Yes.

[U-Boot] [PATCH] net: rtl8169: fix switching between adapters

[Stephen Warren]

On 06/02/2016 04:06 PM, Joe Hershberger wrote:
> On Wed, Jun 1, 2016 at 11:40 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/26/2016 04:05 PM, Joe Hershberger wrote:
>>>
>>> On Tue, Apr 26, 2016 at 4:29 PM, Stephen Warren <swarren@wwwdotorg.org>
>>> wrote:
>>>>
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> The rtl8169 driver uses a global variable to store the register address
>>>> of the adapter being operated upon. This is updated to point at the
>>>> correct adapter when sending or receiving a packet, or shutting down the
>>>> adapter, but not when initializing the adapter. Consequently, switching
>>>> between different adapters within the same U-Boot runtime does not work
>>>> correctly since the driver programs the wrong registers during
>>>> rtl8169_eth_start() -> rtl8169_common_start() -> rtl8169_hw_start().
>>>>
>>>> Note that since rtl8169_eth_stop() does set the global variable, the
>>>> second consecutive attempt to use the "new" adapter did work even before
>>>> this patch, because each time network usage is shut down, the network
>>>> core calls stop, which sets the variable so that the next start does
>>>> actually initialize the hardware, and the adapter works.
>>>>
>>>> Equally, rtl8169_eth_probe() calls rtl_init() which sets the global, so
>>>> if using only a single device, or if picking the "right" device (based on
>>>> probe order) when multiple devices are present, ioaddr will already be
>>>> set
>>>> correctly from the get-go, so the issue does not occur.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>
>>>
>>> Looks reasonable to me.
>>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>>
>> Joe,
>>
>> I assume you'll be applying this?
>
> Yes.

I don't see this applied anywhere yet.

[U-Boot] net: rtl8169: fix switching between adapters

[Joe Hershberger]

Hi Stephen,

https://patchwork.ozlabs.org/patch/615309/ was applied to u-boot-net.git.

Thanks!
-Joe