Re: [PATCH 06/10] ACPI: call acpi_debug_init() explicitly rather than as initcall

[Yinghai Lu <yinghai@kernel.org>]

Bjorn Helgaas wrote:
> On Tuesday 24 March 2009 05:08:12 pm Yinghai Lu wrote:
>> On Tue, Mar 24, 2009 at 3:50 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>>> This patch makes acpi_init() call acpi_debug_init() directly.
>>> Previously, both were subsys_initcalls.  acpi_debug_init()
>>> must happen after acpi_init(), and it's better to call it
>>> explicitly rather than rely on link ordering.
>>>
>>> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
>>> ---
>>>  drivers/acpi/bus.c      |    1 +
>>>  drivers/acpi/debug.c    |   14 ++++++--------
>>>  drivers/acpi/internal.h |    6 ++++++
>>>  3 files changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>>> index c133072..f32cfd6 100644
>>> --- a/drivers/acpi/bus.c
>>> +++ b/drivers/acpi/bus.c
>>> @@ -883,6 +883,7 @@ static int __init acpi_init(void)
>>>        acpi_ec_init();
>>>        acpi_power_init();
>>>        acpi_system_init();
>>> +       acpi_debug_init();
>>>        return result;
>>>  }
>>>
>>> diff --git a/drivers/acpi/debug.c b/drivers/acpi/debug.c
>>> index 20223cb..9cb189f 100644
>>> --- a/drivers/acpi/debug.c
>>> +++ b/drivers/acpi/debug.c
>>> @@ -297,17 +297,15 @@ acpi_system_write_debug(struct file *file,
>>>
>>>        return count;
>>>  }
>>> +#endif
>>>
>>> -static int __init acpi_debug_init(void)
>>> +int __init acpi_debug_init(void)
>>>  {
>>> +#ifdef CONFIG_ACPI_PROCFS
>>>        struct proc_dir_entry *entry;
>>>        int error = 0;
>>>        char *name;
>>>
>>> -
>>> -       if (acpi_disabled)
>>> -               return 0;
>>> -
>>>        /* 'debug_layer' [R/W] */
>>>        name = ACPI_SYSTEM_FILE_DEBUG_LAYER;
>>>        entry =
>>> @@ -338,7 +336,7 @@ static int __init acpi_debug_init(void)
>>>        remove_proc_entry(ACPI_SYSTEM_FILE_DEBUG_LAYER, acpi_root_dir);
>>>        error = -ENODEV;
>>>        goto Done;
>>> -}
>>> -
>>> -subsys_initcall(acpi_debug_init);
>>> +#else
>>> +       return 0;
>>>  #endif
>>> +}
>>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>>> index 4a35f6e..44b8402 100644
>>> --- a/drivers/acpi/internal.h
>>> +++ b/drivers/acpi/internal.h
>>> @@ -3,6 +3,12 @@
>>>  int acpi_scan_init(void);
>>>  int acpi_system_init(void);
>>>
>>> +#ifdef CONFIG_ACPI_DEBUG
>> ==> #if defined(CONFIG_ACPI_DEBUG) && defined(CONFIG_ACPI_PROCFS)
> 
> I could do that, and leave the #ifdefs in debug.c as they were,
> but I thought it was cleaner to make it so that if we compile debug.c
> (i.e., CONFIG_ACPI_DEBUG=y), it always provides acpi_debug_init().
> 
> I moved the #ifdefs in debug.c so that acpi_debug_init() is a no-op
> if CONFIG_ACPI_PROCFS=n.
> 
> So I think my patch already addressed your concern, but let me
> know if not.

you had two copy
+#else
> > +       return 0;
> >  #endif
...

with
#if defined(CONFIG_ACPI_DEBUG) && defined(CONFIG_ACPI_PROCFS) 
in .h

you only need to do
>>> -
>>> -       if (acpi_disabled)
>>> -               return 0;
>>> -
>>>        /* 'debug_layer' [R/W] */
>>>        name = ACPI_SYSTEM_FILE_DEBUG_LAYER;
>>>        entry =
>>> @@ -338,7 +336,7 @@ static int __init acpi_debug_init(void)
>>>        remove_proc_entry(ACPI_SYSTEM_FILE_DEBUG_LAYER, acpi_root_dir);
>>>        error = -ENODEV;
>>>        goto Done;
>>> -}
>>> -
>>> -subsys_initcall(acpi_debug_init);

in debug.c

totally you will have less one #ifdef

YH