Re: [PATCH v2 1/4] ASoC: Intel: Skylake: Add debugfs support

[Vinod Koul <vinod.koul@intel.com>]

On Thu, Jun 29, 2017 at 05:16:05PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> On Jun 29 2017 16:56, Vinod Koul wrote:
> > On Thu, Jun 29, 2017 at 04:43:38PM +0900, Takashi Sakamoto wrote:
> >> Hi,
> > 
> > Hi Takashi,
> > 
> >>> +struct skl_debug *skl_debugfs_init(struct skl *skl)
> >>> +{
> >>> +	struct skl_debug *d;
> >>> +
> >>> +	d = devm_kzalloc(&skl->pci->dev, sizeof(*d), GFP_KERNEL);
> >>> +	if (!d)
> >>> +		return NULL;
> >>> +
> >>> +	/* create the root dir first */
> >>> +	d->fs = debugfs_create_dir(KBUILD_MODNAME, NULL);
> >>
> >> ALSA SoC part has a debugfs support. It adds "asoc" node into debugfs
> >> mount point and export "snd_soc_debugfs_root" symbol as a root node. I
> >> think it a better idea to collect relevant nodes into the place, rather
> >> than dispersing them.
> > 
> > Yes we can use that, but then this is very driver specific info, does it
> > make sense to keep under framework 'asoc' ?
> 
> It makes sense in a point that it's a part of drivers in ALSA SoC part.

Yes that is a very valid point indeed

> > If we decide to use that, a more intuitive place might be "platform" rather
> > than "asoc" which creates dependency on sound card creation which might happen
> > much later.
> >
> > for debug, I would like to avoid complexity and go with simple device
> > approach...
> 
> I have no objection to your opinion. I like an idea "keep it short and
> simple". If no issues I had, I would select the same direction. However,
> space on debugfs is one of shared resources on system. If being polite
> to the other subsystems, it's better to avoid scattering it, in my opinion.
> 
> For naming scheme or directory structure inner the node, please have
> enough discussion with the other developers for ALSA SoC part. But in
> this timing, it would be acceptable to add your node just under "asoc"
> node. The arrangement could be done later.

Okay we rechecked and we could use component.debugfs_root which would point
to component directory which seems more apt, so we will use that instead of
root 'asoc'

Thanks for the suggestions, will send v3 shortly

-- 
~Vinod