Re: [PATCH v1 net-next] net: stmmac: Enable EEE HW LPI timer with auto SW/HW switching

2020-10-12 Thread Jakub Kicinski 
On Fri,  9 Oct 2020 00:11:23 +0800 Voon Weifeng wrote:
> From: "Vineetha G. Jaya Kumaran" 
> 
> This patch enables the HW LPI Timer which controls the automatic entry
> and exit of the LPI state.
> The EEE LPI timer value is configured through ethtool. The driver will
> auto select the LPI HW timer if the value in the HW timer supported range.
> Else, the driver will fallback to SW timer.
> 
> Signed-off-by: Vineetha G. Jaya Kumaran 
> Signed-off-by: Voon Weifeng 

minor nits, but the patch makes sense to me
please repost soon so it makes it into 5.10

> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
> b/drivers/net/ethernet/stmicro/stmmac/common.h
> index df7de50497a0..f59c4a1c1674 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -402,6 +402,9 @@ struct dma_features {
>  /* Default LPI timers */
>  #define STMMAC_DEFAULT_LIT_LS0x3E8
>  #define STMMAC_DEFAULT_TWT_LS0x1E
> +#define STMMAC_ET_MAX0xF
> +#define LPI_ET_ENABLE1
> +#define LPI_ET_DISABLE   0

Don't think you need defines for true / false values like that.
Just use literals.

>  
>  #define STMMAC_CHAIN_MODE0x1
>  #define STMMAC_RING_MODE 0x2

> @@ -268,6 +269,7 @@ int stmmac_dvr_probe(struct device *device,
>struct stmmac_resources *res);
>  void stmmac_disable_eee_mode(struct stmmac_priv *priv);
>  bool stmmac_eee_init(struct stmmac_priv *priv);
> +void stmmac_lpi_entry_timer_enable(struct stmmac_priv *priv, bool en);

This doesn't have to be declared in the header...

>  int stmmac_reinit_queues(struct net_device *dev, u32 rx_cnt, u32 tx_cnt);
>  int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 
> tx_size);
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 220626a8d499..908df3cb12cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -327,6 +327,11 @@ static void stmmac_enable_eee_mode(struct stmmac_priv 
> *priv)
>   */
>  void stmmac_disable_eee_mode(struct stmmac_priv *priv)
>  {
> + if (!priv->eee_sw_timer_en) {
> + stmmac_lpi_entry_timer_enable(priv, LPI_ET_DISABLE);
> + return;
> + }
> +
>   stmmac_reset_eee_mode(priv, priv->hw);
>   del_timer_sync(&priv->eee_ctrl_timer);
>   priv->tx_path_in_lpi_mode = false;
> @@ -347,6 +352,16 @@ static void stmmac_eee_ctrl_timer(struct timer_list *t)
>   mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(priv->tx_lpi_timer));
>  }
>  
> +void stmmac_lpi_entry_timer_enable(struct stmmac_priv *priv, bool en)
> +{

Just move this function up in the file so it's defined before
stmmac_disable_eee_mode(). We prefer to limit forward declarations 
in the kernel.

Also the name of the function would probably be better as
..._timer_config() or such. I find it strange to call a function called
enable() to disable something.

> + int tx_lpi_timer;
> +
> + /* Clear/set the SW EEE timer flag based on LPI ET enablement */
> + priv->eee_sw_timer_en = en ? 0 : 1;
> + tx_lpi_timer  = en ? priv->tx_lpi_timer : 0;
> + stmmac_set_eee_lpi_timer(priv, priv->hw, tx_lpi_timer);
> +}

RE: [PATCH v1 net-next] net: stmmac: Enable EEE HW LPI timer with auto SW/HW switching

2020-10-08 Thread Voon, Weifeng 
> Subject: [PATCH v1 net-next] net: stmmac: Enable EEE HW LPI timer with auto
> SW/HW switching
> 
> From: "Vineetha G. Jaya Kumaran" 
> 
> This patch enables the HW LPI Timer which controls the automatic entry and
> exit of the LPI state.
> The EEE LPI timer value is configured through ethtool. The driver will auto
> select the LPI HW timer if the value in the HW timer supported range.
> Else, the driver will fallback to SW timer.
> 
> Signed-off-by: Vineetha G. Jaya Kumaran 
> Signed-off-by: Voon Weifeng 

