Re: [PATCH] omap:pm: Fix boot-time errors with debugfs disabled

From: "Menon, Nishanth" <nm@ti.com>
To: "Premi, Sanjeev" <premi@ti.com>
Cc: "Hilman, Kevin" <khilman@ti.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: Re: [PATCH] omap:pm: Fix boot-time errors with debugfs disabled
Date: Thu, 19 May 2011 08:58:15 -0500	[thread overview]
Message-ID: <BANLkTi=9hP2Hve-DfeM8QR-D5qvcn3omgg@mail.gmail.com> (raw)
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593024CFDCEE1@dbde02.ent.ti.com>

On Thu, May 19, 2011 at 05:30, Premi, Sanjeev <premi@ti.com> wrote:
>> -----Original Message-----
>> From: Hilman, Kevin
>> Sent: Wednesday, May 18, 2011 10:04 PM
>> To: Premi, Sanjeev
>> Cc: linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] omap:pm: Fix boot-time errors with
>> debugfs disabled
>>
>> Hi Sanjeev,
>>
>> Sanjeev Premi <premi@ti.com> writes:
>>
>> > There is an implicit assumption in current implementation that
>> > debugfs is always enabled.
>> >
>> > When debugfs is disabled, these errors are noticed during boot:
>> >   omap_voltage_late_init: Unable to create voltage debugfs main dir
>> >   vdd_debugfs_init: Unable to create debugfs directory for vdd_mpu
>> >   vdd_debugfs_init: Unable to create debugfs directory for vdd_core
>> >
>> > This patch fixes these errors by enclosing code related to debugfs
>> > in #ifdef CONFIG_DEBUG_FS..#endif.
>> >
>> > Boot tested on OMAP3EVM.
>> >
>> > Signed-off-by: Sanjeev Premi <premi@ti.com>
>>
>> Sorry for the delay here, I've been on the road and finally
>> catching up
>> on the list.
>>
>> Basically, I'm in the process of a pretty major cleanup of
>> the VC/VP and
>> SR layers.  For example, in my pm-wip/voltdm_* branches, the debugfs
>> interface to the voltage layer has been completely removed.  I'm also
>> thinking of removing the SR debugfs interface also, as I don't really
>> think we need a userspace interface for this.  A board-level interface
>> is probably enough (/me waits for flame from Nishanth :)
>>
>> That being said, your approach below isn't quite right in its usage of
>> #ifdefs.  Use of #ifdefs like this is frowned upon for many reasons.
>> For starters, take a look the '#ifdefs are ugly' section of
>> Documentation/SubmittingPatches.
>
> [sp] I don't like #ifdefs either but each time we cannot create
>     a new file changes like this.
>
>     The current code is a mess with debugfs used too frequently.
>     And - all of it is not for debug. The location of ifdefs in
>     in the patch illustrates it quite well.
>
>     BTW, this isn't the only use of ifdefs in a C file in Linux.
in reality the only reason you've had to do this patch was because we
had a wicked handling of debugfs entries in voltage layer - with
voltdm_c these are all gone. further any entries remaining (e.g. SR)
are:
dentry for debugfs file -> just a minor overhead not deserving a #ifdef
all other functions of debugfs (as per include/linux/debugfs.h) when
debugfs is disabled in .config will be static inlined and we will not
need any #ifdefs at all

The real pending question is about functional SR debugfs entries that
need to loose it's critical functionality.
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html