diff mbox series

[net-next,v3,13/19] net: ravb: Set config mode in ndo_open and reset mode in ndo_close

Message ID 20240105082339.1468817-14-claudiu.beznea.uj@bp.renesas.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: ravb: Add suspend to RAM and runtime PM support for RZ/G3S | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1113 this patch: 1113
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1140 this patch: 1140
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1140 this patch: 1140
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Claudiu Jan. 5, 2024, 8:23 a.m. UTC 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

As some IP variants switch to reset mode (and thus registers content is
lost) when setting clocks (due to module standby functionality) to be able
to implement runtime PM and save more power, set the IP's operating mode to
reset at the end of the probe. Along with it, in the ndo_open API the IP
will be switched to configuration, then operation mode. In the ndo_close
API, the IP will be switched back to reset mode. This allows implementing
runtime PM and, along with it, save more power when the IP is not used.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- fixed typos in patch description
- in ravb_probe() switch the hardware to reset mode just after phy
  initialization

Changes in v2:
- none; this patch is new


 drivers/net/ethernet/renesas/ravb_main.c | 78 ++++++++++++++----------
 1 file changed, 46 insertions(+), 32 deletions(-)

Comments

Sergey Shtylyov Jan. 8, 2024, 7:28 p.m. UTC | #1 
On 1/5/24 11:23 AM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> As some IP variants switch to reset mode (and thus registers content is
> lost) when setting clocks (due to module standby functionality) to be able
> to implement runtime PM and save more power, set the IP's operating mode to
> reset at the end of the probe. Along with it, in the ndo_open API the IP
> will be switched to configuration, then operation mode. In the ndo_close
> API, the IP will be switched back to reset mode. This allows implementing
> runtime PM and, along with it, save more power when the IP is not used.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v3:
> - fixed typos in patch description
> - in ravb_probe() switch the hardware to reset mode just after phy
>   initialization
> 
> Changes in v2:
> - none; this patch is new
> 
> 
>  drivers/net/ethernet/renesas/ravb_main.c | 78 ++++++++++++++----------
>  1 file changed, 46 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 1cc1ecd8d6a8..434b4777de5e 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -2746,11 +2755,6 @@ static int ravb_probe(struct platform_device *pdev)
>  	ndev->netdev_ops = &ravb_netdev_ops;
>  	ndev->ethtool_ops = &ravb_ethtool_ops;
>  
> -	/* Set AVB config mode */
> -	error = ravb_set_config_mode(ndev);
> -	if (error)
> -		goto out_rpm_put;
> -
>  	error = ravb_compute_gti(ndev);
>  	if (error)
>  		goto out_rpm_put;
> @@ -2785,13 +2789,23 @@ static int ravb_probe(struct platform_device *pdev)
>  		eth_hw_addr_random(ndev);
>  	}
>  
> +	/* Set config mode as this is needed for PHY initialization. */
> +	error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);

   Hm... don't you need this at laest before calling ravb_read_mac_address()
just above?

> +	if (error)
> +		goto out_rpm_put;
> +
>  	/* MDIO bus init */
>  	error = ravb_mdio_init(priv);
>  	if (error) {
>  		dev_err(&pdev->dev, "failed to initialize MDIO\n");
> -		goto out_dma_free;
> +		goto out_reset_mode;
>  	}
>  
> +	/* Undo previous switch to config opmode. */
> +	error = ravb_set_opmode(ndev, CCC_OPC_RESET);
> +	if (error)
> +		goto out_mdio_release;
> +
>  	netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll);
>  	if (info->nc_queues)
>  		netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll);
[...]

MBR, Sergey
Claudiu Jan. 9, 2024, 5:42 a.m. UTC | #2 
On 08.01.2024 21:28, Sergey Shtylyov wrote:
> On 1/5/24 11:23 AM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> As some IP variants switch to reset mode (and thus registers content is
>> lost) when setting clocks (due to module standby functionality) to be able
>> to implement runtime PM and save more power, set the IP's operating mode to
>> reset at the end of the probe. Along with it, in the ndo_open API the IP
>> will be switched to configuration, then operation mode. In the ndo_close
>> API, the IP will be switched back to reset mode. This allows implementing
>> runtime PM and, along with it, save more power when the IP is not used.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v3:
>> - fixed typos in patch description
>> - in ravb_probe() switch the hardware to reset mode just after phy
>>   initialization
>>
>> Changes in v2:
>> - none; this patch is new
>>
>>
>>  drivers/net/ethernet/renesas/ravb_main.c | 78 ++++++++++++++----------
>>  1 file changed, 46 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index 1cc1ecd8d6a8..434b4777de5e 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
>> @@ -2746,11 +2755,6 @@ static int ravb_probe(struct platform_device *pdev)
>>  	ndev->netdev_ops = &ravb_netdev_ops;
>>  	ndev->ethtool_ops = &ravb_ethtool_ops;
>>  
>> -	/* Set AVB config mode */
>> -	error = ravb_set_config_mode(ndev);
>> -	if (error)
>> -		goto out_rpm_put;
>> -
>>  	error = ravb_compute_gti(ndev);
>>  	if (error)
>>  		goto out_rpm_put;
>> @@ -2785,13 +2789,23 @@ static int ravb_probe(struct platform_device *pdev)
>>  		eth_hw_addr_random(ndev);
>>  	}
>>  
>> +	/* Set config mode as this is needed for PHY initialization. */
>> +	error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
> 
>    Hm... don't you need this at laest before calling ravb_read_mac_address()
> just above?

I asked myself this, haven't experienced issues w/ it while working on this
patch thus I kept it as is. In theory, yes, it should be above that call.
I'll move it there.

Thank you,
Claudiu Beznea

