Re: [PATCH v9 1/2] phy: core: add debugfs files

[Chunfeng Yun (云春峰)]

On Mon, 2023-04-03 at 20:36 +0530, Vinod Koul wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 16-03-23, 15:14, Chunfeng Yun wrote:
> > Add a debugfs root for phy class, and create a debugfs directory
> > under
> > the root when create phy, then phy drivers can add debugfs files.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx>
> > ---
> > v9: create a 'phy' debugfs root, add a debugfs entry in struct phy
> > suggested by Vinod;
> > 
> > No v8 sent out;
> > 
> > v2~v7: no changes
> > ---
> >  drivers/phy/phy-core.c  | 14 ++++++++++++++
> >  include/linux/phy/phy.h |  2 ++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 9951efc03eaa..05715352881e 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/export.h>
> >  #include <linux/module.h>
> >  #include <linux/err.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/device.h>
> >  #include <linux/slab.h>
> >  #include <linux/of.h>
> > @@ -20,6 +21,7 @@
> >  #include <linux/regulator/consumer.h>
> > 
> >  static struct class *phy_class;
> > +static struct dentry *phy_debugfs_root;
> >  static DEFINE_MUTEX(phy_provider_mutex);
> >  static LIST_HEAD(phy_provider_list);
> >  static LIST_HEAD(phys);
> > @@ -996,6 +998,8 @@ struct phy *phy_create(struct device *dev,
> > struct device_node *node,
> >               pm_runtime_no_callbacks(&phy->dev);
> >       }
> > 
> > +     phy->debugfs = debugfs_create_dir(dev_name(&phy->dev),
> > phy_debugfs_root);
> > +
> >       return phy;
> > 
> >  put_dev:
> > @@ -1226,6 +1230,7 @@ static void phy_release(struct device *dev)
> > 
> >       phy = to_phy(dev);
> >       dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
> > +     debugfs_remove_recursive(phy->debugfs);
> >       regulator_put(phy->pwr);
> >       ida_simple_remove(&phy_ida, phy->id);
> >       kfree(phy);
> > @@ -1242,6 +1247,15 @@ static int __init phy_core_init(void)
> > 
> >       phy_class->dev_release = phy_release;
> > 
> > +     phy_debugfs_root = debugfs_create_dir("phy", NULL);
> > +
> >       return 0;
> >  }
> >  device_initcall(phy_core_init);
> > +
> > +static void __exit phy_core_exit(void)
> > +{
> > +     debugfs_remove_recursive(phy_debugfs_root);
> 
> This should be moved to phy_release() we want the debugfs to be
> cleaned
> first
phy_release() will remove its debugfs file, but phy_debugfs_root is the
root of phy core, can't be removed in phy_release()

> 
> > +     class_destroy(phy_class);
> 
> hmmm we are missing this already, that might be an issue!
> 
> > +}
> > +module_exit(phy_core_exit);
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 3a570bc59fc7..f6d607ef0e80 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -148,6 +148,7 @@ struct phy_attrs {
> >   * @power_count: used to protect when the PHY is used by multiple
> > consumers
> >   * @attrs: used to specify PHY specific attributes
> >   * @pwr: power regulator associated with the phy
> > + * @debugfs: debugfs directory
> >   */
> >  struct phy {
> >       struct device           dev;
> > @@ -158,6 +159,7 @@ struct phy {
> >       int                     power_count;
> >       struct phy_attrs        attrs;
> >       struct regulator        *pwr;
> > +     struct dentry           *debugfs;
> >  };
> > 
> >  /**
> > --
> > 2.18.0
> 
> --
> ~Vinod