Re: [Devel] [PATCH 0/4] int to bool conversion
by Moore, Robert
"bool" can be problematic as it isn't totally portable. It is usually implemented as a macro.
That’s why ACPICA doesn't use it.
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:[email protected]]
> Sent: Monday, January 26, 2015 5:33 AM
> To: Quentin Lambert
> Cc: Zhang, Rui; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown;
> Shaohua Li; linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-
> kernel(a)vger.kernel.org
> Subject: Re: [PATCH 0/4] int to bool conversion
>
> On Monday, January 26, 2015 09:30:55 AM Quentin Lambert wrote:
> > Sorry for the delay in answering ....
> >
> > On 22/01/2015 17:18, Rafael J. Wysocki wrote:
> > > On Thursday, January 22, 2015 09:49:41 AM Quentin Lambert wrote:
> > >> These patches convert local variables from int to bool when relevant.
> > > And what exactly is the need for that? Does that fix any functional
> problems?
> > >
> > >
> > It doesn't fix any functional problem. The point of this patch is to
> > increase the code readability by lifting some of the ambiguities that
> > appear when using an integer variable as boolean.
> >
> > My understanding is that by explicitly using a boolean declaration
> > when it is relevant it clearly informs the reader that the variable is
> > going to represent a binary state. Moreover, using the keywords true
> > and false help indicate that the variable will not be involved in any
> > computation other than boolean arithmetic.
>
> Well, in the new code, yes. The existing code is a different matter
> though and it doesn't actually hurt if you leave the ints where they are,
> so there's no reason to make those changes.
>
> If you change old code and the change is not trivial (eg. fixes of white
> space or comments, or kernel messages etc.) and someone enounters a bug
> that may be related to it, they will have to go through your changes to
> see if that's not the source of the bug. That's not really productive.
>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
7 years, 4 months
Re: [Devel] [PATCH v2 3/5] ACPI: add arch-specific compilation for _OSI and the blacklist
by Al Stone
On 02/04/2015 07:03 AM, Rafael J. Wysocki wrote:
> On Wednesday, February 04, 2015 03:00:15 PM Rafael J. Wysocki wrote:
>> On Tuesday, February 03, 2015 05:21:42 PM al.stone(a)linaro.org wrote:
>>> From: Al Stone <al.stone(a)linaro.org>
>>>
>>> Now that all of the _OSI functionality has been separated out, we can
>>> provide arch-specific functionality for it. This also allows us to do
>>> the same for the acpi_blacklisted() function.
>>>
>>> Whether arch-specific functions are used or not now depends on the config
>>> options CONFIG_ACPI_ARCH_SPECIFIC_OSI and CONFIG_ARCH_SPECIFIC_BLACKLIST.
>>> By default, both are set false which causes the x86/ia64 versions to be
>>> used, just as is done today. Setting one or both of these options true
>>> will cause architecture-specific implementations to be built instead; this
>>> patch also provides arm64 implementations.
>>>
>>> For x86/ia64, there is no functional change.
>>>
>>> For arm64, any use of _OSI will issue a warning that it is deprecated.
>>> All use of _OSI will return false -- i.e., it will return no useful
>>> information to any firmware using it. The ability to temporarily turn
>>> on _OSI, or turn off _OSI, or affect it in other ways from the command
>>> line is no longer available for arm64, either. The blacklist for ACPI
>>> on arm64 is empty. This will, of course, require ACPI to be enabled
>>> for arm64.
>>>
>>> Signed-off-by: Al Stone <al.stone(a)linaro.org>
>>> ---
>>> drivers/acpi/Kconfig | 22 ++++++++++++++++++++++
>>> drivers/acpi/Makefile | 19 ++++++++++++++++++-
>>> drivers/acpi/blacklist-arm.c | 20 ++++++++++++++++++++
>>> drivers/acpi/blacklist.c | 5 +++++
>>> drivers/acpi/osi-arm.c | 25 +++++++++++++++++++++++++
>>> 5 files changed, 90 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/acpi/blacklist-arm.c
>>> create mode 100644 drivers/acpi/osi-arm.c
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 3e3bd35..4190940 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -369,6 +369,28 @@ config ACPI_REDUCED_HARDWARE_ONLY
>>>
>>> If you are unsure what to do, do not enable this option.
>>>
>>> +config ACPI_ARCH_SPECIFIC_OSI
>>
>> I woulnd't make this and the other one user-selectable. Let architectures
>> select them from their top-level Kconfig files.
>>
>> That's what we do with the other CONFIG_ARCH_ things.
>>
>> So in the architecture-specific Kconfig you'll have
>>
>> config ACPI_ARCH_SPECIFIC_OSI
>> def_bool n
>> depends on ACPI
>>
>> Moreover, I'd call that ARCH_SPECIFIC_ACPI_OSI.
>
> Or even better, you can define them here (drivers/acpi/Kconfig/) as
>
> config ARCH_SPECIFIC_ACPI_OSI
> def_bool n
>
> and then do
>
> select ARCH_SPECIFIC_ACPI_OSI if ACPI
>
> as you did in [4/5].
>
>
Ah, indeed I did. Okay; I'll touch that up.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------
7 years, 4 months
Re: [Devel] [PATCH v2 1/5] ACPI: move acpi_os_handler() so it can be made arch-dependent later
by Al Stone
On 02/04/2015 06:50 AM, Rafael J. Wysocki wrote:
> On Tuesday, February 03, 2015 05:21:40 PM al.stone(a)linaro.org wrote:
>> From: Al Stone <al.stone(a)linaro.org>
>>
>> In order to deprecate the use of _OSI for arm64 or other new architectures,
>> we need to make the default handler something we can change for various
>> platforms. This patch moves the definition of acpi_osi_handler() -- the
>> function used by ACPICA as a callback for evaluating _OSI -- into a separate
>> file. Subsequent patches will change which files get built so that we can
>> then build the version of _OSI we need for a particular architecture.
>>
>> There is no functional change.
>>
>> Signed-off-by: Al Stone <al.stone(a)linaro.org>
>> ---
>> drivers/acpi/Makefile | 2 +-
>> drivers/acpi/osi.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/osl.c | 24 ------------
>> include/linux/acpi.h | 1 +
>> 4 files changed, 102 insertions(+), 25 deletions(-)
>> create mode 100644 drivers/acpi/osi.c
>>
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index c346011..df348b3 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -18,7 +18,7 @@ obj-y += acpi.o \
>> acpica/
>>
>> # All the builtin files are in the "acpi." module_param namespace.
>> -acpi-y += osl.o utils.o reboot.o
>> +acpi-y += osl.o utils.o reboot.o osi.o
>> acpi-y += nvs.o
>>
>> # Power management related files
>> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
>> new file mode 100644
>> index 0000000..fff2b0c
>> --- /dev/null
>> +++ b/drivers/acpi/osi.c
>> @@ -0,0 +1,100 @@
>> +/*
>> + * osi.c - _OSI implementation (moved from drivers/acpi/osl.c)
>> + *
>> + * Copyright (C) 2000 Andrew Henroid
>> + * Copyright (C) 2001, 2002 Andy Grover <andrew.grover(a)intel.com>
>> + * Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh(a)intel.com>
>> + * Copyright (c) 2008 Intel Corporation
>> + * Author: Matthew Wilcox <willy(a)linux.intel.com>
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>
> Nit: The street address of the FSF is not really useful here. What if they move? :-)
This was one of the things checkpatch complained about, understandably :). It's
a direct cut'n'paste from osl.c.
I can clean these up in the new file; would it help to clean up osl.c (at least
from checkpatch's point of view), as long as I'm at it?
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + */
[snip...]
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------
7 years, 4 months
ACPICA version 20150204 released
by David E. Box
04 February 2015. Summary of changes for version 20150204:
This release is available at https://acpica.org/downloads
ACPICA kernel-resident subsystem:
Updated all ACPICA copyrights and signons to 2015. Added the 2015
copyright to all module headers and signons, including the standard Linux
header. This affects virtually every file in the ACPICA core subsystem,
iASL compiler, all ACPICA utilities, and the test suites.
Events: Introduce ACPI_GPE_DISPATCH_RAW_HANDLER to fix GPE storm issues.
A raw gpe handling mechanism was created to allow better handling of GPE
storms that aren't easily managed by the normal handler. The raw handler
allows disabling/renabling of the the GPE so that interrupt storms can be
avoided in cases where events cannot be timely serviced. In this scenario,
handlers should use the AcpiSetGpe() API to disable/enable the GPE. This API
will leave the reference counts undisturbed, thereby preventing unintentional
clearing of the GPE when the intent in only to temporarily disable it. Raw
handlers allow enabling and disabling of a GPE by removing GPE register
locking. As such, raw handlers much provide their own locks while using
GPE API's to protect access to GPE data structures.
Lv Zheng
Events: Always modify GPE registers under the GPE lock.
Bug fix. Applies GPE lock around AcpiFinishGpe() to protect access to GPE
register values.
Unix makefiles: Separate option to disable optimizations and _FORTIFY_SOURCE.
This change removes the _FORTIFY_SOURCE flag from the NOOPT disable option and
creates a separate flag (NOFORTIFY) for this purpose. Some toolchains may
define _FORTIFY_SOURCE which leads redefined errors when building ACPICA. This
allows disabling the option without also having to disable optimazations.
David Box
Current Release:
Non-Debug Version: 101.7K Code, 27.9K Data, 129.6K Total
Debug Version: 199.2K Code, 82.4K Data, 281.6K Total
7 years, 4 months