mbox series

[0/5,SRU,F] Add r8117 support and fix firmware base issue

Message ID 20200628075842.771536-1-aaron.ma@canonical.com
Headers show
Series Add r8117 support and fix firmware base issue | expand

Message

Aaron Ma June 28, 2020, 7:58 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1885072

[Impact]
Realtek 8117 is failed to be initialized on 5.4- kernel,
even failed to work after loading firmware on 5.5+ kernel.

[Fix]
Add rtl8117 support for 5.4- kernel.
On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
Correct the ocp_base in the driver.
Then it works well.

[Test]
Verified on hardware, link status is OK, iperf test result is good.

[Regression Potential]
Low.
Fixed issue, cherry-picked from linux-next tree.
SRU for OEM-5.6/U was already done. this is for focal because of more
commits.
Bionic kernel set rtl8117 as default r8169. It just works. no need these
changes.

Backport 0002 patch because of another upstream patch is already merged.

Heiner Kallweit (5):
  r8169: add helper r8168g_phy_param
  r8169: add support for RTL8117
  r8169: load firmware for RTL8168fp/RTL8117
  r8169: fix OCP access on RTL8117
  r8169: fix firmware not resetting tp->ocp_base

 drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
 1 file changed, 241 insertions(+), 204 deletions(-)

Comments

Andrea Righi June 29, 2020, 8:23 a.m. UTC | #1
On Sun, Jun 28, 2020 at 03:58:37AM -0400, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072

The descrition in the bug mentions that 5.4 requires additional patches.
I assume these are all the patches that are required. Can you confirm
this?

> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.

Just to make sure, this has been tested both with 5.4 (focal) and a 5.5+
kernel, correct?

> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.

Which commit has been cherry-picked from linux-next? If I see correctly
all the commits in this patch set are all upstream commits.

Thanks,
-Andrea
Aaron Ma June 29, 2020, 8:52 a.m. UTC | #2
On 6/29/20 4:23 PM, Andrea Righi wrote:
> On Sun, Jun 28, 2020 at 03:58:37AM -0400, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> The descrition in the bug mentions that 5.4 requires additional patches.
> I assume these are all the patches that are required. Can you confirm
> this?
> 

Yes. This SRU is for focal.

>>
>> [Impact]
>> Realtek 8117 is failed to be initialized on 5.4- kernel,
>> even failed to work after loading firmware on 5.5+ kernel.
>>
>> [Fix]
>> Add rtl8117 support for 5.4- kernel.
>> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
>> Correct the ocp_base in the driver.
>> Then it works well.
>>
>> [Test]
>> Verified on hardware, link status is OK, iperf test result is good.
> 
> Just to make sure, this has been tested both with 5.4 (focal) and a 5.5+
> kernel, correct?
> 

Yes, all are tested.

>>
>> [Regression Potential]
>> Low.
>> Fixed issue, cherry-picked from linux-next tree.
> 
> Which commit has been cherry-picked from linux-next? If I see correctly
> all the commits in this patch set are all upstream commits.

My mistake, when test the 5th patch is cherry-picked from linux-next,
after test I found that it's already in upstream. I should remove this line.

Thanks,
Aaron

> 
> Thanks,
> -Andrea
>
Stefan Bader June 29, 2020, 9:29 a.m. UTC | #3
On 28.06.20 09:58, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.
> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.
> SRU for OEM-5.6/U was already done. this is for focal because of more
> commits.
> Bionic kernel set rtl8117 as default r8169. It just works. no need these
> changes.
> 
> Backport 0002 patch because of another upstream patch is already merged.
> 
> Heiner Kallweit (5):
>   r8169: add helper r8168g_phy_param
>   r8169: add support for RTL8117
>   r8169: load firmware for RTL8168fp/RTL8117
>   r8169: fix OCP access on RTL8117
>   r8169: fix firmware not resetting tp->ocp_base
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>  1 file changed, 241 insertions(+), 204 deletions(-)
> 

For the last 3 patches, I cannot without doubt say that this would not also have
some effect on some of the other chips which the r8169 driver supports. Was
there any regression testing on any hw beside the actual target platform?

