[rafael-pm:bleeding-edge] BUILD REGRESSION 7451d4b13852834afd17b629fb7d4410cefba74c
by kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
branch HEAD: 7451d4b13852834afd17b629fb7d4410cefba74c Merge branch 'acpi-apei' into bleeding-edge
Error/Warning reports:
https://lore.kernel.org/linux-acpi/[email protected]
https://lore.kernel.org/linux-acpi/[email protected]
Error/Warning: (recently discovered and may have been fixed)
drivers/bus/hisi_lpc.c:511:41: error: 'struct acpi_device' has no member named 'children'
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
|-- arm64-allyesconfig
| |-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
| `-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
|-- ia64-allmodconfig
| |-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
| |-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
| `-- include-linux-compiler_types.h:error:expression-in-static-assertion-is-not-an-integer
`-- ia64-allyesconfig
|-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
`-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
elapsed time: 722m
configs tested: 52
configs skipped: 2
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm allyesconfig
ia64 allmodconfig
arc allyesconfig
alpha allyesconfig
m68k allyesconfig
m68k allmodconfig
powerpc allnoconfig
mips allyesconfig
powerpc allmodconfig
sh allmodconfig
i386 defconfig
i386 allyesconfig
x86_64 randconfig-a002
x86_64 randconfig-a006
x86_64 randconfig-a004
i386 randconfig-a001
i386 randconfig-a003
i386 randconfig-a005
x86_64 randconfig-a013
x86_64 randconfig-a011
x86_64 randconfig-a015
i386 randconfig-a014
i386 randconfig-a012
i386 randconfig-a016
arc randconfig-r043-20220629
s390 randconfig-r044-20220629
riscv randconfig-r042-20220629
um i386_defconfig
um x86_64_defconfig
x86_64 defconfig
x86_64 allyesconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kunit
x86_64 rhel-8.3-kselftests
x86_64 rhel-8.3-syz
x86_64 rhel-8.3-func
clang tested configs:
x86_64 randconfig-a005
x86_64 randconfig-a001
x86_64 randconfig-a003
i386 randconfig-a002
i386 randconfig-a006
i386 randconfig-a004
x86_64 randconfig-a014
x86_64 randconfig-a016
x86_64 randconfig-a012
i386 randconfig-a013
i386 randconfig-a015
i386 randconfig-a011
hexagon randconfig-r045-20220629
hexagon randconfig-r041-20220629
--
0-DAY CI Kernel Test Service
https://01.org/lkp
1 month, 2 weeks
[rafael-pm:bleeding-edge 37/63] include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
by kernel test robot
tree: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
head: 7451d4b13852834afd17b629fb7d4410cefba74c
commit: 022b762e0e96a96bbc1ec271f59c2f6adf1d594b [37/63] ACPI: bus: Drop unused list heads from struct acpi_device
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220630/202206301543.JpCxX65V-lk...)
compiler: ia64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commi...
git remote add rafael-pm https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
git fetch --no-tags rafael-pm bleeding-edge
git checkout 022b762e0e96a96bbc1ec271f59c2f6adf1d594b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/bus/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp(a)intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/list.h:5,
from include/linux/kobject.h:19,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c: In function 'hisi_lpc_acpi_probe':
drivers/bus/hisi_lpc.c:511:41: error: 'struct acpi_device' has no member named 'children'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~
include/linux/container_of.h:18:33: note: in definition of macro 'container_of'
18 | void *__mptr = (void *)(ptr); \
| ^~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/bits.h:22,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:41: error: 'struct acpi_device' has no member named 'children'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:41: error: 'struct acpi_device' has no member named 'children'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | __same_type(*(ptr), void), \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/ia64/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:248,
from include/linux/build_bug.h:5,
from include/linux/bits.h:22,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/stddef.h:16:58: note: in definition of macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:531:9: note: in expansion of macro 'list_entry'
531 | list_entry((ptr)->next, type, member)
| ^~~~~~~~~~
include/linux/list.h:674:20: note: in expansion of macro 'list_first_entry'
674 | for (pos = list_first_entry(head, typeof(*pos), member); \
| ^~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/kobject.h:19,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/list.h:665:16: note: in definition of macro 'list_entry_is_head'
665 | (&pos->member == (head))
| ^~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:41: error: 'struct acpi_device' has no member named 'children'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~
include/linux/list.h:665:27: note: in definition of macro 'list_entry_is_head'
665 | (&pos->member == (head))
| ^~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/list.h:5,
from include/linux/kobject.h:19,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/container_of.h:18:33: note: in definition of macro 'container_of'
18 | void *__mptr = (void *)(ptr); \
| ^~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/bits.h:22,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | __same_type(*(ptr), void), \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/ia64/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:248,
from include/linux/build_bug.h:5,
from include/linux/bits.h:22,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/bus/hisi_lpc.c:9:
drivers/bus/hisi_lpc.c:511:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~
include/linux/stddef.h:16:58: note: in definition of macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~
include/linux/list.h:520:9: note: in expansion of macro 'container_of'
520 | container_of(ptr, type, member)
| ^~~~~~~~~~~~
include/linux/list.h:564:9: note: in expansion of macro 'list_entry'
564 | list_entry((pos)->member.next, typeof(*(pos)), member)
| ^~~~~~~~~~
include/linux/list.h:676:20: note: in expansion of macro 'list_next_entry'
676 | pos = list_next_entry(pos, member))
| ^~~~~~~~~~~~~~~
drivers/bus/hisi_lpc.c:511:9: note: in expansion of macro 'list_for_each_entry'
511 | list_for_each_entry(child, &adev->children, node) {
| ^~~~~~~~~~~~~~~~~~~
vim +293 include/linux/compiler_types.h
eb111869301e15 Rasmus Villemoes 2019-09-13 291
d15155824c5014 Will Deacon 2017-10-24 292 /* Are two types/vars the same type (ignoring qualifiers)? */
d15155824c5014 Will Deacon 2017-10-24 @293 #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
d15155824c5014 Will Deacon 2017-10-24 294
:::::: The code at line 293 was first introduced by commit
:::::: d15155824c5014803d91b829736d249c500bdda6 linux/compiler.h: Split into compiler.h and compiler_types.h
:::::: TO: Will Deacon <will.deacon(a)arm.com>
:::::: CC: Ingo Molnar <mingo(a)kernel.org>
--
0-DAY CI Kernel Test Service
https://01.org/lkp
1 month, 2 weeks
Re: [PATCH 0/4] ACPI: EC: Various cleanups
by Rafael J. Wysocki
On Mon, Jun 20, 2022 at 11:26 AM Hans de Goede <hdegoede(a)redhat.com> wrote:
>
> Hi All,
>
> Here is a set of cleanups / removal of no longer necessary
> quirks (or so I believe please review carefully). These are all
> things which I noticed while working on my:
> "[RFC 0/4] ACPI[CA]: fix ECDT EC probe ordering issues" series.
>
> Regards,
>
> Hans
>
> p.s.
> Talking about my "[RFC 0/4] ACPI[CA]: fix ECDT EC probe ordering issues"
> series, it would be great if someone can take a look at this and let me
> know if that series seems sane. Then I can convert the ACPICA changes
> from kernel patches to upstream github acpica patches and submit a
> pull-req for those at github.
>
>
> Hans de Goede (4):
> ACPI: EC: Remove duplicate ThinkPad X1 Carbon 6th entry from DMI
> quirks
> ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk
> ACPI: EC: Re-use boot_ec when possible even when
> EC_FLAGS_TRUST_DSDT_GPE is set
> ACPI: EC: Drop unused ident initializers from dmi_system_id tables
>
> drivers/acpi/ec.c | 140 ++++++++++++++++------------------------------
> 1 file changed, 48 insertions(+), 92 deletions(-)
>
> --
All four patches applied as 5.20 material, thanks!
1 month, 2 weeks
Re: [PATCH 1/2] ACPI: Drop redundant check in acpi_device_remove()
by Rafael J. Wysocki
On Sat, Jun 18, 2022 at 1:23 PM Uwe Kleine-König
<u.kleine-koenig(a)pengutronix.de> wrote:
>
> A bus remove callback is only ever called by the device core with a
> bound driver. So there is no need to check if the driver is non-NULL.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig(a)pengutronix.de>
> ---
> drivers/acpi/bus.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 86fa61a21826..67a3f8cf42f9 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1062,12 +1062,11 @@ static void acpi_device_remove(struct device *dev)
> struct acpi_device *acpi_dev = to_acpi_device(dev);
> struct acpi_driver *acpi_drv = acpi_dev->driver;
>
> - if (acpi_drv) {
> - if (acpi_drv->ops.notify)
> - acpi_device_remove_notify_handler(acpi_dev);
> - if (acpi_drv->ops.remove)
> - acpi_drv->ops.remove(acpi_dev);
> - }
> + if (acpi_drv->ops.notify)
> + acpi_device_remove_notify_handler(acpi_dev);
> + if (acpi_drv->ops.remove)
> + acpi_drv->ops.remove(acpi_dev);
> +
> acpi_dev->driver = NULL;
> acpi_dev->driver_data = NULL;
>
>
> base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
> --
Applied (with minor modifications) as 5.20 material along with the [2/2].
Thanks!
1 month, 2 weeks
Re: [PATCH v2] ACPI/processor: Remove unused function acpi_processor_get_limit_info()
by Rafael J. Wysocki
On Fri, Jun 17, 2022 at 11:42 AM Punit Agrawal
<punit.agrawal(a)bytedance.com> wrote:
>
> Riwen Lu <luriwen(a)hotmail.com> writes:
>
> > From: Riwen Lu <luriwen(a)kylinos.cn>
> >
> > Commit 22e7551eb6fd ("ACPI / processor: Remove acpi_processor_get_limit_info()"),
> > left behind this, remove it.
> >
> > Signed-off-by: Riwen Lu <luriwen(a)kylinos.cn>
> >
> > ---
> > v1 -> v2:
> > - Make this patch base on ("ACPI: Split out processor thermal register
> > from ACPI PSS").
>
> For such changes, it is better to send all the related patches as a
> series so it's easy to see the dependencies . In a series the easy /
> obvious fixes should be earlier so it's easier for them to be merged
> while the more significant changes are still being discussed.
>
> Hopefully in this case Rafael too agrees with the dependency patch -
> otherwise, it's just extra churn on the lists.
>
> But don't resend just yet - give some time for others to add their
> feedback.
>
> > ---
> > include/acpi/processor.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> > index ba1e3ed98d3d..9fa49686957a 100644
> > --- a/include/acpi/processor.h
> > +++ b/include/acpi/processor.h
> > @@ -441,7 +441,6 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> > #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
> >
> > /* in processor_thermal.c */
> > -int acpi_processor_get_limit_info(struct acpi_processor *pr);
> > int acpi_processor_thermal_init(struct acpi_processor *pr,
> > struct acpi_device *device);
> > void acpi_processor_thermal_exit(struct acpi_processor *pr,
>
> Fwiw,
>
> Reviewed-by: Punit Agrawal <punit.agrawal(a)bytedance.com>
Applied as 5.20 material with some edits in the subject and changelog.
Thanks!
1 month, 2 weeks
Re: [PATCH v3] ACPI: Split out processor thermal register from ACPI PSS
by Rafael J. Wysocki
On Fri, Jun 17, 2022 at 11:26 AM Punit Agrawal
<punit.agrawal(a)bytedance.com> wrote:
>
> Riwen Lu <luriwen(a)hotmail.com> writes:
>
> > From: Riwen Lu <luriwen(a)kylinos.cn>
> >
> > Commit 239708a3af44 ("ACPI: Split out ACPI PSS from ACPI Processor
> > driver"), moves processor thermal registration to acpi_pss_perf_init(),
> > which doesn't get executed if ACPI_CPU_FREQ_PSS is not enabled.
> >
> > As ARM64 supports P-states using CPPC, it should be possible to also
> > support processor passive cooling even if PSS is not enabled. Split
> > out the processor thermal cooling register from ACPI PSS to support
> > this, and move it into a separate function in processor_thermal.c.
> >
> > Signed-off-by: Riwen Lu <luriwen(a)kylinos.cn>
> >
> > ---
> > v1 -> v2:
> > - Reword the commit message.
> > - Update the signature of acpi_pss_perf_init() to void, and remove the
> > acpi_device parameter.
> > - Move the processor thermal register/remove into a separate function in
> > processor_thermal.c.
> >
> > v2 -> v3:
> > - Remove the "pr" NULL check in processor thermal init/exit fuction.
> > - Pass the acpi_device into processor thermal init/exit, and remove the
> > convert in it.
> > ---
> > drivers/acpi/Kconfig | 2 +-
> > drivers/acpi/Makefile | 5 +--
> > drivers/acpi/processor_driver.c | 72 ++++----------------------------
> > drivers/acpi/processor_thermal.c | 54 ++++++++++++++++++++++++
> > include/acpi/processor.h | 8 +++-
> > 5 files changed, 71 insertions(+), 70 deletions(-)
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 1e34f846508f..2457ade3f82d 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -255,7 +255,6 @@ config ACPI_DOCK
> >
> > config ACPI_CPU_FREQ_PSS
> > bool
> > - select THERMAL
> >
> > config ACPI_PROCESSOR_CSTATE
> > def_bool y
> > @@ -287,6 +286,7 @@ config ACPI_PROCESSOR
> > depends on X86 || IA64 || ARM64 || LOONGARCH
> > select ACPI_PROCESSOR_IDLE
> > select ACPI_CPU_FREQ_PSS if X86 || IA64 || LOONGARCH
> > + select THERMAL
> > default y
> > help
> > This driver adds support for the ACPI Processor package. It is required
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index b5a8d3e00a52..0002eecbf870 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -109,10 +109,9 @@ obj-$(CONFIG_ACPI_PPTT) += pptt.o
> > obj-$(CONFIG_ACPI_PFRUT) += pfr_update.o pfr_telemetry.o
> >
> > # processor has its own "processor." module_param namespace
> > -processor-y := processor_driver.o
> > +processor-y := processor_driver.o processor_thermal.o
> > processor-$(CONFIG_ACPI_PROCESSOR_IDLE) += processor_idle.o
> > -processor-$(CONFIG_ACPI_CPU_FREQ_PSS) += processor_throttling.o \
> > - processor_thermal.o
> > +processor-$(CONFIG_ACPI_CPU_FREQ_PSS) += processor_throttling.o
> > processor-$(CONFIG_CPU_FREQ) += processor_perflib.o
> >
> > obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
> > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > index 368a9edefd0c..1278969eec1f 100644
> > --- a/drivers/acpi/processor_driver.c
> > +++ b/drivers/acpi/processor_driver.c
> > @@ -139,75 +139,17 @@ static int acpi_soft_cpu_dead(unsigned int cpu)
> > }
> >
> > #ifdef CONFIG_ACPI_CPU_FREQ_PSS
> > -static int acpi_pss_perf_init(struct acpi_processor *pr,
> > - struct acpi_device *device)
> > +static void acpi_pss_perf_init(struct acpi_processor *pr)
> > {
> > - int result = 0;
> > -
> > acpi_processor_ppc_has_changed(pr, 0);
> >
> > acpi_processor_get_throttling_info(pr);
> >
> > if (pr->flags.throttling)
> > pr->flags.limit = 1;
> > -
> > - pr->cdev = thermal_cooling_device_register("Processor", device,
> > - &processor_cooling_ops);
> > - if (IS_ERR(pr->cdev)) {
> > - result = PTR_ERR(pr->cdev);
> > - return result;
> > - }
> > -
> > - dev_dbg(&device->dev, "registered as cooling_device%d\n",
> > - pr->cdev->id);
> > -
> > - result = sysfs_create_link(&device->dev.kobj,
> > - &pr->cdev->device.kobj,
> > - "thermal_cooling");
> > - if (result) {
> > - dev_err(&device->dev,
> > - "Failed to create sysfs link 'thermal_cooling'\n");
> > - goto err_thermal_unregister;
> > - }
> > -
> > - result = sysfs_create_link(&pr->cdev->device.kobj,
> > - &device->dev.kobj,
> > - "device");
> > - if (result) {
> > - dev_err(&pr->cdev->device,
> > - "Failed to create sysfs link 'device'\n");
> > - goto err_remove_sysfs_thermal;
> > - }
> > -
> > - return 0;
> > -
> > - err_remove_sysfs_thermal:
> > - sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> > - err_thermal_unregister:
> > - thermal_cooling_device_unregister(pr->cdev);
> > -
> > - return result;
> > -}
> > -
> > -static void acpi_pss_perf_exit(struct acpi_processor *pr,
> > - struct acpi_device *device)
> > -{
> > - if (pr->cdev) {
> > - sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> > - sysfs_remove_link(&pr->cdev->device.kobj, "device");
> > - thermal_cooling_device_unregister(pr->cdev);
> > - pr->cdev = NULL;
> > - }
> > }
> > #else
> > -static inline int acpi_pss_perf_init(struct acpi_processor *pr,
> > - struct acpi_device *device)
> > -{
> > - return 0;
> > -}
> > -
> > -static inline void acpi_pss_perf_exit(struct acpi_processor *pr,
> > - struct acpi_device *device) {}
> > +static inline void acpi_pss_perf_init(struct acpi_processor *pr) {}
> > #endif /* CONFIG_ACPI_CPU_FREQ_PSS */
> >
> > static int __acpi_processor_start(struct acpi_device *device)
> > @@ -229,7 +171,9 @@ static int __acpi_processor_start(struct acpi_device *device)
> > if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> > acpi_processor_power_init(pr);
> >
> > - result = acpi_pss_perf_init(pr, device);
> > + acpi_pss_perf_init(pr);
> > +
> > + result = acpi_processor_thermal_init(pr, device);
> > if (result)
> > goto err_power_exit;
> >
> > @@ -239,7 +183,7 @@ static int __acpi_processor_start(struct acpi_device *device)
> > return 0;
> >
> > result = -ENODEV;
> > - acpi_pss_perf_exit(pr, device);
> > + acpi_processor_thermal_exit(pr, device);
> >
> > err_power_exit:
> > acpi_processor_power_exit(pr);
> > @@ -277,10 +221,10 @@ static int acpi_processor_stop(struct device *dev)
> > return 0;
> > acpi_processor_power_exit(pr);
> >
> > - acpi_pss_perf_exit(pr, device);
> > -
> > acpi_cppc_processor_exit(pr);
> >
> > + acpi_processor_thermal_exit(pr, device);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> > index d8b2dfcd59b5..db6ac540e924 100644
> > --- a/drivers/acpi/processor_thermal.c
> > +++ b/drivers/acpi/processor_thermal.c
> > @@ -266,3 +266,57 @@ const struct thermal_cooling_device_ops processor_cooling_ops = {
> > .get_cur_state = processor_get_cur_state,
> > .set_cur_state = processor_set_cur_state,
> > };
> > +
> > +int acpi_processor_thermal_init(struct acpi_processor *pr,
> > + struct acpi_device *device)
> > +{
> > + int result = 0;
> > +
> > + pr->cdev = thermal_cooling_device_register("Processor", device,
> > + &processor_cooling_ops);
> > + if (IS_ERR(pr->cdev)) {
> > + result = PTR_ERR(pr->cdev);
> > + return result;
> > + }
> > +
> > + dev_dbg(&device->dev, "registered as cooling_device%d\n",
> > + pr->cdev->id);
> > +
> > + result = sysfs_create_link(&device->dev.kobj,
> > + &pr->cdev->device.kobj,
> > + "thermal_cooling");
> > + if (result) {
> > + dev_err(&device->dev,
> > + "Failed to create sysfs link 'thermal_cooling'\n");
> > + goto err_thermal_unregister;
> > + }
> > +
> > + result = sysfs_create_link(&pr->cdev->device.kobj,
> > + &device->dev.kobj,
> > + "device");
> > + if (result) {
> > + dev_err(&pr->cdev->device,
> > + "Failed to create sysfs link 'device'\n");
> > + goto err_remove_sysfs_thermal;
> > + }
> > +
> > + return 0;
> > +
> > +err_remove_sysfs_thermal:
> > + sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> > +err_thermal_unregister:
> > + thermal_cooling_device_unregister(pr->cdev);
> > +
> > + return result;
> > +}
> > +
> > +void acpi_processor_thermal_exit(struct acpi_processor *pr,
> > + struct acpi_device *device)
> > +{
> > + if (pr->cdev) {
> > + sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> > + sysfs_remove_link(&pr->cdev->device.kobj, "device");
> > + thermal_cooling_device_unregister(pr->cdev);
> > + pr->cdev = NULL;
> > + }
> > +}
> > diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> > index 194027371928..ba1e3ed98d3d 100644
> > --- a/include/acpi/processor.h
> > +++ b/include/acpi/processor.h
> > @@ -442,8 +442,12 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> >
> > /* in processor_thermal.c */
> > int acpi_processor_get_limit_info(struct acpi_processor *pr);
> > +int acpi_processor_thermal_init(struct acpi_processor *pr,
> > + struct acpi_device *device);
> > +void acpi_processor_thermal_exit(struct acpi_processor *pr,
> > + struct acpi_device *device);
> > extern const struct thermal_cooling_device_ops processor_cooling_ops;
> > -#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
> > +#ifdef CONFIG_CPU_FREQ
> > void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy);
> > void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy);
> > #else
> > @@ -455,6 +459,6 @@ static inline void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
> > {
> > return;
> > }
> > -#endif /* CONFIG_ACPI_CPU_FREQ_PSS */
> > +#endif /* CONFIG_CPU_FREQ */
> >
> > #endif
>
> Thanks for updating the patch.
>
> FWIW,
>
> Reviewed-by: Punit Agrawal <punit.agrawal(a)bytedance.com>
Applied as 5.20 material under edited subject, thanks!
1 month, 2 weeks
Re: [RFC 4/4] ACPI: fix ECDT EC probe ordering issues
by Rafael J. Wysocki
On Tue, Jun 28, 2022 at 9:42 PM Hans de Goede <hdegoede(a)redhat.com> wrote:
>
> Hi,
>
> On 6/28/22 21:15, Rafael J. Wysocki wrote:
> > On Mon, Jun 27, 2022 at 5:03 PM Hans de Goede <hdegoede(a)redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 6/27/22 16:21, Rafael J. Wysocki wrote:
> >>> First off, thanks for taking care of this issue!
> >>
> >> You're welcome.
> >>
> >>> On Wed, Jun 15, 2022 at 9:57 PM Hans de Goede <hdegoede(a)redhat.com> wrote:
> >>>>
> >>>> ACPI-2.0 says that the EC OpRegion handler must be available immediately
> >>>> (like the standard default OpRegion handlers). So acpi_bus_init() calls
> >>>> acpi_ec_ecdt_probe(), which calls acpi_install_address_space_handler() to
> >>>> install the EC's OpRegion handler, early on.
> >>>
> >>> I think that the key question is what Windows does in this respect.
> >>>
> >>> I honestly don't think that it uses an allowlist to early call _INI
> >>> for a few specific devices.
> >>
> >> Right, I guess that windows does things more hierarchicallly first
> >> calling \._INI then \._SB._INI and then calling other _INI-s around
> >> the time it enters there parts of the ACPI device hierarchy.
> >>
> >> I have the feeling that Windows e.g. has the root PCI bridge fully
> >> setup before even parsing any ACPI objects under it.
> >
> > Which means that if the EC object is under PCI0, it will not evaluate
> > its _REG before enumerating the PCI bus.
>
> Ack, assuming that my hunch of Windows behavior is correct.
>
> >> This is quite different from how Linux (or ACPICa for that manner)
> >> works. So ATM I believe that the fixed list of object paths to
> >> call _INI on early is the best we can do.
> >
> > Well, ACPICA follows the spec quite literally in that respect (and the
> > spec says that _INI should be evaluated "at the table load time"
> > IIRC).
>
> <offtopic for the EC issue, sorry (but stronly related)>
Yes, it is related.
> I did notice one interesting thing while working on this, ACPICA
> has drivers/acpi/acpica/nsinit.c: acpi_ns_initialize_objects()
> which calls _INI for *all* objects and it also has:
> drivers/acpi/acpica/utxfinit.c: acpi_initialize_objects():
>
> #ifdef ACPI_OBSOLETE_BEHAVIOR
> /*
> * 05/2019: Removed, initialization now happens at both object
> * creation and table load time
> */
>
> /*
> * Initialize the objects that remain uninitialized. This
> * runs the executable AML that may be part of the
> * declaration of these objects: operation_regions, buffer_fields,
> * bank_fields, Buffers, and Packages.
> */
> if (!(flags & ACPI_NO_OBJECT_INIT)) {
> status = acpi_ns_initialize_objects();
> if (ACPI_FAILURE(status)) {
> return_ACPI_STATUS(status);
> }
> }
> #endif
>
> Which means that at some point in time it used to indeed call
> _INI on all objects early on. But now this no longer happens
> and instead acpi_initialize_objects() ends up in acpi_ns_initialize_devices()
> which does:
>
> 1. Run \._INI
> 2. Run \_SB._INI
> 3. Run _REG for all SystemIO / SystemMemory / PCI_Config handlers
Except that after (Linux) commit 8b1cafdcb4b7 ("ACPICA: Never run _REG
on system_memory and system_IO") the former two are not run anyway.
> 4. Run _INI for all other objects
>
> And I believe that this is causing similar issues as the EC issue
> we are discussing here in some cases. Specifically the earlier
> mentioned touchpad issue:
>
> """
> On a "Lenovo Thinkbook 14-ILL" \\_SB_.PCI0.I2C0._REG gets
> evaluated before \_SB.PCI0._INI and that _REG contains:
>
> If ((OSYS == 0x07DF))
> {
> ...
> LNUX = Zero
> TPID = 0x4E
> }
> else
> {
> LNUX = One
> TPID = 0xBB
> }
>
> And then later on the TPID value gets used to decide for which of multiple
> devices describing the touchpad _STA should return 0xF and the one which
> gets enabled by TPID=0xBB is broken, causing to the touchpad to not work:
> https://bugzilla.redhat.com/show_bug.cgi?id=1842039
> """
>
> The difference between the EC problem and this one is that the troublesome
> code in the EC _REG handler is inside a:
>
> If (((Arg0 == 0x03) && (Arg1 == One)))
> {
> }
>
> block, so any non EmbeddedController OpRegion _REG calls are not an issue.
>
> Where as in the "Lenovo Thinkbook 14-ILL" touchpad case the troublesome
> _REG looks like this (simplified):
>
> Method (_REG, 2, NotSerialized) // _REG: Region Availability
> {
> If ((TPID != Zero))
> {
> Return (Zero)
> }
>
> If ((OSYS == 0x07DF))
> {
> TPID = <one we want>
> }
> Else
> {
> TPID = <wrong-one>
> }
>
> Return (Zero)
> }
>
> So it only checks OSYS the first time it runs (when TPID == 0)
> and the above mentioned steps 1-4 done by acpi_ns_initialize_devices()
> runs this _REG too early.
I think that this is a bug.
> The reasons why this runs too early is
> because the \_SB_.PCI0.I2C0 device which this is the _REG method for
> also uses a PCI_Config OpRegion, so the early _REG calls done
> by acpi_ns_initialize_devices() will run this _REG.
Hmm.
> So even if we fix the EC issue by having the EC code call _REG later
> for the EC object we still have this issue wrt general _REG vs _INI
> ordering, any suggestions welcome. Otherwise I will likely still try
> to get patches 1 + 2 from this series upstream to fix this.
>
> Or maybe we need to just make acpi_ns_initialize_devices() call
> all _INI methods before calling any _REG methods?
At least we should make sure that _INI for all ancestors have been
evaluated before evaluating _REG for a descendant (and obviously _REG
should not be evaluated before _INI for the same device).
> That does somehow feel right, but I'm not sure how well it will work in practice.
I would even say that this is mandated by the spec, but of course the
question regarding practice is a good one.
> </semi-offtopic>
>
> >>>> This not only installs the OpRegion handler, but also calls the EC's
> >>>> _REG method. The _REG method call is a problem because it may rely on
> >>>> initialization done by the _INI methods of one of the PCI / _SB root devs,
> >>>> see for example: https://bugzilla.kernel.org/show_bug.cgi?id=214899 .
> >>>>
> >>>> This _REG depends on _INI problem can be fixed by calling the new ACPICA
> >>>> acpi_early_initialize_objects() function before acpi_ec_ecdt_probe().
> >>>>
> >>>> But on some boards (e.g. Lenovo X1C8) the root devices _INI method
> >>>> relies on the EC OpRegion so executing the _INI methods before
> >>>> registering the EC OpRegion handler leads to errors there.
> >>>>
> >>>> To allow fixing this the ACPICA code now allows to do the OpRegion handler
> >>>> installation early on (without calling _REG) and to do the EC's _REG
> >>>> execution later on as a separate step.
> >>>>
> >>>> This commit uses this new ACPICA functions to fix the EC probe ordering
> >>>> by changing the acpi_bus_init() initialization order to this:
> >>>>
> >>>> 1. acpi_load_tables()
> >>>> 2. acpi_ec_ecdt_probe()
> >>>> This now calls acpi_install_address_space_handler(ACPI_NO_EXEC__REG)
> >>>> which installs the OpRegion handler without executing _REG
> >>>
> >>> I'm not sure if installing an opregion without evaluating _REG for it
> >>> is particularly useful.
> >>
> >> We already do this for the SystemMemory / SystemIO and PCI_Config OpRegions.
> >> The handlers for these get installed from acpi_enable_subsystem()
> >> and their _REG functions get called later from acpi_initialize_objects().
> >>
> >> This patch basically makes the EmbeddedControl OpRegion behave the
> >> same and according to the current code:
> >
> > I think that the evaluation of _REG can be deferred in this case.
> >
> >>
> >> /*
> >> * ACPI 2.0 requires the EC driver to be loaded and work before the EC
> >> * device is found in the namespace.
> >> *
> >> * This is accomplished by looking for the ECDT table and getting the EC
> >> * parameters out of that.
> >> *
> >> * Do that before calling acpi_initialize_objects() which may trigger EC
> >> * address space accesses.
> >> */
> >> acpi_ec_ecdt_probe();
> >>
> >> This is mandated by ACPI-2.0 which also seems to match my analysis
> >> of this problem where on e.g. my Lenovo X1 carbon gen 8
> >> the \_SB.PCI0._INI method uses an EmbeddedControl OpRegion without
> >> any _REG related checks.
> >>> No AML should use it before _REG is evaluated anyway.
> >>
> >> See above, ACPI 2.x seems to allow this, like how it allows it
> >> for SystemIO / SystemMemory / PCI_Config. This seems to be the
> >> whole reason why there is a separate table describing the EC
> >> (the ECDT) so that the EC can be made available before any parsing
> >> of the DSDT has been done.
> >>
> >> So we need to have the OpRegion available before running the
> >> "early" _INI methods. And because of _REG relying on the OSYS
> >> GVNS variable in some cases, which gets set from _INI on the
> >> PCI root bridge, that means running _REG after running
> >> (some) _INI methods.
> >
> > OK, so at least on some systems the EC address space will be accessed
> > regardless of the _REG for it.
> >
> >>>> 3. acpi_enable_subsystem()
> >>>> 4. acpi_early_initialize_objects()
> >>>> This calls the _INI method of the PCI and _SB root devices
> >>>
> >>> So this is a change in behavior that will affect every system using
> >>> ACPI on the planet, not just the ones where the problem at hand is
> >>> present. This appears to be somewhat risky to me.
> >>
> >> The ACPICA code already calls \.INI and \_SB._INI at the start of
> >> acpi_initialize_objects(), before evalutating any _REG methods,
> >> so this call in itself is no a change.
> >>
> >> Except that \_SB.PCI0._INI and \_SB.PC0._INI are now also
> >> run early (patch 2/4).
> >>>> 5. acpi_ec_ecdt_exec_reg();
> >>>> This executes the EC's _REG now that the root devices _INI has run
> >>
> >> This is the actual change moving the calling of _REG for the EC to after
> >> the running of the "early" _INI calls.
> >>
> >>>> 6. acpi_initialize_objects(ACPI_NO_EARLY_DEVICE_INIT)
> >>>>
> >>>> This allows the EC's _REG method to depend on e.g. the \OSYS global/GVNS
> >>>> variable often set by a root-device's _INI method, while at the same time
> >>>> allowing these _INI methods to access EmbeddedController OpRegions.
> >>>
> >>> I'm wondering if it is possible to change the ordering of
> >>> acpi_ec_ecdt_probe() or the part of it that installs the oprtegion
> >>> handler to be called later?
> >>
> >> Note splitting the install vs _REG calling of OpRegion handlers
> >> requires ACPICA changes (patch 3/4).
> >>
> >> Assuming those changes are in place then we could delay calling
> >> of _REG also until the actual acpi-ec driver binds. This would
> >> put it a lot later in the init sequence though. So that would
> >> be much more of a change to the ordering then done in this
> >> RFC series.
> >>
> >> I did consider this and I do think it makes sense to only call
> >> _REG once we actually have fully setup the ACPI device for
> >> the EC (rather then just parsed the ECDT as we do now), but
> >> it is a big change.
> >
> > That's what I would like to do here.
> >
> >> This would also put the _REG late enough that the _INI
> >> setting the OSYS variable has already run, avoiding
> >> the need for the "early" _INI calls from the EC _REG
> >> evaluation pov (1).
> >>
> >> If we go this route then acpi_bus_init() would not need any
> >> changes. In this case the changes would be:
> >>
> >> 1. Change acpi_ec_ecdt_probe(); to only install the
> >> OpRegion handler and not call _REG
> >>
> >> 2. Call _REG from acpi_ec_add() before it calls
> >> acpi_dev_clear_dependencies(device);
> >>
> >> If you think that is better I can implement this and ask
> >> the reporter of:
> >>
> >> https://bugzilla.kernel.org/show_bug.cgi?id=214899
> >>
> >> to give this new approach a test run.
> >
> > Yes, please.
>
> Ok, will do. Not sure when I will get around to this though.
No problem, please take your time.
1 month, 2 weeks
[rafael-pm:bleeding-edge] BUILD REGRESSION 4035647418c1b31bcc12e6cfafc01f61f0c649ea
by kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
branch HEAD: 4035647418c1b31bcc12e6cfafc01f61f0c649ea Merge branch 'acpi-bus' into linux-next
Error/Warning reports:
https://lore.kernel.org/linux-acpi/[email protected]
Error/Warning: (recently discovered and may have been fixed)
drivers/bus/hisi_lpc.c:488:41: error: 'struct acpi_device' has no member named 'children'
drivers/bus/hisi_lpc.c:488:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
|-- arm64-allyesconfig
| |-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
| `-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
`-- ia64-allmodconfig
|-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
`-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
elapsed time: 722m
configs tested: 70
configs skipped: 2
gcc tested configs:
arm64 allyesconfig
arm defconfig
arm allyesconfig
i386 randconfig-c001-20220627
powerpc ppc40x_defconfig
arm realview_defconfig
arm pleb_defconfig
parisc64 defconfig
arm pxa910_defconfig
ia64 bigsur_defconfig
arm aspeed_g5_defconfig
sh sh7770_generic_defconfig
arc hsdk_defconfig
arm cm_x300_defconfig
riscv allyesconfig
ia64 allmodconfig
x86_64 randconfig-k001-20220627
arc allyesconfig
alpha allyesconfig
m68k allmodconfig
m68k allyesconfig
powerpc allnoconfig
mips allyesconfig
powerpc allmodconfig
sh allmodconfig
i386 defconfig
i386 allyesconfig
x86_64 randconfig-a012-20220627
x86_64 randconfig-a016-20220627
x86_64 randconfig-a011-20220627
x86_64 randconfig-a013-20220627
x86_64 randconfig-a014-20220627
x86_64 randconfig-a015-20220627
i386 randconfig-a014-20220627
i386 randconfig-a011-20220627
i386 randconfig-a012-20220627
i386 randconfig-a015-20220627
i386 randconfig-a016-20220627
i386 randconfig-a013-20220627
arc randconfig-r043-20220627
s390 randconfig-r044-20220627
riscv randconfig-r042-20220627
um i386_defconfig
um x86_64_defconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 allyesconfig
x86_64 rhel-8.3-kunit
x86_64 rhel-8.3-kselftests
x86_64 rhel-8.3-syz
x86_64 rhel-8.3-func
clang tested configs:
arm pxa255-idp_defconfig
arm dove_defconfig
x86_64 randconfig-a004-20220627
x86_64 randconfig-a006-20220627
x86_64 randconfig-a001-20220627
x86_64 randconfig-a005-20220627
x86_64 randconfig-a002-20220627
x86_64 randconfig-a003-20220627
i386 randconfig-a002-20220627
i386 randconfig-a004-20220627
i386 randconfig-a003-20220627
i386 randconfig-a001-20220627
i386 randconfig-a005-20220627
i386 randconfig-a006-20220627
x86_64 randconfig-a012
x86_64 randconfig-a014
x86_64 randconfig-a016
hexagon randconfig-r041-20220627
hexagon randconfig-r045-20220627
--
0-DAY CI Kernel Test Service
https://01.org/lkp
1 month, 2 weeks
Re: [RFC 4/4] ACPI: fix ECDT EC probe ordering issues
by Rafael J. Wysocki
On Mon, Jun 27, 2022 at 5:03 PM Hans de Goede <hdegoede(a)redhat.com> wrote:
>
> Hi,
>
> On 6/27/22 16:21, Rafael J. Wysocki wrote:
> > First off, thanks for taking care of this issue!
>
> You're welcome.
>
> > On Wed, Jun 15, 2022 at 9:57 PM Hans de Goede <hdegoede(a)redhat.com> wrote:
> >>
> >> ACPI-2.0 says that the EC OpRegion handler must be available immediately
> >> (like the standard default OpRegion handlers). So acpi_bus_init() calls
> >> acpi_ec_ecdt_probe(), which calls acpi_install_address_space_handler() to
> >> install the EC's OpRegion handler, early on.
> >
> > I think that the key question is what Windows does in this respect.
> >
> > I honestly don't think that it uses an allowlist to early call _INI
> > for a few specific devices.
>
> Right, I guess that windows does things more hierarchicallly first
> calling \._INI then \._SB._INI and then calling other _INI-s around
> the time it enters there parts of the ACPI device hierarchy.
>
> I have the feeling that Windows e.g. has the root PCI bridge fully
> setup before even parsing any ACPI objects under it.
Which means that if the EC object is under PCI0, it will not evaluate
its _REG before enumerating the PCI bus.
> This is quite different from how Linux (or ACPICa for that manner)
> works. So ATM I believe that the fixed list of object paths to
> call _INI on early is the best we can do.
Well, ACPICA follows the spec quite literally in that respect (and the
spec says that _INI should be evaluated "at the table load time"
IIRC).
> >
> >> This not only installs the OpRegion handler, but also calls the EC's
> >> _REG method. The _REG method call is a problem because it may rely on
> >> initialization done by the _INI methods of one of the PCI / _SB root devs,
> >> see for example: https://bugzilla.kernel.org/show_bug.cgi?id=214899 .
> >>
> >> This _REG depends on _INI problem can be fixed by calling the new ACPICA
> >> acpi_early_initialize_objects() function before acpi_ec_ecdt_probe().
> >>
> >> But on some boards (e.g. Lenovo X1C8) the root devices _INI method
> >> relies on the EC OpRegion so executing the _INI methods before
> >> registering the EC OpRegion handler leads to errors there.
> >>
> >> To allow fixing this the ACPICA code now allows to do the OpRegion handler
> >> installation early on (without calling _REG) and to do the EC's _REG
> >> execution later on as a separate step.
> >>
> >> This commit uses this new ACPICA functions to fix the EC probe ordering
> >> by changing the acpi_bus_init() initialization order to this:
> >>
> >> 1. acpi_load_tables()
> >> 2. acpi_ec_ecdt_probe()
> >> This now calls acpi_install_address_space_handler(ACPI_NO_EXEC__REG)
> >> which installs the OpRegion handler without executing _REG
> >
> > I'm not sure if installing an opregion without evaluating _REG for it
> > is particularly useful.
>
> We already do this for the SystemMemory / SystemIO and PCI_Config OpRegions.
> The handlers for these get installed from acpi_enable_subsystem()
> and their _REG functions get called later from acpi_initialize_objects().
>
> This patch basically makes the EmbeddedControl OpRegion behave the
> same and according to the current code:
I think that the evaluation of _REG can be deferred in this case.
>
> /*
> * ACPI 2.0 requires the EC driver to be loaded and work before the EC
> * device is found in the namespace.
> *
> * This is accomplished by looking for the ECDT table and getting the EC
> * parameters out of that.
> *
> * Do that before calling acpi_initialize_objects() which may trigger EC
> * address space accesses.
> */
> acpi_ec_ecdt_probe();
>
> This is mandated by ACPI-2.0 which also seems to match my analysis
> of this problem where on e.g. my Lenovo X1 carbon gen 8
> the \_SB.PCI0._INI method uses an EmbeddedControl OpRegion without
> any _REG related checks.
> > No AML should use it before _REG is evaluated anyway.
>
> See above, ACPI 2.x seems to allow this, like how it allows it
> for SystemIO / SystemMemory / PCI_Config. This seems to be the
> whole reason why there is a separate table describing the EC
> (the ECDT) so that the EC can be made available before any parsing
> of the DSDT has been done.
>
> So we need to have the OpRegion available before running the
> "early" _INI methods. And because of _REG relying on the OSYS
> GVNS variable in some cases, which gets set from _INI on the
> PCI root bridge, that means running _REG after running
> (some) _INI methods.
OK, so at least on some systems the EC address space will be accessed
regardless of the _REG for it.
> >> 3. acpi_enable_subsystem()
> >> 4. acpi_early_initialize_objects()
> >> This calls the _INI method of the PCI and _SB root devices
> >
> > So this is a change in behavior that will affect every system using
> > ACPI on the planet, not just the ones where the problem at hand is
> > present. This appears to be somewhat risky to me.
>
> The ACPICA code already calls \.INI and \_SB._INI at the start of
> acpi_initialize_objects(), before evalutating any _REG methods,
> so this call in itself is no a change.
>
> Except that \_SB.PCI0._INI and \_SB.PC0._INI are now also
> run early (patch 2/4).
> >> 5. acpi_ec_ecdt_exec_reg();
> >> This executes the EC's _REG now that the root devices _INI has run
>
> This is the actual change moving the calling of _REG for the EC to after
> the running of the "early" _INI calls.
>
> >> 6. acpi_initialize_objects(ACPI_NO_EARLY_DEVICE_INIT)
> >>
> >> This allows the EC's _REG method to depend on e.g. the \OSYS global/GVNS
> >> variable often set by a root-device's _INI method, while at the same time
> >> allowing these _INI methods to access EmbeddedController OpRegions.
> >
> > I'm wondering if it is possible to change the ordering of
> > acpi_ec_ecdt_probe() or the part of it that installs the oprtegion
> > handler to be called later?
>
> Note splitting the install vs _REG calling of OpRegion handlers
> requires ACPICA changes (patch 3/4).
>
> Assuming those changes are in place then we could delay calling
> of _REG also until the actual acpi-ec driver binds. This would
> put it a lot later in the init sequence though. So that would
> be much more of a change to the ordering then done in this
> RFC series.
>
> I did consider this and I do think it makes sense to only call
> _REG once we actually have fully setup the ACPI device for
> the EC (rather then just parsed the ECDT as we do now), but
> it is a big change.
That's what I would like to do here.
> This would also put the _REG late enough that the _INI
> setting the OSYS variable has already run, avoiding
> the need for the "early" _INI calls from the EC _REG
> evaluation pov (1).
>
> If we go this route then acpi_bus_init() would not need any
> changes. In this case the changes would be:
>
> 1. Change acpi_ec_ecdt_probe(); to only install the
> OpRegion handler and not call _REG
>
> 2. Call _REG from acpi_ec_add() before it calls
> acpi_dev_clear_dependencies(device);
>
> If you think that is better I can implement this and ask
> the reporter of:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=214899
>
> to give this new approach a test run.
Yes, please.
1 month, 2 weeks
[rafael-pm:bleeding-edge] BUILD REGRESSION 3ea94b067c49378a574fc477647ece5c2f41dccb
by kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
branch HEAD: 3ea94b067c49378a574fc477647ece5c2f41dccb Merge branch 'acpi-bus' into bleeding-edge
Error/Warning reports:
https://lore.kernel.org/linux-acpi/[email protected]
Error/Warning: (recently discovered and may have been fixed)
drivers/bus/hisi_lpc.c:488:41: error: 'struct acpi_device' has no member named 'children'
drivers/bus/hisi_lpc.c:488:53: error: 'struct acpi_device' has no member named 'node'; did you mean 'fwnode'?
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
`-- arm64-allyesconfig
|-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-children
`-- drivers-bus-hisi_lpc.c:error:struct-acpi_device-has-no-member-named-node
elapsed time: 726m
configs tested: 52
configs skipped: 2
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm allyesconfig
ia64 allmodconfig
m68k allyesconfig
arc allyesconfig
alpha allyesconfig
m68k allmodconfig
mips allyesconfig
powerpc allmodconfig
powerpc allnoconfig
sh allmodconfig
i386 defconfig
i386 allyesconfig
x86_64 randconfig-a012-20220627
x86_64 randconfig-a016-20220627
x86_64 randconfig-a011-20220627
x86_64 randconfig-a013-20220627
x86_64 randconfig-a014-20220627
x86_64 randconfig-a015-20220627
i386 randconfig-a012-20220627
i386 randconfig-a015-20220627
i386 randconfig-a011-20220627
i386 randconfig-a016-20220627
i386 randconfig-a013-20220627
i386 randconfig-a014-20220627
arc randconfig-r043-20220627
s390 randconfig-r044-20220627
riscv randconfig-r042-20220627
um i386_defconfig
um x86_64_defconfig
x86_64 rhel-8.3-func
x86_64 rhel-8.3-kunit
x86_64 rhel-8.3-kselftests
x86_64 rhel-8.3-syz
x86_64 defconfig
x86_64 rhel-8.3
x86_64 allyesconfig
clang tested configs:
x86_64 randconfig-a002-20220627
x86_64 randconfig-a003-20220627
x86_64 randconfig-a001-20220627
x86_64 randconfig-a006-20220627
x86_64 randconfig-a005-20220627
x86_64 randconfig-a004-20220627
i386 randconfig-a002-20220627
i386 randconfig-a004-20220627
i386 randconfig-a003-20220627
i386 randconfig-a001-20220627
i386 randconfig-a006-20220627
i386 randconfig-a005-20220627
hexagon randconfig-r041-20220627
hexagon randconfig-r045-20220627
--
0-DAY CI Kernel Test Service
https://01.org/lkp
1 month, 2 weeks