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

Chunfeng Yun (云春峰)
2023-01-17T09:39:48Z

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@mediatek.com>
> > ---
> > 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>