Re: [PATCH net-next] r8169: remove not needed PHY soft reset in rtl8168e_2_hw_phy_config

[David Miller]

From: Heiner Kallweit 
Date: Wed, 21 Feb 2018 07:29:24 +0100

> Am 21.02.2018 um 05:27 schrieb David Miller:
>> From: Heiner Kallweit 
>> Date: Tue, 20 Feb 2018 07:30:16 +0100
>> 
>>> rtl8169_init_phy() resets the PHY anyway after applying the chip-specific
>>> PHY configuration. So we don't need to soft-reset the PHY as part of the
>>> chip-specific configuration.
>>>
>>> Signed-off-by: Heiner Kallweit 
>> 
>> I'm not so comfortable with this.
>> 
>> There are so many r8169 chip variants out there.
>> 
>> And who knows, maybe one of them needs this second PHY reset or due to
>> some way the driver is coded it is necessary.
>> 
>> Unless you can test this change on every r8169 chip type, I'm very
>> reluctant to apply this patch.
>> 
> I understand the concern, the change however is in rtl8168e_2_hw_phy_config()
> which is specific to chip version 34 (RTL8168evl).
> On my system with this chip version the change didn't change system behavior.

Ok, that does alleviate my concerns.

Patch applied, thank you.

Re: [PATCH net-next] r8169: remove not needed PHY soft reset in rtl8168e_2_hw_phy_config

[Heiner Kallweit]

Am 21.02.2018 um 05:27 schrieb David Miller:
> From: Heiner Kallweit 
> Date: Tue, 20 Feb 2018 07:30:16 +0100
> 
>> rtl8169_init_phy() resets the PHY anyway after applying the chip-specific
>> PHY configuration. So we don't need to soft-reset the PHY as part of the
>> chip-specific configuration.
>>
>> Signed-off-by: Heiner Kallweit 
> 
> I'm not so comfortable with this.
> 
> There are so many r8169 chip variants out there.
> 
> And who knows, maybe one of them needs this second PHY reset or due to
> some way the driver is coded it is necessary.
> 
> Unless you can test this change on every r8169 chip type, I'm very
> reluctant to apply this patch.
> 
I understand the concern, the change however is in rtl8168e_2_hw_phy_config()
which is specific to chip version 34 (RTL8168evl).
On my system with this chip version the change didn't change system behavior.


> Sorry.
> 

Re: [PATCH net-next] r8169: remove not needed PHY soft reset in rtl8168e_2_hw_phy_config

[David Miller]

From: Heiner Kallweit 
Date: Tue, 20 Feb 2018 07:30:16 +0100

> rtl8169_init_phy() resets the PHY anyway after applying the chip-specific
> PHY configuration. So we don't need to soft-reset the PHY as part of the
> chip-specific configuration.
> 
> Signed-off-by: Heiner Kallweit 

I'm not so comfortable with this.

There are so many r8169 chip variants out there.

And who knows, maybe one of them needs this second PHY reset or due to
some way the driver is coded it is necessary.

Unless you can test this change on every r8169 chip type, I'm very
reluctant to apply this patch.

Sorry.

[PATCH net-next] r8169: remove not needed PHY soft reset in rtl8168e_2_hw_phy_config

[Heiner Kallweit]

rtl8169_init_phy() resets the PHY anyway after applying the chip-specific
PHY configuration. So we don't need to soft-reset the PHY as part of the
chip-specific configuration.

Signed-off-by: Heiner Kallweit 
---
 drivers/net/ethernet/realtek/r8169.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c 
b/drivers/net/ethernet/realtek/r8169.c
index 0bf7d1759..92b50b688 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -3805,8 +3805,6 @@ static void rtl8168e_2_hw_phy_config(struct 
rtl8169_private *tp)
rtl_writephy(tp, 0x1f, 0x0005);
rtl_w0w1_phy(tp, 0x01, 0x0100, 0x);
rtl_writephy(tp, 0x1f, 0x);
-   /* soft-reset phy */
-   rtl_writephy(tp, MII_BMCR, BMCR_RESET | BMCR_ANENABLE | BMCR_ANRESTART);
 
/* Broken BIOS workaround: feed GigaMAC registers with MAC address. */
rtl_rar_exgmac_set(tp, tp->dev->dev_addr);
-- 
2.16.1