> 
>> +	if (error)
>> +		goto out_rpm_put;
>> +
>>  	/* MDIO bus init */
>>  	error = ravb_mdio_init(priv);
>>  	if (error) {
>>  		dev_err(&pdev->dev, "failed to initialize MDIO\n");
>> -		goto out_dma_free;
>> +		goto out_reset_mode;
>>  	}
>>  
>> +	/* Undo previous switch to config opmode. */
>> +	error = ravb_set_opmode(ndev, CCC_OPC_RESET);
>> +	if (error)
>> +		goto out_mdio_release;
>> +
>>  	netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll);
>>  	if (info->nc_queues)
>>  		netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll);
> [...]
> 
> MBR, Sergey
diff mbox series

Patch

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1cc1ecd8d6a8..434b4777de5e 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1717,6 +1717,27 @@  static const struct ethtool_ops ravb_ethtool_ops = {
 	.set_wol		= ravb_set_wol,
 };
 
+static int ravb_set_config_mode(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+	int error;
+
+	if (info->gptp) {
+		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
+		if (error)
+			return error;
+		/* Set CSEL value */
+		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
+	} else if (info->ccc_gac) {
+		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG | CCC_GAC | CCC_CSEL_HPB);
+	} else {
+		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
+	}
+
+	return error;
+}
+
 static void ravb_set_gti(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -1825,13 +1846,19 @@  static int ravb_open(struct net_device *ndev)
 	if (info->nc_queues)
 		napi_enable(&priv->napi[RAVB_NC]);
 
+	/* Set AVB config mode */
+	error = ravb_set_config_mode(ndev);
+	if (error)
+		goto out_napi_off;
+
 	ravb_set_delay_mode(ndev);
 	ravb_write(ndev, priv->desc_bat_dma, DBAT);
 
 	/* Device init */
 	error = ravb_dmac_init(ndev);
 	if (error)
-		goto out_napi_off;
+		goto out_set_reset;
+
 	ravb_emac_init(ndev);
 
 	ravb_set_gti(ndev);
@@ -1854,6 +1881,8 @@  static int ravb_open(struct net_device *ndev)
 	if (info->gptp || info->ccc_gac)
 		ravb_ptp_stop(ndev);
 	ravb_stop_dma(ndev);
+out_set_reset:
+	ravb_set_opmode(ndev, CCC_OPC_RESET);
 out_napi_off:
 	if (info->nc_queues)
 		napi_disable(&priv->napi[RAVB_NC]);
@@ -2197,7 +2226,8 @@  static int ravb_close(struct net_device *ndev)
 	if (info->nc_queues)
 		ravb_ring_free(ndev, RAVB_NC);
 
-	return 0;
+	/* Set reset mode. */
+	return ravb_set_opmode(ndev, CCC_OPC_RESET);
 }
 
 static int ravb_hwtstamp_get(struct net_device *ndev, struct ifreq *req)
@@ -2527,27 +2557,6 @@  static const struct of_device_id ravb_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, ravb_match_table);
 
-static int ravb_set_config_mode(struct net_device *ndev)
-{
-	struct ravb_private *priv = netdev_priv(ndev);
-	const struct ravb_hw_info *info = priv->info;
-	int error;
-
-	if (info->gptp) {
-		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
-		if (error)
-			return error;
-		/* Set CSEL value */
-		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-	} else if (info->ccc_gac) {
-		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG | CCC_GAC | CCC_CSEL_HPB);
-	} else {
-		error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
-	}
-
-	return error;
-}
-
 static int ravb_setup_irq(struct ravb_private *priv, const char *irq_name,
 			  const char *ch, int *irq, irq_handler_t handler)
 {
@@ -2746,11 +2755,6 @@  static int ravb_probe(struct platform_device *pdev)
 	ndev->netdev_ops = &ravb_netdev_ops;
 	ndev->ethtool_ops = &ravb_ethtool_ops;
 
-	/* Set AVB config mode */
-	error = ravb_set_config_mode(ndev);
-	if (error)
-		goto out_rpm_put;
-
 	error = ravb_compute_gti(ndev);
 	if (error)
 		goto out_rpm_put;
@@ -2785,13 +2789,23 @@  static int ravb_probe(struct platform_device *pdev)
 		eth_hw_addr_random(ndev);
 	}
 
+	/* Set config mode as this is needed for PHY initialization. */
+	error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
+	if (error)
+		goto out_rpm_put;
+
 	/* MDIO bus init */
 	error = ravb_mdio_init(priv);
 	if (error) {
 		dev_err(&pdev->dev, "failed to initialize MDIO\n");
-		goto out_dma_free;
+		goto out_reset_mode;
 	}
 
+	/* Undo previous switch to config opmode. */
+	error = ravb_set_opmode(ndev, CCC_OPC_RESET);
+	if (error)
+		goto out_mdio_release;
+
 	netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll);
 	if (info->nc_queues)
 		netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll);
@@ -2814,8 +2828,10 @@  static int ravb_probe(struct platform_device *pdev)
 		netif_napi_del(&priv->napi[RAVB_NC]);
 
 	netif_napi_del(&priv->napi[RAVB_BE]);
+out_mdio_release:
 	ravb_mdio_release(priv);
-out_dma_free:
+out_reset_mode:
+	ravb_set_opmode(ndev, CCC_OPC_RESET);
 	dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
 			  priv->desc_bat_dma);
 out_rpm_put:
@@ -2846,8 +2862,6 @@  static void ravb_remove(struct platform_device *pdev)
 	dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
 			  priv->desc_bat_dma);
 
-	ravb_set_opmode(ndev, CCC_OPC_RESET);
-
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	clk_unprepare(priv->refclk);