Re: [PATCH 1/3] mmc: Export internal host state through debugfs

From: Haavard Skinnemoen
Date: Sat Jun 28 2008 - 09:37:06 EST


Pierre Ossman <drzeus-mmc@xxxxxxxxx> wrote:
> On Thu, 26 Jun 2008 13:09:47 +0200
> Haavard Skinnemoen <haavard.skinnemoen@xxxxxxxxx> wrote:
>
> > From: Haavard Skinnemoen <hskinnemoen@xxxxxxxxx>
> >
> > This adds a new config option, MMC_DEBUG_FS which will, when enabled,
> > create a few files under /sys/kernel/debug containing information
> > about an mmc host's internal state.
> >
> > Host drivers can add additional files and directories under the host's
> > root directory by passing the debugfs_root field in struct mmc_host as
> > the 'parent' parameter to debugfs_create_*.
> >
> > Unfinished: No files are actually created yet.
> >
> > Signed-off-by: Haavard Skinnemoen <hskinnemoen@xxxxxxxxx>
> > ---
>
> Is there any point to having a separate option for this? Can't we just
> include it if DEBUG_FS is defined?

Sure. It doesn't introduce any additional overhead, just a bit of extra
code.

> Otherwise the system looks ok to me, assuming that everyone is of the
> opinion that debugfs is _not_ a stable API.

That's my assumption at least. Debugfs is just a place where you can
throw random stuff that may help you debugging stuff.

> > +
> > +#else
> > +static inline void mmc_add_host_debugfs(struct mmc_host *host)
> > +{
> > +
> > +}
> > +
> > +static inline void mmc_remove_host_debugfs(struct mmc_host *host)
> > +{
> > +
> > +}
> > +#endif
> > +
>
> In the other places we have conditional function, it is done by
> ifdef:ing the calls instead. There are just two of them, so it should
> be less mess.

Hmm. You mean it's better with three #ifdefs than one?

Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/