Received-SPF: pass (google.com: domain of linux-kernel+bounces-212936-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@amd.com>
To: Sean Anderson <sean.anderson@linux.dev>, Laurent Pinchart
	<laurent.pinchart@ideasonboard.com>, "linux-phy@lists.infradead.org"
	<linux-phy@lists.infradead.org>
CC: Vinod Koul <vkoul@kernel.org>, "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "Simek, Michal" <michal.simek@amd.com>,
	Kishon Vijay Abraham I <kishon@kernel.org>
Subject: RE: [PATCH v2 4/4] phy: zynqmp: Add debugfs support
Thread-Topic: [PATCH v2 4/4] phy: zynqmp: Add debugfs support
Thread-Index: AQJ83aedMQVFXzZR+sovobCavopdtgGvrfvSsHOmC7A=
Date: Thu, 13 Jun 2024 09:20:41 +0000
Message-ID:
 <MN0PR12MB5953519A685A222CC603A2FEB7C12@MN0PR12MB5953.namprd12.prod.outlook.com>
References: <20240506170110.2874724-1-sean.anderson@linux.dev>
 <20240506170110.2874724-5-sean.anderson@linux.dev>
In-Reply-To: <20240506170110.2874724-5-sean.anderson@linux.dev>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?o7SS+O53ScM3yIX04AJWTyeTZkm5vC4fmJ9+Me2Gw1asGzzBEyIHmlRxWq56?=
 =?us-ascii?Q?aXdNWNhmkVEYTo1z3keJDVOvOSNy7P1Nccvm9E2nYo9ncM8hKYkjKgAoA6db?=
 =?us-ascii?Q?upM4sKBeRW3NaBv1x52339mbuS5alh3LFFLHrtyR5OvsiN/qN8Gv7/H+3u7a?=
 =?us-ascii?Q?AvgRteb8hk7798zKeowLKvhXHz1LUidJT4hCgZ01V4JNzru26+qGMLcrau0T?=
 =?us-ascii?Q?hXitO8+CQlryja+QBh1+SgkXLba3ykzRpnT3jiTZVqdOUQPce/paatqs6RyJ?=
 =?us-ascii?Q?U3Q0vgfBSGA5UpwKwWqwKo6GgMOGu9LhWV/BxyXlsA3vHysvakwcEbK0O4EI?=
 =?us-ascii?Q?vxoVI2MQ4V3VAGnwubzU0MLG0ksTKObSsNpuN5TtuqWLeh4mnWHxBhaMhisz?=
 =?us-ascii?Q?tJj8y0CGcSYp6yS5rLFkHcUzmoeq/WA05OgE/Na6rxOY3EiH/+MvIwWHgaMz?=
 =?us-ascii?Q?7R8A5vJk64WPOie37gIrHINMe/nOOWWJVSFswqMqhuusQ0oaMXP43e4rb/VW?=
 =?us-ascii?Q?UMANYwFYn7dn5huWbzpgS9peMTVIFCwkpiYRQVzzZVDalfF+OzubgrtFinVK?=
 =?us-ascii?Q?CwOE1y6+c9J6cI7apZ774onZ58SlIN47JnxMz9uIPXWoSasFlp4SU0gBINq8?=
 =?us-ascii?Q?EdwM3v6yx6LYTRC3jKdeW3DbHe3atuOlxMczeqBfGAip5Tx/kMONTGjToWUm?=
 =?us-ascii?Q?wyUdKTZa4kBSlcS5PB47WqqBekmNKwsTEzws1zJQAAc2j1f6KD1vLSuFeuLA?=
 =?us-ascii?Q?kLwyg/0Cb4vnydZBggKM1IKXjAUAphUnCZaHvdJEnPLto3LlBlnldja9lC9o?=
 =?us-ascii?Q?8wSa2Vimuc9MYaVyN6qfTci2UcNU0rFAtOcwTreCmZGIHIfRDN7gYw6s7Xod?=
 =?us-ascii?Q?ExtXXdun4fz1eL1TT7YZ43aRYV8ej4AvsyZ5gNPcv3lknvs6BYhwxEb7salg?=
 =?us-ascii?Q?/mSg/zmcqACQeej8yzVTLTCmp4AWPUTIqMY1C+mom2ohoqUyf30t2NQhIJN9?=
 =?us-ascii?Q?U91tpqT1kDwGhLDGwFk+AV6enqzQqKytB8d107Hojclk014uSlsrQnw43+fR?=
 =?us-ascii?Q?+d1rLLTWjEBYfGAlu4SNBObSX+0Ts3jPcSG98Z2Yh97p8Flb2/KlEZlSJ27E?=
 =?us-ascii?Q?WcCMYv5MwqBRlcQJPAAWx8G1LfIeVpRvxUD/kuXgK3ajk7Ikh+sguSuAOOCV?=
 =?us-ascii?Q?iu4xjkHPb0W/4Z+wYaOSiRZ2PuZbhK34OD0LrZ25AWnK9HOzT363F+f7cEm9?=
 =?us-ascii?Q?PwbQ1WE6FFHsYYuY5cu5AZYxV4PoyihwybIVX9YXUO+KsND8P20Idysuxfb5?=
 =?us-ascii?Q?F9z/qmCg0zMaGp7i1Vfqr4VEK2R0pO8IZsAIi1w/Agul6oo7XikRHTR9/Crt?=
 =?us-ascii?Q?Z6+Kc7rcvHJXC/Zg97EqIw5warQXXChS6bhoCkdO71q5V86mlD9NiZbftCQS?=
 =?us-ascii?Q?ziBzyQOP4Nh87yyUZKL0whCI7Bi7D/d4swku18DNLue/DhNqTOPaXF2XhJ2p?=
 =?us-ascii?Q?rzLkGcK8wuYo+v8S27aID/kCRvtSmA4hp1OcX9Waa/GBE906Zgc7RPSB/jCf?=
 =?us-ascii?Q?wT7n/h3or0bYhj1ZG/c=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: amd.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB5953.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d3bf7d4b-dbd7-467f-5dbc-08dc8b8a186d
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jun 2024 09:20:41.0900
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1Cc80lz2j7bgJ1+O6q0Ssc0YduRwYBnGtyk1CiYVBBCp/qiCQLHxHv01wXSnN3Xu
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6448

> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Monday, May 6, 2024 10:31 PM
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; linux-
> phy@lists.infradead.org
> Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Michal Simek <michal.simek@amd.com>;
> Kishon Vijay Abraham I <kishon@kernel.org>; Sean Anderson
> <sean.anderson@linux.dev>
> Subject: [PATCH v2 4/4] phy: zynqmp: Add debugfs support
>=20
> Add support for printing some basic status information to debugfs. This
> is helpful when debugging phy consumers to make sure they are configuring
> the phy appropriately.
>=20
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>=20
> Changes in v2:
> - Use debugfs_create_devm_seqfile
>=20
>  drivers/phy/xilinx/phy-zynqmp.c | 40
> +++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>=20
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-
> zynqmp.c
> index b86fe2a249a8..2520c75a4a77 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -13,6 +13,7 @@
>   */
>=20
>  #include <linux/clk.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> @@ -123,6 +124,15 @@
>  #define ICM_PROTOCOL_DP			0x4
>  #define ICM_PROTOCOL_SGMII		0x5
>=20
> +static const char *const xpsgtr_icm_str[] =3D {
> +	[ICM_PROTOCOL_PD] =3D "powered down",

Protocol saying powered down seems confusing.
Should we say None or None(power down)?

> +	[ICM_PROTOCOL_PCIE] =3D "PCIe",
> +	[ICM_PROTOCOL_SATA] =3D "SATA",
> +	[ICM_PROTOCOL_USB] =3D "USB",
> +	[ICM_PROTOCOL_DP] =3D "DisplayPort",
> +	[ICM_PROTOCOL_SGMII] =3D "SGMII",
> +};
> +
>  /* Test Mode common reset control  parameters */
>  #define TM_CMN_RST			0x10018
>  #define TM_CMN_RST_EN			0x1
> @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev,
>  	return ERR_PTR(-EINVAL);
>  }
>=20
> +/*
> + * DebugFS
> + */
> +
> +static int xpsgtr_status_read(struct seq_file *seq, void *data)
> +{
> +	struct device *dev =3D seq->private;
> +	struct xpsgtr_phy *gtr_phy =3D dev_get_drvdata(dev);
> +	struct clk *clk;
> +	u32 pll_status;
> +
> +	mutex_lock(&gtr_phy->phy->mutex);

Do we see any need for this lock? This function simply read hw register=20
/phy members and decodes it.

> +	pll_status =3D xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1);
> +	clk =3D gtr_phy->dev->clk[gtr_phy->refclk];
> +
> +	seq_printf(seq, "Lane:            %u\n", gtr_phy->lane);
> +	seq_printf(seq, "Protocol:        %s\n",
> +		   xpsgtr_icm_str[gtr_phy->protocol]);
> +	seq_printf(seq, "Instance:        %u\n", gtr_phy->instance);
> +	seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk);
> +	seq_printf(seq, "Reference rate:  %lu\n", clk_get_rate(clk));
> +	seq_printf(seq, "PLL locked:      %s\n",
> +		   pll_status & PLL_STATUS_LOCKED ? "yes" : "no");
> +
> +	mutex_unlock(&gtr_phy->phy->mutex);
> +	return 0;
> +}
> +
>  /*
>   * Power Management
>   */
> @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device *pdev)
>=20
>  		gtr_phy->phy =3D phy;
>  		phy_set_drvdata(phy, gtr_phy);
> +		debugfs_create_devm_seqfile(&phy->dev, "status", phy-
> >debugfs,
> +					    xpsgtr_status_read);
>  	}
>=20
>  	/* Register the PHY provider. */
> --
> 2.35.1.1320.gc452695387.dirty