Received-SPF: pass (google.com: domain of linux-kernel+bounces-59709-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Biju Das <biju.das.jz@bp.renesas.com>
To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>, "s.shtylyov@omp.ru"
	<s.shtylyov@omp.ru>, "davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>, "kuba@kernel.org"
	<kuba@kernel.org>, "pabeni@redhat.com" <pabeni@redhat.com>
CC: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Claudiu.Beznea
	<claudiu.beznea@tuxon.dev>, Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Subject: RE: [PATCH net-next v2 4/5] net: ravb: Do not apply RX checksum
 settings to hardware if the interface is down
Thread-Topic: [PATCH net-next v2 4/5] net: ravb: Do not apply RX checksum
 settings to hardware if the interface is down
Thread-Index: AQHaW3pX9CB/Daj1+EissME+82JRZbECPfNg
Date: Fri, 9 Feb 2024 17:11:54 +0000
Message-ID:
 <TYCPR01MB1126949DF3F7C498956AAB629864B2@TYCPR01MB11269.jpnprd01.prod.outlook.com>
References: <20240209170459.4143861-1-claudiu.beznea.uj@bp.renesas.com>
 <20240209170459.4143861-5-claudiu.beznea.uj@bp.renesas.com>
In-Reply-To: <20240209170459.4143861-5-claudiu.beznea.uj@bp.renesas.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?qvYWf12eBV8VZ20bmvWb41PjRfZfNNIhTfV69ycJ26lzlSGftq0G8xukx41E?=
 =?us-ascii?Q?UXIDNWOolCTQF2dmfgLKr7Qu65VxjVRba1EMs7OrUjKFQ5MoztbZR2tGwctK?=
 =?us-ascii?Q?OnwQDEsFgCaAxSX24YOmCxPfvza3SFfJEa7jDzzVwuwbP/MS0NoH6KBQW7VB?=
 =?us-ascii?Q?EE7YFFZ6uFgWfDncksHjAk1RtWHnqWmMnzDrb5L2YARG9Nlnmxu5IJQJ+ml6?=
 =?us-ascii?Q?DyQ4qa6edO0A4PBE9Nq0xeMVabVd4/sW0QETM9Ht4BiaQbHOEnRgRa1BKKC4?=
 =?us-ascii?Q?Xghz49NHjAe0ZFtZsP6v6hf2vKmJmfVkJTsTP/7GHXlZRfjIKjOmYLdjmrWX?=
 =?us-ascii?Q?gOGu8SIkwK/ZqlE1+A7XVqkRbrf9TYNN2KxiT7wNYP6lFE4B5gwSCFeXsU0k?=
 =?us-ascii?Q?nYzJ+e2355i/4SzR2PBRvfATTKLJQupITb09/2H+jfKy9oo/IhwvwtifiJOj?=
 =?us-ascii?Q?9A1o9PmWAlNHSZRr//U1OReekmmxkHWiTjFG2KRQq8z06764Dh9wMGJE7jeD?=
 =?us-ascii?Q?VkkbEeWtFz6LzrL5lJQMrf+8OQr66vzPGnFPH6kdF+v5es3nihhep/5W+0sc?=
 =?us-ascii?Q?4fVUo4c7kCgM8NJmcXu0H1XGZhKjIOHDVqoMKnZm1HSPhEVoYb7fR4ljstqa?=
 =?us-ascii?Q?uG/lEUZksPbvMd2R9w2y8Y/d7hAT4wch/Hguy8m2iPCHTHYkqoXbYTTQ165I?=
 =?us-ascii?Q?bFp3TzIi94bWSZx861sdH7mz593ZhBRTS9Lw5OVgoR0ePkDUwkxOyJPlnZbB?=
 =?us-ascii?Q?/D9MiHQRdTNYLSDxHOAoMOwlguML0+1t+pvD2Lj0VoMZWrlW0jYah71WmmiY?=
 =?us-ascii?Q?3URu/Pb+PS63dfk2OiZDmErqwhczdSRUHN5fmxUzqZzej4mCmcFqUK2MQEXt?=
 =?us-ascii?Q?iD9zrDC8N8koConUmpRhYtkI13DipMdUTKqcEx/cv3Rsdn+B5Hb75cAxnp4x?=
 =?us-ascii?Q?fw4moJOQRpbg2jtAfpa/TAE+raNt25reyiYW6DM4tZeZAqo+lnpdtsjMUX/f?=
 =?us-ascii?Q?4Rd+snYSoTK+W++JVXXe6lqVA4bAy3UhLNASuA3yeJmvTl7Cz6vVFWSBqCWN?=
 =?us-ascii?Q?5CCKkZMcJwLuj7kEp7BM//GKZG+wCT4ttUAj63Fu1m6I4FSwOdAokZa3+KoH?=
 =?us-ascii?Q?bTZHAZezs8Tt7wNVa4Km7mQA6vGu95cQyqOnCfWss1KnNm5Ov4Apwj3W6tBB?=
 =?us-ascii?Q?R6f30oYcaxCxNoNsC8lfxYjAeD0gQqSrDA8U6gJ9lQqdeB7XZrPZf8jaYSDN?=
 =?us-ascii?Q?KieJlqI8JdsB+qiC/gWBsmw5iRoBwpmm0w4cDfFhYeW67Pm9m6fBNAbRF0rN?=
 =?us-ascii?Q?6vrdWdIJMf+d//j/RsdkFZ8ewJs3a4tYjJzJXkfWE2zRzDaz6WhM8IbognjO?=
 =?us-ascii?Q?9SVsFBuBvfKtrd2M0vZdWivCTGoCwbX7VRug1pwm/6TfH1DTb7kUgZr9c+Ol?=
 =?us-ascii?Q?pUoBTRjn1QAkDrGd5SIWOhQTEDv++ylFofvmS3GyPEdyiYusi7X3Py2My3z5?=
 =?us-ascii?Q?A4dOAvR4LCoExqzMJySPLC9nBAYAZ47wUd/nPvoppGaDb7eAfHGDAk9HHHBd?=
 =?us-ascii?Q?wN7ucdDI5grq4/r6vo8EK8Dd567T5YxVNdN478PvD98PsQw61AWO+j0scPAS?=
 =?us-ascii?Q?GA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: bp.renesas.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: TYCPR01MB11269.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a991c6b7-6440-4c49-6e47-08dc29923752
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2024 17:11:54.9729
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: HjupWeFC6bYVH/1JHkpQXaa42KEem1Bril2lMrOJb0qHJC6Nmw03AdRFYUCCCTmeo24GcrlhKxmMwrf7e2P1fzNLEcOzgp604k4h/u+0WAw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY3PR01MB10598

Hi Claudiu Beznea,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: Friday, February 9, 2024 5:05 PM
> Subject: [PATCH net-next v2 4/5] net: ravb: Do not apply RX checksum
> settings to hardware if the interface is down
>=20
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>=20
> Do not apply the RX checksum settings to hardware if the interface is
> down.
> In case runtime PM is enabled, and while the interface is down, the IP
> will be in reset mode (as for some platforms disabling the clocks will
> switch the IP to reset mode, which will lead to losing register contents)
> and applying settings in reset mode is not an option. Instead, cache the
> RX checksum settings and apply them in ravb_open() through
> ravb_emac_init().
> This has been solved by introducing pm_runtime_active() check. The device
> runtime PM usage counter has been incremented to avoid disabling the
> device clocks while the check is in progress (if any).
>=20
> Commit prepares for the addition of runtime PM.
>=20
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>=20
> Changes in v2:
> - fixed typo in patch description
> - adjusted ravb_set_features_gbeth(); didn't collect the Sergey's Rb
>   tag due to this
>=20
> Changes since [2]:
> - use pm_runtime_get_noresume() and pm_runtime_active() and updated the
>   commit message to describe that
> - fixed typos
> - s/CSUM/checksum in patch title and description
>=20
> Changes in v3 of [2]:
> - this was patch 20/21 in v2
> - fixed typos in patch description
> - removed code from ravb_open()
> - use ndev->flags & IFF_UP checks instead of netif_running()
>=20
> Changes in v2 of [2]:
> - none; this patch is new
>=20
> [2]
>=20
>  drivers/net/ethernet/renesas/ravb_main.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>=20
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> b/drivers/net/ethernet/renesas/ravb_main.c
> index 7a7f743a1fef..f4be08f0198d 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2478,8 +2478,14 @@ static int ravb_change_mtu(struct net_device *ndev=
,
> int new_mtu)  static void ravb_set_rx_csum(struct net_device *ndev, bool
> enable)  {
>  	struct ravb_private *priv =3D netdev_priv(ndev);
> +	struct device *dev =3D &priv->pdev->dev;
>  	unsigned long flags;
>=20
> +	pm_runtime_get_noresume(dev);
> +
> +	if (!pm_runtime_active(dev))
> +		goto out_rpm_put;


Thanks for the patch,

Why can't this be handled in ravb_set_features() to avoid code
duplication??

Cheers,
Biju

> +
>  	spin_lock_irqsave(&priv->lock, flags);
>=20
>  	/* Disable TX and RX */
> @@ -2492,6 +2498,9 @@ static void ravb_set_rx_csum(struct net_device
> *ndev, bool enable)
>  	ravb_rcv_snd_enable(ndev);
>=20
>  	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +out_rpm_put:
> +	pm_runtime_put_noidle(dev);
>  }
>=20
>  static int ravb_endisable_csum_gbeth(struct net_device *ndev, enum
> ravb_reg reg, @@ -2515,10 +2524,16 @@ static int
> ravb_set_features_gbeth(struct net_device *ndev,  {
>  	netdev_features_t changed =3D ndev->features ^ features;
>  	struct ravb_private *priv =3D netdev_priv(ndev);
> +	struct device *dev =3D &priv->pdev->dev;
>  	unsigned long flags;
>  	int ret =3D 0;
>  	u32 val;
>=20
> +	pm_runtime_get_noresume(dev);
> +
> +	if (!pm_runtime_active(dev))
> +		goto out_rpm_put;
> +
>  	spin_lock_irqsave(&priv->lock, flags);
>  	if (changed & NETIF_F_RXCSUM) {
>  		if (features & NETIF_F_RXCSUM)
> @@ -2542,9 +2557,12 @@ static int ravb_set_features_gbeth(struct
> net_device *ndev,
>  			goto done;
>  	}
>=20
> -	ndev->features =3D features;
>  done:
>  	spin_unlock_irqrestore(&priv->lock, flags);
> +out_rpm_put:
> +	pm_runtime_put_noidle(dev);
> +	if (!ret)
> +		ndev->features =3D features;
>=20
>  	return ret;
>  }
> --
> 2.39.2
>=20