-Stefan
Aaron Ma June 29, 2020, 10:23 a.m. UTC | #4
On 6/29/20 5:29 PM, Stefan Bader wrote:
> On 28.06.20 09:58, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1885072
>>
>> [Impact]
>> Realtek 8117 is failed to be initialized on 5.4- kernel,
>> even failed to work after loading firmware on 5.5+ kernel.
>>
>> [Fix]
>> Add rtl8117 support for 5.4- kernel.
>> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
>> Correct the ocp_base in the driver.
>> Then it works well.
>>
>> [Test]
>> Verified on hardware, link status is OK, iperf test result is good.
>>
>> [Regression Potential]
>> Low.
>> Fixed issue, cherry-picked from linux-next tree.
>> SRU for OEM-5.6/U was already done. this is for focal because of more
>> commits.
>> Bionic kernel set rtl8117 as default r8169. It just works. no need these
>> changes.
>>
>> Backport 0002 patch because of another upstream patch is already merged.
>>
>> Heiner Kallweit (5):
>>   r8169: add helper r8168g_phy_param
>>   r8169: add support for RTL8117
>>   r8169: load firmware for RTL8168fp/RTL8117
>>   r8169: fix OCP access on RTL8117
>>   r8169: fix firmware not resetting tp->ocp_base
>>
>>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>>  1 file changed, 241 insertions(+), 204 deletions(-)
>>
> 
> For the last 3 patches, I cannot without doubt say that this would not also have
> some effect on some of the other chips which the r8169 driver supports. Was
> there any regression testing on any hw beside the actual target platform?
> 

Yes, tested on two other hw with RTL8168ep/8111ep.

Aaron

> -Stefan
>
Stefan Bader June 29, 2020, 11:27 a.m. UTC | #5
On 28.06.20 09:58, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.
> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.
> SRU for OEM-5.6/U was already done. this is for focal because of more
> commits.
> Bionic kernel set rtl8117 as default r8169. It just works. no need these
> changes.
> 
> Backport 0002 patch because of another upstream patch is already merged.
> 
> Heiner Kallweit (5):
>   r8169: add helper r8168g_phy_param
>   r8169: add support for RTL8117
>   r8169: load firmware for RTL8168fp/RTL8117
>   r8169: fix OCP access on RTL8117
>   r8169: fix firmware not resetting tp->ocp_base
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>  1 file changed, 241 insertions(+), 204 deletions(-)
> 
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Colin King June 29, 2020, 11:30 a.m. UTC | #6
On 28/06/2020 08:58, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.
> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.
> SRU for OEM-5.6/U was already done. this is for focal because of more
> commits.
> Bionic kernel set rtl8117 as default r8169. It just works. no need these
> changes.
> 
> Backport 0002 patch because of another upstream patch is already merged.
> 
> Heiner Kallweit (5):
>   r8169: add helper r8168g_phy_param
>   r8169: add support for RTL8117
>   r8169: load firmware for RTL8168fp/RTL8117
>   r8169: fix OCP access on RTL8117
>   r8169: fix firmware not resetting tp->ocp_base
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>  1 file changed, 241 insertions(+), 204 deletions(-)
> 
Given the positive replies on the mailing list concerning testing:

Acked-by: Colin Ian King <colin.king@canonical.com>
Kamal Mostafa June 29, 2020, 9:33 p.m. UTC | #7
LGTM.

Acked-by: Kamal Mostafa <kamal@canonical.com>

 -Kamal

On Sun, Jun 28, 2020 at 03:58:37AM -0400, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.
> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.
> SRU for OEM-5.6/U was already done. this is for focal because of more
> commits.
> Bionic kernel set rtl8117 as default r8169. It just works. no need these
> changes.
> 
> Backport 0002 patch because of another upstream patch is already merged.
> 
> Heiner Kallweit (5):
>   r8169: add helper r8168g_phy_param
>   r8169: add support for RTL8117
>   r8169: load firmware for RTL8168fp/RTL8117
>   r8169: fix OCP access on RTL8117
>   r8169: fix firmware not resetting tp->ocp_base
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>  1 file changed, 241 insertions(+), 204 deletions(-)
> 
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Kelsey Skunberg Aug. 3, 2020, 8:19 p.m. UTC | #8
Applied to focal/master-next. Thank you!

-Kelsey

On 2020-06-28 03:58:37 , Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1885072
> 
> [Impact]
> Realtek 8117 is failed to be initialized on 5.4- kernel,
> even failed to work after loading firmware on 5.5+ kernel.
> 
> [Fix]
> Add rtl8117 support for 5.4- kernel.
> On 5.5+ kernel, firmware should reset ocp_base, but not on r8117.
> Correct the ocp_base in the driver.
> Then it works well.
> 
> [Test]
> Verified on hardware, link status is OK, iperf test result is good.
> 
> [Regression Potential]
> Low.
> Fixed issue, cherry-picked from linux-next tree.
> SRU for OEM-5.6/U was already done. this is for focal because of more
> commits.
> Bionic kernel set rtl8117 as default r8169. It just works. no need these
> changes.
> 
> Backport 0002 patch because of another upstream patch is already merged.
> 
> Heiner Kallweit (5):
>   r8169: add helper r8168g_phy_param
>   r8169: add support for RTL8117
>   r8169: load firmware for RTL8168fp/RTL8117
>   r8169: fix OCP access on RTL8117
>   r8169: fix firmware not resetting tp->ocp_base
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 445 ++++++++++++----------
>  1 file changed, 241 insertions(+), 204 deletions(-)
> 
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team