Received-SPF: pass (google.com: domain of linux-kernel+bounces-63322-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
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 v3 5/6] net: ravb: Do not apply features to
 hardware if the interface is down
Thread-Topic: [PATCH net-next v3 5/6] net: ravb: Do not apply features to
 hardware if the interface is down
Thread-Index: AQHaXmDfj3p/d+D+vE6GR/WgPunNWLEICrVg
Date: Tue, 13 Feb 2024 10:13:16 +0000
Message-ID:
 <TYCPR01MB112698DE07AAA9C535776805D864F2@TYCPR01MB11269.jpnprd01.prod.outlook.com>
References: <20240213094110.853155-1-claudiu.beznea.uj@bp.renesas.com>
 <20240213094110.853155-6-claudiu.beznea.uj@bp.renesas.com>
In-Reply-To: <20240213094110.853155-6-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?F6EkvkeoCR/NArjYagnS32QVW8sQnpfrnR7k/pV4ZNX6KyHcDVaB30rad+Nm?=
 =?us-ascii?Q?f3nCFaPql8+kgStrtU6WV/835AtGPLOHwgHzl6aDO6dFdrONBqHiTgBmFkqt?=
 =?us-ascii?Q?z1poW9NYGFDmiAdFDfWQJEOzGzMIFTFqzsei989HsKYs8NsruULSJpzJWEdD?=
 =?us-ascii?Q?hLUX/I9Hf8osFrjk138KhUITSWT1wvoUgwrcw12U4nQLSQT0DwdjpMCDgGPc?=
 =?us-ascii?Q?HxsVx0eAPIlXFmP56GB/6gjmMj9yhphyzVA9Iv0bkjOtH+YU9sFr9p+UJIXY?=
 =?us-ascii?Q?hRm1/am3zk3kDNXCwoBgYoxa573j02580nQSio1UoCWOPNbgHkUR1ghXFQdb?=
 =?us-ascii?Q?rCMcOjMBff9h0FlqW4ujknXN8VZaht6tmtxN9wan9UnLo4FiGdCHj8fcnX5n?=
 =?us-ascii?Q?wHCPXfuFvRXU8BXXMVniJkEGqUYBky4auP115H4qY0tWwJhz+vCPvUOk2ZR2?=
 =?us-ascii?Q?/EHggeCsUu2MyA0XwF3O05Bf4nNG3CkTaFmwuovg9K1ym5oH6+QM/FqigngP?=
 =?us-ascii?Q?HUEGSy7lpw7wUzOodYT7eTjS6xAF4QLJdw+1sf37sYPx+0RaI6EOMpO1+Dtl?=
 =?us-ascii?Q?Ea66N3p33b7HvmOzCGDLSD7ecnHd1BiBE5q5SOc3/GVB5sQRsoqIEexfLWbG?=
 =?us-ascii?Q?trQ0OdAcfDBEWXSqI0vOFvnnvnlxW+7+adaaWa1GhajL44FPNPXw7MaYo1Ff?=
 =?us-ascii?Q?+YdHyeFZmHL5kN2MorWbT7lNy1BdSlPTLUixQeJWaJ8aRyOakPOuicwXGECz?=
 =?us-ascii?Q?vqn5szoPareUbRBu3+ayVhl1en/5U5j3ape84ca+NrPJeIf2TANYkGMHJdEo?=
 =?us-ascii?Q?EmXXMT8bsgTpaClmK2pzO0jp2lMYEiHgZywRvwojK7QJYT94Vy67GzfKsCEQ?=
 =?us-ascii?Q?f8X9g1N51SwS8D2lUzjeilPPqI7Qw+48K+qxEc5aXU/fWY+9DJjasEMIAv6A?=
 =?us-ascii?Q?byQkEAiYj1F2mGeacAtTYwizmcPBEPD3pJeWoWU20h4l9meBJIxfIBnwAVPB?=
 =?us-ascii?Q?9zd6wyS7Liiu47nEHh3KnwPEjDr7nV/KwDDnv1bPs1mRG3vjk7zsnH4fGD/W?=
 =?us-ascii?Q?RXzDsxPJQtizDWWRS9GTGI+QaWDHPONOogDUXtcCVH8CrDnDJGAFEB+gOONV?=
 =?us-ascii?Q?kwvaxyy26p/qnMi+b6jPzbbz9tH6xSpZGCBcyPIY8qRHMplJiMljrus+/KWS?=
 =?us-ascii?Q?gVr57mM+qNR2eBfXthA5HCIb3TvVPTE3FXCqd2WQqAqc9oOhL7WC4CVVmFYC?=
 =?us-ascii?Q?tpEVBT1HgeR1HMJ59PM74mX/Y5XJXToCCFHLJTMV8cu9vHRPNqXjVuVlcmBU?=
 =?us-ascii?Q?kTDrdlMOx8bBvukBfq3E08xRX/K/al1OSiXG3r9iTgIivZkRsi/D7oRM2T+R?=
 =?us-ascii?Q?+43WUq2xINPWFNGDkUHpHmQxK6/uJlPa6inbMgjOZC1tOlbOj9k1fGzKDomY?=
 =?us-ascii?Q?ExIZmyiKpksKGYLFl35h07Lt5ww0HNk00a7cKhUAYC0EPE3tGWuLq9XMcsV4?=
 =?us-ascii?Q?fV7KWcUqdWSv408QsomFmtSgQXqm8sQgaDRtljt25qllBghWJaBX7zKBYbko?=
 =?us-ascii?Q?wX/noOMekak5sdgSPUsedRQ423x5QFrd/b8lCzIGjFl1JcEWhRaPdrSVn6rV?=
 =?us-ascii?Q?Sg=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: 884b732c-909e-4e82-3126-08dc2c7c6570
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Feb 2024 10:13:16.8875
 (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: FnTeJHU0XKbV+CJy5flGoE6LTel0Gwp0JNsqfZrh3WqeGwDffsV9M0GUe/TDZP+e059XrX2Lrp8XGAR3rMKoB+2GI9of1F2uItDcKNnzdLA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYCPR01MB11988


Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, February 13, 2024 9:41 AM
> Subject: [PATCH net-next v3 5/6] net: ravb: Do not apply features to
> hardware if the interface is down
>=20
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>=20
> Do not apply features 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 features and
> apply them in ravb_open() through ravb_emac_init().
>=20
> To avoid accessing the hardware while the interface is down
> pm_runtime_active() check was introduced. Along with it the device runtim=
e
> PM usage counter has been incremented to avoid disabling the device clock=
s
> 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 v3:
> - updated patch title and description
> - updated patch content due to patch 4/6
>=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
>=20
>  drivers/net/ethernet/renesas/ravb_main.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> b/drivers/net/ethernet/renesas/ravb_main.c
> index b3b91783bb7a..4dd0520dea90 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2566,15 +2566,23 @@ static int ravb_set_features(struct net_device
> *ndev,  {
>  	struct ravb_private *priv =3D netdev_priv(ndev);
>  	const struct ravb_hw_info *info =3D priv->info;
> -	int ret;
> +	struct device *dev =3D &priv->pdev->dev;
> +	int ret =3D 0;
> +
> +	pm_runtime_get_noresume(dev);
> +
> +	if (!pm_runtime_active(dev))
> +		goto out_set_features;

This can be simplified, which avoids 1 goto statement and
Unnecessary ret initialization. I am leaving to you and Sergey.

	if (!pm_runtime_active(dev))
		ret =3D 0;
	else
		ret =3D info->set_feature(ndev, features);

	pm_runtime_put_noidle(dev);
	if (ret)
		goto err;

	ndev->features =3D features;

err:
	return ret;

Cheers,
Biju

>=20
>  	ret =3D info->set_feature(ndev, features);
>  	if (ret)
> -		return ret;
> +		goto out_rpm_put;
>=20
> +out_set_features:
>  	ndev->features =3D features;
> -
> -	return 0;
> +out_rpm_put:
> +	pm_runtime_put_noidle(dev);
> +	return ret;
>  }
>=20
>  static const struct net_device_ops ravb_netdev_ops =3D {
> --
> 2.39.2