Please help to review and comment. Thanks. 
Weifeng

[PATCH v1 net-next] net: stmmac: Enable EEE HW LPI timer with auto SW/HW switching

2020-10-08 Thread Voon Weifeng 
From: "Vineetha G. Jaya Kumaran" 

This patch enables the HW LPI Timer which controls the automatic entry
and exit of the LPI state.
The EEE LPI timer value is configured through ethtool. The driver will
auto select the LPI HW timer if the value in the HW timer supported range.
Else, the driver will fallback to SW timer.

Signed-off-by: Vineetha G. Jaya Kumaran 
Signed-off-by: Voon Weifeng 
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  3 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  2 ++
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 24 ++
 drivers/net/ethernet/stmicro/stmmac/hwif.h|  3 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +--
 6 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index df7de50497a0..f59c4a1c1674 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -402,6 +402,9 @@ struct dma_features {
 /* Default LPI timers */
 #define STMMAC_DEFAULT_LIT_LS  0x3E8
 #define STMMAC_DEFAULT_TWT_LS  0x1E
+#define STMMAC_ET_MAX  0xF
+#define LPI_ET_ENABLE  1
+#define LPI_ET_DISABLE 0
 
 #define STMMAC_CHAIN_MODE  0x1
 #define STMMAC_RING_MODE   0x2
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 592b043f9676..82df91c130f7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -176,9 +176,11 @@ enum power_event {
  */
 #define GMAC4_LPI_CTRL_STATUS  0xd0
 #define GMAC4_LPI_TIMER_CTRL   0xd4
+#define GMAC4_LPI_ENTRY_TIMER  0xd8
 
 /* LPI control and status defines */
 #define GMAC4_LPI_CTRL_STATUS_LPITCSE  BIT(21) /* LPI Tx Clock Stop Enable */
+#define GMAC4_LPI_CTRL_STATUS_LPIATE   BIT(20) /* LPI Timer Enable */
 #define GMAC4_LPI_CTRL_STATUS_LPITXA   BIT(19) /* Enable LPI TX Automate */
 #define GMAC4_LPI_CTRL_STATUS_PLS  BIT(17) /* PHY Link Status */
 #define GMAC4_LPI_CTRL_STATUS_LPIENBIT(16) /* LPI Enable */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 002791b77356..3ed4f4cda7f9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -379,6 +379,27 @@ static void dwmac4_set_eee_pls(struct mac_device_info *hw, 
int link)
writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
 }
 
