lkml.org 
[lkml]   [2009]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 03/15] ACPICA: move common private headers under kernel/acpi/acpica/

* Len Brown <lenb@kernel.org> wrote:

> From: Len Brown <len.brown@intel.com>
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---

> kernel/acpi/acpica/dsfield.c | 2 +-
> kernel/acpi/acpica/dsinit.c | 2 +-
> kernel/acpi/acpica/dsmethod.c | 2 +-
> kernel/acpi/acpica/dsmthdat.c | 2 +-
> kernel/acpi/acpica/dsobject.c | 2 +-
> kernel/acpi/acpica/dsopcode.c | 2 +-
> kernel/acpi/acpica/dsutils.c | 2 +-
> kernel/acpi/acpica/dswexec.c | 2 +-
> kernel/acpi/acpica/dswload.c | 2 +-
> kernel/acpi/acpica/dswscope.c | 2 +-
> kernel/acpi/acpica/dswstate.c | 2 +-
> kernel/acpi/acpica/evevent.c | 2 +-
> kernel/acpi/acpica/evgpe.c | 2 +-
> kernel/acpi/acpica/evgpeblk.c | 2 +-
> kernel/acpi/acpica/evmisc.c | 2 +-
> kernel/acpi/acpica/evregion.c | 2 +-
> kernel/acpi/acpica/evrgnini.c | 2 +-
> kernel/acpi/acpica/evsci.c | 2 +-
> kernel/acpi/acpica/evxface.c | 2 +-
> kernel/acpi/acpica/evxfevnt.c | 2 +-
> kernel/acpi/acpica/evxfregn.c | 2 +-
> kernel/acpi/acpica/exconfig.c | 2 +-
> kernel/acpi/acpica/exconvrt.c | 2 +-
> kernel/acpi/acpica/excreate.c | 2 +-
> kernel/acpi/acpica/exdump.c | 2 +-
> kernel/acpi/acpica/exfield.c | 2 +-
>
> [... etc ...]

hm, dunno. Do we really want to introduce 'driver/platform' space items
like this in the core kernel/* ?
If it goes there then IMHO the ACPI code needs to be cleaned up
_significantly_ to not wrap native Linux calls like spinlocks, allocators,
etc.

Random example - i dont think stuff like this is readable [in to-be
kernel/acpi/utilities/utcache.c]:

if (cache->current_depth >= cache->max_depth) {
ACPI_FREE(object);
ACPI_MEM_TRACKING(cache->total_freed++);
}
/* Otherwise put this object back into the cache */
else {
status = acpi_ut_acquire_mutex(ACPI_MTX_CACHES);
if (ACPI_FAILURE(status)) {
return (status);
}
acpi_ut_acquire_mutex() under [to-be] kernel/acpi/utilities/utmutex.c
looks absolutely horrible, redirected after some debugging layer to its
final destination:

./include/acpi/acpiosxf.h:#define acpi_os_acquire_mutex(handle,time)
acpi_os_wait_semaphore (handle, 1, time), which does [kernel/acpi/osl.c]:

/*
* TODO: Support for units > 1?
*/
acpi_status acpi_os_wait_semaphore(acpi_handle handle, u32 units, u16 timeout)
{
acpi_status status = AE_OK;
struct semaphore *sem = (struct semaphore *)handle;
long jiffies;
int ret = 0;
if (!sem || (units < 1))
return AE_BAD_PARAMETER;
if (units > 1)
return AE_SUPPORT;
ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for semaphore[%p|%d|%d]\n",
handle, units, timeout));
if (timeout == ACPI_WAIT_FOREVER)
jiffies = MAX_SCHEDULE_TIMEOUT;
else
jiffies = msecs_to_jiffies(timeout);
ret = down_timeout(sem, jiffies);
if (ret)
status = AE_TIME;
if (ACPI_FAILURE(status)) {
ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
"Failed to acquire semaphore[%p|%d|%d], %s",
handle, units, timeout,
acpi_format_exception(status)));
} else {
ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
"Acquired semaphore[%p|%d|%d]", handle,
units, timeout));
}
return status;
}
so it's a glorified down_timeout(). While we have mutex_timeout(). What's
the plan here?

Ingo


\
 
 \ /
  Last update: 2009-01-02 20:17    [from the cache]
©2003-2011 Jasper Spaans