On Thu, 2012-04-26 at 09:16 -0600, Bjorn Helgaas wrote:
> On Tue, Apr 10, 2012 at 4:21 PM, Toshi Kani <toshi.kani@xxxxxx> wrote:
> > Added macro definitions of _OST source event and status codes.
> > Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
> > since this _OSC bit is not specific to CPU hotplug. This bit is
> > defined in table 6-147 of ACPI 5.0 as follows.
> >
> > Bits: 3
> > Field Name: Insertion / Ejection _OST Processing Support
> > Definition: This bit is set if OSPM will evaluate the _OST
> > object defined under a device when processing
> > insertion and ejection source event codes.
> >
> > This patch sets OSC_SB_HOTPLUG_OST_SUPPORT to the platform-wide
> > OSPM capabilities when CONFIG option of ACPI CPU hotplug or memory
> > hotplug is enabled.
> >
> > Signed-off-by: Toshi Kani <toshi.kani@xxxxxx>
> > ---
> > drivers/acpi/bus.c | 5 +++++
> > include/linux/acpi.h | 26 +++++++++++++++++++++++++-
> > 2 files changed, 30 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 3263b68..a492d64 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -544,6 +544,11 @@ static void acpi_bus_osc_support(void)
> > capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_PPC_OST_SUPPORT;
> > #endif
> >
> > +#if defined(CONFIG_ACPI_HOTPLUG_CPU) || defined(CONFIG_ACPI_HOTPLUG_MEMORY) ||\
> > + defined(CONFIG_ACPI_HOTPLUG_MEMORY_MODULE)
> > + capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_HOTPLUG_OST_SUPPORT;
> > +#endif
>
> This seems a bit strange to me. For one thing, the _OSC discussion
> doesn't seem to indicate that _OST support is specific to CPU or
> memory hotplug. If we tell the platform that we support _OST, the
> platform can assume that we'll evaluate _OST for *any* device, which
> is not the case. I guess this is just another reason why we need
> hotplug support in the ACPI core, not in the individual drivers. Then
> we wouldn't have the ifdefs at all.
I agree, and it should be called for any device. As you know, the issue
is that the global system notify handler acpi_bus_notify() and driver's
notify handler work independently, and their functionality conflicts
each other. I have not thought out yet, but my current thinking is that
we may be able to integrate them into acpi_bus_notify() in the following
way. This model allows a choice -- use the default handler or driver's
handler. We can then call _OST for any device. It can also eliminate
redundant ACPI namespace walks performed by drivers when registering
their notify handlers. What do you think?
struct acpi_device_ops {
:
acpi_op_sys_notify sys_notify; // driver's system notify (new entry)
}
acpi_bus_notify()
{
driver = lookup-driver-with-HID();
if ((driver) && (driver->ops->sys_notify)
driver->ops.sys_notify();
else
Call the system default notify handler;
Call _OST if present;
}
> The second thing is that the kernel should be correct at every point
> in a patch series. In this case, if only patch [1/4] is applied, we
> claim to support _OST, but we actually don't. So I think the patches
> that add support for evaluating _OST should be first, and this one
> should be last. Or even better, a single patch could enable _OST
> evaluation and add this _OSC bit at the same time.
I see. I will change the ordering, if that is OK. I am a little afraid
that the patches may be hard to review when combined into a single
patch.
Thanks!
-Toshi
>
> Bjorn
>
> > +
> > if (!ghes_disable)
> > capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_APEI_SUPPORT;
> > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index f421dd8..4127df8 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -277,7 +277,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
> > #define OSC_SB_PAD_SUPPORT 1
> > #define OSC_SB_PPC_OST_SUPPORT 2
> > #define OSC_SB_PR3_SUPPORT 4
> > -#define OSC_SB_CPUHP_OST_SUPPORT 8
> > +#define OSC_SB_HOTPLUG_OST_SUPPORT 8
> > #define OSC_SB_APEI_SUPPORT 16
> >
> > extern bool osc_sb_apei_support_acked;
> > @@ -309,6 +309,30 @@ extern bool osc_sb_apei_support_acked;
> >
> > extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
> > u32 *mask, u32 req);
> > +
> > +/* _OST General Processing Status Code */
> > +#define ACPI_OST_SC_SUCCESS 0x0
> > +#define ACPI_OST_SC_NON_SPECIFIC_FAILURE 0x1
> > +#define ACPI_OST_SC_UNRECOGNIZED_NOTIFY 0x2
> > +
> > +/* _OST OS Shutdown Processing (0x100) Status Code */
> > +#define ACPI_OST_SC_OS_SHUTDOWN_DENIED 0x80
> > +#define ACPI_OST_SC_OS_SHUTDOWN_IN_PROGRESS 0x81
> > +#define ACPI_OST_SC_OS_SHUTDOWN_COMPLETED 0x82
> > +#define ACPI_OST_SC_OS_SHUTDOWN_NOT_SUPPORTED 0x83
> > +
> > +/* _OST Ejection Request (0x3, 0x103) Status Code */
> > +#define ACPI_OST_SC_EJECT_NOT_SUPPORTED 0x80
> > +#define ACPI_OST_SC_DEVICE_IN_USE 0x81
> > +#define ACPI_OST_SC_DEVICE_BUSY 0x82
> > +#define ACPI_OST_SC_EJECT_DEPENDENCY_BUSY 0x83
> > +#define ACPI_OST_SC_EJECT_IN_PROGRESS 0x84
> > +
> > +/* _OST Insertion Request (0x200) Status Code */
> > +#define ACPI_OST_SC_INSERT_IN_PROGRESS 0x80
> > +#define ACPI_OST_SC_DRIVER_LOAD_FAILURE 0x81
> > +#define ACPI_OST_SC_INSERT_NOT_SUPPORTED 0x82
> > +
> > extern void acpi_early_init(void);
> >
> > extern int acpi_nvs_register(__u64 start, __u64 size);
> > --
> > 1.7.7.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