+static void dwmac4_set_eee_lpi_entry_timer(struct mac_device_info *hw, int et)
+{
+   void __iomem *ioaddr = hw->pcsr;
+   int value = et & STMMAC_ET_MAX;
+   int regval;
+
+   /* Program LPI entry timer value into register */
+   writel(value, ioaddr + GMAC4_LPI_ENTRY_TIMER);
+
+   /* Enable/disable LPI entry timer */
+   regval = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
+   regval |= GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA;
+
+   if (et)
+   regval |= GMAC4_LPI_CTRL_STATUS_LPIATE;
+   else
+   regval &= ~GMAC4_LPI_CTRL_STATUS_LPIATE;
+
+   writel(regval, ioaddr + GMAC4_LPI_CTRL_STATUS);
+}
+
 static void dwmac4_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
 {
void __iomem *ioaddr = hw->pcsr;
@@ -1164,6 +1185,7 @@ const struct stmmac_ops dwmac4_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
@@ -1206,6 +1228,7 @@ const struct stmmac_ops dwmac410_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
@@ -1249,6 +1272,7 @@ const struct stmmac_ops dwmac510_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h 
b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index e2dca9b6e992..b40b2e0667bb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -337,6 +337,7 @@ struct stmmac_ops {
void (*set_eee_mode)(struct mac_device_info *hw,
 bool en_tx_lp

[PATCH v1 net-next] net: stmmac: Enable EEE HW LPI timer with auto SW/HW switching

2020-10-06 Thread Voon Weifeng 
From: "Vineetha G. Jaya Kumaran" 

This patch enables the HW LPI Timer which controls the automatic entry
and exit of the LPI state.
The EEE LPI timer value is configured through ethtool. The driver will
auto select the LPI HW timer if the value in the HW timer supported range.
Else, the driver will fallback to SW timer.

Signed-off-by: Vineetha G. Jaya Kumaran 
Signed-off-by: Voon Weifeng 
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  3 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  2 ++
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 24 ++
 drivers/net/ethernet/stmicro/stmmac/hwif.h|  3 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +--
 6 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index df7de50497a0..f59c4a1c1674 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -402,6 +402,9 @@ struct dma_features {
 /* Default LPI timers */
 #define STMMAC_DEFAULT_LIT_LS  0x3E8
 #define STMMAC_DEFAULT_TWT_LS  0x1E
+#define STMMAC_ET_MAX  0xF
+#define LPI_ET_ENABLE  1
+#define LPI_ET_DISABLE 0
 
 #define STMMAC_CHAIN_MODE  0x1
 #define STMMAC_RING_MODE   0x2
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 592b043f9676..82df91c130f7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -176,9 +176,11 @@ enum power_event {
  */
 #define GMAC4_LPI_CTRL_STATUS  0xd0
 #define GMAC4_LPI_TIMER_CTRL   0xd4
+#define GMAC4_LPI_ENTRY_TIMER  0xd8
 
 /* LPI control and status defines */
 #define GMAC4_LPI_CTRL_STATUS_LPITCSE  BIT(21) /* LPI Tx Clock Stop Enable */
+#define GMAC4_LPI_CTRL_STATUS_LPIATE   BIT(20) /* LPI Timer Enable */
 #define GMAC4_LPI_CTRL_STATUS_LPITXA   BIT(19) /* Enable LPI TX Automate */
 #define GMAC4_LPI_CTRL_STATUS_PLS  BIT(17) /* PHY Link Status */
 #define GMAC4_LPI_CTRL_STATUS_LPIENBIT(16) /* LPI Enable */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 002791b77356..3ed4f4cda7f9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -379,6 +379,27 @@ static void dwmac4_set_eee_pls(struct mac_device_info *hw, 
int link)
writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
 }
 
+static void dwmac4_set_eee_lpi_entry_timer(struct mac_device_info *hw, int et)
+{
+   void __iomem *ioaddr = hw->pcsr;
+   int value = et & STMMAC_ET_MAX;
+   int regval;
+
+   /* Program LPI entry timer value into register */
+   writel(value, ioaddr + GMAC4_LPI_ENTRY_TIMER);
+
+   /* Enable/disable LPI entry timer */
+   regval = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
+   regval |= GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA;
+
+   if (et)
+   regval |= GMAC4_LPI_CTRL_STATUS_LPIATE;
+   else
+   regval &= ~GMAC4_LPI_CTRL_STATUS_LPIATE;
+
+   writel(regval, ioaddr + GMAC4_LPI_CTRL_STATUS);
+}
+
 static void dwmac4_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
 {
void __iomem *ioaddr = hw->pcsr;
@@ -1164,6 +1185,7 @@ const struct stmmac_ops dwmac4_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
@@ -1206,6 +1228,7 @@ const struct stmmac_ops dwmac410_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
@@ -1249,6 +1272,7 @@ const struct stmmac_ops dwmac510_ops = {
.get_umac_addr = dwmac4_get_umac_addr,
.set_eee_mode = dwmac4_set_eee_mode,
.reset_eee_mode = dwmac4_reset_eee_mode,
+   .set_eee_lpi_entry_timer = dwmac4_set_eee_lpi_entry_timer,
.set_eee_timer = dwmac4_set_eee_timer,
.set_eee_pls = dwmac4_set_eee_pls,
.pcs_ctrl_ane = dwmac4_ctrl_ane,
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h 
b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index e2dca9b6e992..b40b2e0667bb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -337,6 +337,7 @@ struct stmmac_ops {
void (*set_eee_mode)(struct mac_device_info *hw,
 bool en_tx_lp