Re: [PATCH 2/2] ACPI / scan: Simplify container driver

From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Toshi Kani <toshi.kani@hp.com>
Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Matthew Garrett <matthew.garrett@nebula.com>,
	Yinghai Lu <yinghai@kernel.org>, Jiang Liu <liuj97@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] ACPI / scan: Simplify container driver
Date: Thu, 07 Feb 2013 01:55:09 +0100	[thread overview]
Message-ID: <6374398.88OyYZxaMu@vostro.rjw.lan> (raw)
In-Reply-To: <1360189938.23410.276.camel@misato.fc.hp.com>

On Wednesday, February 06, 2013 03:32:18 PM Toshi Kani wrote:
> On Mon, 2013-02-04 at 00:47 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > The only useful thing that the ACPI container driver does is to
> > install system notify handlers for all container and module device
> > objects it finds in the namespace.  The driver structure,
> > acpi_container_driver, and the data structures created by its
> > .add() callback are in fact not used by the driver, so remove
> > them entirely.
> > 
> > It also makes a little sense to build that driver as a module,
> > so make it non-modular and add its initialization to the
> > namespace scanning code.
> > 
> > In addition to that, make the namespace walk callback used for
> > installing the notify handlers more straightforward.
> 
> I think the container driver needs to be registered as an ACPI scan
> driver so that sysfs eject will continue to work for container devices,
> such as ACPI0004:XX/eject.  Since the container driver does not support
> ACPI eject notification (and we have been discussing how system device
> hot-plug should work), this sysfs eject is the only way to eject a
> container device at this point.  I will send an update patchset that
> applies on top of this patch.
> 
> With the update in my patchset:
> Reviewed-by: Toshi Kani <toshi.kani@hp.com>

Thanks, but I'd like to (1) apply your patch from
https://patchwork.kernel.org/patch/2108851/ first and then (2) fold your [2/2]
into my [2/2], if you don't mind, and apply that next.

Moreover, I'm wondering if the #ifndef FORCE_EJECT thing in acpi_eject_store()
actually makes sense after the recent changes to acpi_bus_trim(), because that
can't fail now, so the eject will always be carried out.  So perhaps we can
simply remove the acpi_device->driver check from there entirely in the first
place?

If we really want to be able to prevent ejects from happening in some cases,
we need to implement something along the lines discussed with Greg.

Thanks,
Rafael

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.