Re: [PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 23:45 +0530, Vinod Koul wrote:
> On 04-01-23, 21:26, Chunfeng Yun wrote:
> > These debugfs files are mainly used to make eye diagram test
> > easier,
> > especially helpful to do HQA test for a new IC without efuse
> > enabled.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx>
> > ---
> > v6: no changes
> > 
> > v5: using common debugfs config CONFIG_DEBUG_FS
> > 
> > v4: fix build warning of sometimes uninitialized variable
> > 
> > v3: fix typo of "debugfs" suggested by AngeloGioacchino
> > 
> > v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
> > ---
> >  drivers/phy/mediatek/phy-mtk-tphy.c | 405
> > +++++++++++++++++++++++++++-
> >  1 file changed, 404 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > index e906a82791bd..923e5ee119f3 100644
> > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > @@ -7,6 +7,7 @@
> >  <skip>
> > +
> > +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct
> > mtk_phy_instance *inst)
> > +{
> > +	char name[16];
> > +
> > +	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);
> 
> I wouold suggest driver name/ device name rather than phy.foo...
> again
> folks needs to see what is foo
> 
The driver creates a root folder by its device name, e.g. 
t-phy@11290000, it has two subphy, when use dev_name(&phy->dev) to
create folder for each phy, it will be:

# ls /sys/kernel/debug/phy/t-phy@11290000/
phy-t-phy@11290000.0  phy-t-phy@11290000.1

the phy's device name is created by
"dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id)"

dev_name(dev) is the parent name, it's t-phy@11290000 in example,
id is the phy->id.

due to the root folder already provide its parent device name, seems no
need provide it again in each phy's folder.

How about create the phy's folder by : "snprintf(name, sizeof(name) -
1, "phy-%d", inst->phy->id);"

then it becomes as below, seems brief:
# ls /sys/kernel/debug/phy/t-phy@11290000/
phy-0  phy-1

Thanks a lot

> 
> > +	inst->dbgfs = debugfs_create_dir(name, tphy->dbgfs_root);
> > +
> > +	debugfs_create_file("type", 0444, inst->dbgfs, inst,
> > &tphy_type_fops);
> > +
> > +	switch (inst->type) {
> > +	case PHY_TYPE_USB2:
> > +		u2_phy_dbgfs_files_create(inst);
> > +		break;
> > +	case PHY_TYPE_USB3:
> > +	case PHY_TYPE_PCIE:
> > +		u3_phy_dbgfs_files_create(inst);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> > +
<skip>