[SRU] [Bionic/OEM-A] [PATCH 0/2] Update Aquantia driver to v4.17 to fix regression

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic/OEM-A] [PATCH 0/2] Update Aquantia driver to v4.17 to fix regression

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1767088

[Impact]
Aquantia ethernet stops working after upgrade Bionic kernel to
4.15.0-20-generic.
This is because I rebased the Aquntia driver to v4.16.

[Fix]
Qoute from the commit log: "To fix this we forcibly change FW state to
idle state before doing the full reset. This makes FW to restore link
state."

[Test]
The user reported the commits fix the issue.

[Regresion Potential]
Low. It's in mainline, also no additional fixes in the -rc2+ cycle,
which means there's no regresion so far.

Igor Russkikh (2):
  net: aquantia: Regression on reset with 1.x firmware
  net: aquantia: oops when shutdown on already stopped device

 drivers/net/ethernet/aquantia/atlantic/aq_nic.c  |  8 +++++---
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c      | 16 ++++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic/OEM-A] [PATCH 1/2] net: aquantia: Regression on reset with 1.x firmware

Kai-Heng Feng
From: Igor Russkikh <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1767088

On ASUS XG-C100C with 1.5.44 firmware a special mode called "dirty wake"
is active. With this mode when motherboard gets powered (but no poweron
happens yet), NIC automatically enables powersave link and watches
for WOL packet.
This normally allows to powerup the PC after AC power failures.

Not all motherboards or bios settings gives power to PCI slots,
so this mode is not enabled on all the hardware.

4.16 linux driver introduced full hardware reset sequence
This is required since before that we had no NIC hardware
reset implemented and there were side effects of "not clean start".

But this full reset is incompatible with "dirty wake" WOL feature
it keeps the PHY link in a special mode forever. As a consequence,
driver sees no link and no traffic.

To fix this we forcibly change FW state to idle state before doing
the full reset. This makes FW to restore link state.

Fixes: c8c82eb net: aquantia: Introduce global AQC hardware reset sequence
Signed-off-by: Igor Russkikh <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit cce96d1883dae4b79f44890e5118243d806da286)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index d3b847ec7465..c58b2c227260 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -48,6 +48,8 @@
 #define FORCE_FLASHLESS 0
 
 static int hw_atl_utils_ver_match(u32 ver_expected, u32 ver_actual);
+static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
+      enum hal_atl_utils_fw_state_e state);
 
 int hw_atl_utils_initfw(struct aq_hw_s *self, const struct aq_fw_ops **fw_ops)
 {
@@ -247,6 +249,20 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
 
  self->rbl_enabled = (boot_exit_code != 0);
 
+ /* FW 1.x may bootup in an invalid POWER state (WOL feature).
+ * We should work around this by forcing its state back to DEINIT
+ */
+ if (!hw_atl_utils_ver_match(HW_ATL_FW_VER_1X,
+    aq_hw_read_reg(self,
+   HW_ATL_MPI_FW_VERSION))) {
+ int err = 0;
+
+ hw_atl_utils_mpi_set_state(self, MPI_DEINIT);
+ AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR) &
+       HW_ATL_MPI_STATE_MSK) == MPI_DEINIT,
+       10, 1000U);
+ }
+
  if (self->rbl_enabled)
  return hw_atl_utils_soft_reset_rbl(self);
  else
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic/OEM-A] [PATCH 2/2] net: aquantia: oops when shutdown on already stopped device

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Igor Russkikh <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1767088

In case netdev is closed at the moment of pci shutdown, aq_nic_stop
gets called second time. napi_disable in that case hangs indefinitely.
In other case, if device was never opened at all, we get oops because
of null pointer access.

We should invoke aq_nic_stop conditionally, only if device is running
at the moment of shutdown.

Reported-by: David Arcari <[hidden email]>
Fixes: 90869ddfefeb ("net: aquantia: Implement pci shutdown callback")
Signed-off-by: Igor Russkikh <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 9a11aff25fd43d5bd2660ababdc9f564b0ba183a)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index c96a92118b8b..32f6d2e24d66 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -951,9 +951,11 @@ void aq_nic_shutdown(struct aq_nic_s *self)
 
  netif_device_detach(self->ndev);
 
- err = aq_nic_stop(self);
- if (err < 0)
- goto err_exit;
+ if (netif_running(self->ndev)) {
+ err = aq_nic_stop(self);
+ if (err < 0)
+ goto err_exit;
+ }
  aq_nic_deinit(self);
 
 err_exit:
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/OEM-A] [PATCH 1/2] net: aquantia: Regression on reset with 1.x firmware

Colin Ian King-2
In reply to this post by Kai-Heng Feng
On 02/05/18 05:59, Kai-Heng Feng wrote:

> From: Igor Russkikh <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1767088
>
> On ASUS XG-C100C with 1.5.44 firmware a special mode called "dirty wake"
> is active. With this mode when motherboard gets powered (but no poweron
> happens yet), NIC automatically enables powersave link and watches
> for WOL packet.
> This normally allows to powerup the PC after AC power failures.
>
> Not all motherboards or bios settings gives power to PCI slots,
> so this mode is not enabled on all the hardware.
>
> 4.16 linux driver introduced full hardware reset sequence
> This is required since before that we had no NIC hardware
> reset implemented and there were side effects of "not clean start".
>
> But this full reset is incompatible with "dirty wake" WOL feature
> it keeps the PHY link in a special mode forever. As a consequence,
> driver sees no link and no traffic.
>
> To fix this we forcibly change FW state to idle state before doing
> the full reset. This makes FW to restore link state.
>
> Fixes: c8c82eb net: aquantia: Introduce global AQC hardware reset sequence
> Signed-off-by: Igor Russkikh <[hidden email]>
> Signed-off-by: David S. Miller <[hidden email]>
> (cherry picked from commit cce96d1883dae4b79f44890e5118243d806da286)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> ---
>  .../aquantia/atlantic/hw_atl/hw_atl_utils.c      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> index d3b847ec7465..c58b2c227260 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> @@ -48,6 +48,8 @@
>  #define FORCE_FLASHLESS 0
>  
>  static int hw_atl_utils_ver_match(u32 ver_expected, u32 ver_actual);
> +static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
> +      enum hal_atl_utils_fw_state_e state);
>  
>  int hw_atl_utils_initfw(struct aq_hw_s *self, const struct aq_fw_ops **fw_ops)
>  {
> @@ -247,6 +249,20 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
>  
>   self->rbl_enabled = (boot_exit_code != 0);
>  
> + /* FW 1.x may bootup in an invalid POWER state (WOL feature).
> + * We should work around this by forcing its state back to DEINIT
> + */
> + if (!hw_atl_utils_ver_match(HW_ATL_FW_VER_1X,
> +    aq_hw_read_reg(self,
> +   HW_ATL_MPI_FW_VERSION))) {
> + int err = 0;
> +
> + hw_atl_utils_mpi_set_state(self, MPI_DEINIT);
> + AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR) &
> +       HW_ATL_MPI_STATE_MSK) == MPI_DEINIT,
> +       10, 1000U);
> + }
> +
>   if (self->rbl_enabled)
>   return hw_atl_utils_soft_reset_rbl(self);
>   else
>
Clean upstream cherry pick.

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/OEM-A] [PATCH 2/2] net: aquantia: oops when shutdown on already stopped device

Colin Ian King-2
In reply to this post by Kai-Heng Feng
On 02/05/18 05:59, Kai-Heng Feng wrote:

> From: Igor Russkikh <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1767088
>
> In case netdev is closed at the moment of pci shutdown, aq_nic_stop
> gets called second time. napi_disable in that case hangs indefinitely.
> In other case, if device was never opened at all, we get oops because
> of null pointer access.
>
> We should invoke aq_nic_stop conditionally, only if device is running
> at the moment of shutdown.
>
> Reported-by: David Arcari <[hidden email]>
> Fixes: 90869ddfefeb ("net: aquantia: Implement pci shutdown callback")
> Signed-off-by: Igor Russkikh <[hidden email]>
> Signed-off-by: David S. Miller <[hidden email]>
> (cherry picked from commit 9a11aff25fd43d5bd2660ababdc9f564b0ba183a)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> ---
>  drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index c96a92118b8b..32f6d2e24d66 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -951,9 +951,11 @@ void aq_nic_shutdown(struct aq_nic_s *self)
>  
>   netif_device_detach(self->ndev);
>  
> - err = aq_nic_stop(self);
> - if (err < 0)
> - goto err_exit;
> + if (netif_running(self->ndev)) {
> + err = aq_nic_stop(self);
> + if (err < 0)
> + goto err_exit;
> + }
>   aq_nic_deinit(self);
>  
>  err_exit:
>
Clean upstream cherry pick.

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/OEM-A] [PATCH 0/2] Update Aquantia driver to v4.17 to fix regression

Kleber Souza
In reply to this post by Kai-Heng Feng
On 05/02/18 06:59, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1767088
>
> [Impact]
> Aquantia ethernet stops working after upgrade Bionic kernel to
> 4.15.0-20-generic.
> This is because I rebased the Aquntia driver to v4.16.
>
> [Fix]
> Qoute from the commit log: "To fix this we forcibly change FW state to
> idle state before doing the full reset. This makes FW to restore link
> state."
>
> [Test]
> The user reported the commits fix the issue.
>
> [Regresion Potential]
> Low. It's in mainline, also no additional fixes in the -rc2+ cycle,
> which means there's no regresion so far.
>
> Igor Russkikh (2):
>   net: aquantia: Regression on reset with 1.x firmware
>   net: aquantia: oops when shutdown on already stopped device
>
>  drivers/net/ethernet/aquantia/atlantic/aq_nic.c  |  8 +++++---
>  .../aquantia/atlantic/hw_atl/hw_atl_utils.c      | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>

Clean cherry-picks, fixing a regression and tested by the reporter.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Applied[B]: [SRU] [Bionic/OEM-A] [PATCH 0/2] Update Aquantia driver to v4.17 to fix regression

Kleber Souza
In reply to this post by Kai-Heng Feng
On 05/02/18 06:59, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1767088
>
> [Impact]
> Aquantia ethernet stops working after upgrade Bionic kernel to
> 4.15.0-20-generic.
> This is because I rebased the Aquntia driver to v4.16.
>
> [Fix]
> Qoute from the commit log: "To fix this we forcibly change FW state to
> idle state before doing the full reset. This makes FW to restore link
> state."
>
> [Test]
> The user reported the commits fix the issue.
>
> [Regresion Potential]
> Low. It's in mainline, also no additional fixes in the -rc2+ cycle,
> which means there's no regresion so far.
>
> Igor Russkikh (2):
>   net: aquantia: Regression on reset with 1.x firmware
>   net: aquantia: oops when shutdown on already stopped device
>
>  drivers/net/ethernet/aquantia/atlantic/aq_nic.c  |  8 +++++---
>  .../aquantia/atlantic/hw_atl/hw_atl_utils.c      | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>

Applied to bionic/master-next branch.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team