Subject: Re: [PATCH] Declare PNP option parsing functions as __init
From: Thomas Renninger <trenn@suse.de>
Reply-To: trenn@suse.de
To: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Rene Herman <rene.herman@keyaccess.nl>,
       linux-kernel <linux-kernel@vger.kernel.org>,
       akpm <akpm@linux-foundation.org>, "Li, Shaohua" <shaohua.li@intel.com>
In-Reply-To: <200711301652.42110.bjorn.helgaas@hp.com>
References: <1196442277.23251.318.camel@queen.suse.de>
	 <47509EB6.20300@keyaccess.nl>  <200711301652.42110.bjorn.helgaas@hp.com>
Content-Type: text/plain
Date: Sun, 02 Dec 2007 14:32:07 +0100
Message-Id: <1196602327.4049.48.camel@queen.suse.de>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2806
Lines: 71


On Fri, 2007-11-30 at 16:52 -0700, Bjorn Helgaas wrote:
> On Friday 30 November 2007 04:37:26 pm Rene Herman wrote:
> > On 30-11-07 18:04, Thomas Renninger wrote:
> > > If I have not overseen something, it should be rather obvious that those
> > > can all be declared __init...
> > > ---------------
> > > 
> > > Declare PNP option parsing functions as __init
> > > 
> > > There are three kind of parse functions provided by PNP acpi/bios:
> > >  - get current resources
> > >  - set resources
> > >  - get possible resources
> > > The first two may be needed later at runtime.
> > > The possible resource settings should never change dynamically.
> > > And even if this would make any sense (I doubt it), the current implementation
> > > only parses possible resource settings at early init time:
> > >   -> declare all the option parsing __init
> > > 
> > > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > 
> > Yes. Obviousness aside,
> > 
> > (0) pnpacpi_add_device                          is only caller of
> > ...
> 
> I agree this is probably safe in the current implementation.
> 
> However, I think the current implementation is just broken because
> we can't really handle hotplug of ACPI devices.  Specifically, I think
> the first TBD in acpi_bus_check_device() should be fleshed out so it
> does something like pnpacpi_add_device().
Yes, making the ACPI layer more hotplug capable is something that should
be worked on again.

> So my dissenting opinion is that this patch would just get reverted
> soon anyway when somebody finishes implementing ACPI hotplug, and
> therefore it's not worth doing.

Yes you are right. I thought _PSR could always be called at init time,
whether present or not (which would certainly work for most/all devices
as _PSR info should be kind of static), but ACPI spec forbids it:

6.3.7 _STA (Status)
If bit 0 is cleared, then bit 1 must also be cleared (in other words, a
device that is not present cannot be enabled).
A device can only decode its hardware resources if both bits 0 and 1 are
set. If the device is not present (bit 0 cleared) or not enabled (bit 1
cleared), then the device must not decode its resources.

I only saw:
6.2.10 _PRS (Possible Resource Settings):
If the device is disabled when _PRS is called, it must remain disabled.

But disabled and not present are different things...


It's stupid as the possible resources of a device will always remain the
same, whether it is present or not, but if it is written down in the
spec, there is not much to argue about it...

Thanks,

    Thomas

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