From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Matthieu Baerts <matthieu.baerts@tessares.net>
Cc: Linux ACPI <linux-acpi@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Erik Kaneda <erik.kaneda@intel.com>,
Bob Moore <robert.moore@intel.com>
Subject: Re: [PATCH 4/6] ACPICA: Add support for using logical addresses of GPE blocks
Date: Fri, 16 Oct 2020 19:15:20 +0200 [thread overview]
Message-ID: <1735226.fPvPZg4QOa@kreacher> (raw)
In-Reply-To: <3b69e0d0-fb8a-92b4-42fd-f2a8fcdd642b@tessares.net>
On Friday, October 16, 2020 4:30:55 PM CEST Matthieu Baerts wrote:
> Hi Rafael,
Hi,
> On 04/09/2020 19:24, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > The logical address of every GPE block in system memory must be
> > known before passing it to acpi_ev_initialize_gpe_block(), because
> > memory cannot be mapped on the fly from an interrupt handler.
> > Accordingly, the host OS must map every GPE block in system
> > memory upfront and it can store the logical addresses of GPE
> > blocks for future use.
>
> (...)
>
> > diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
> > index a0e71f34c77a..37bb67ef3232 100644
> > --- a/drivers/acpi/acpica/hwgpe.c
> > +++ b/drivers/acpi/acpica/hwgpe.c
> > @@ -46,8 +46,13 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg)
> > u32 value32;
> >
> > if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > +#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
> > + *value = (u64)ACPI_GET8(reg->address);
>
> Thank you for the patch!
>
> When compiling net-next repo, recently sync with Linus repo, I got an
> error when using i386 arch because of this line above.
>
> Here are the commands I used:
>
>
> ================================================
> $ make defconfig KBUILD_DEFCONFIG=i386_defconfig
> *** Default configuration is based on 'i386_defconfig'
> #
> # configuration written to .config
> #
> $ scripts/config --disable DRM --disable PCCARD --disable ATA --disable
> MD --disable PPS --disable SOUND --disable USB --disable IOMMU_SUPPORT
> --disable INPUT_LEDS --disable AGP --disable VGA_ARB --disable EFI
> --disable WLAN --disable WIRELESS --disable LOGO --disable NFS_FS
> --disable XFRM_USER --disable INET6_AH --disable INET6_ESP --disable
> NETDEVICES -e KUNIT -d KUNIT_DEBUGFS -d KUNIT_TEST -d KUNIT_EXAMPLE_TEST
> -d EXT4_KUNIT_TESTS -d SYSCTL_KUNIT_TEST -d LIST_KUNIT_TEST -d
> LINEAR_RANGES_TEST -d BITS_TEST -d KUNIT_ALL_TESTS -e INET_DIAG -d
> INET_UDP_DIAG -d INET_RAW_DIAG -d INET_DIAG_DESTROY -e MPTCP -e
> MPTCP_IPV6 -e MPTCP_KUNIT_TESTS
> $ KCFLAGS=-Werror make -j8 -l8
> scripts/kconfig/conf --syncconfig Kconfig
> (...)
> CC drivers/acpi/acpica/hwgpe.o
> In file included from ./include/acpi/acpi.h:24,
> from drivers/acpi/acpica/hwgpe.c:10:
> drivers/acpi/acpica/hwgpe.c: In function 'acpi_hw_gpe_read':
> ./include/acpi/actypes.h:501:48: error: cast to pointer from integer of
> different size [-Werror=int-to-pointer-cast]
> 501 | #define ACPI_CAST_PTR(t, p) ((t *) (acpi_uintptr_t)
> (p))
> | ^
> drivers/acpi/acpica/acmacros.h:18:41: note: in expansion of macro
> 'ACPI_CAST_PTR'
> 18 | #define ACPI_CAST8(ptr) ACPI_CAST_PTR (u8, (ptr))
> | ^~~~~~~~~~~~~
> drivers/acpi/acpica/acmacros.h:22:43: note: in expansion of macro
> 'ACPI_CAST8'
> 22 | #define ACPI_GET8(ptr) (*ACPI_CAST8 (ptr))
> | ^~~~~~~~~~
> drivers/acpi/acpica/hwgpe.c:50:17: note: in expansion of macro 'ACPI_GET8'
> 50 | *value = (u64)ACPI_GET8(reg->address);
> | ^~~~~~~~~
> drivers/acpi/acpica/hwgpe.c: In function 'acpi_hw_gpe_write':
> ./include/acpi/actypes.h:501:48: error: cast to pointer from integer of
> different size [-Werror=int-to-pointer-cast]
> 501 | #define ACPI_CAST_PTR(t, p) ((t *) (acpi_uintptr_t)
> (p))
> | ^
> drivers/acpi/acpica/acmacros.h:18:41: note: in expansion of macro
> 'ACPI_CAST_PTR'
> 18 | #define ACPI_CAST8(ptr) ACPI_CAST_PTR (u8, (ptr))
> | ^~~~~~~~~~~~~
> drivers/acpi/acpica/acmacros.h:26:43: note: in expansion of macro
> 'ACPI_CAST8'
> 26 | #define ACPI_SET8(ptr, val) (*ACPI_CAST8 (ptr) =
> (u8) (val))
> | ^~~~~~~~~~
> drivers/acpi/acpica/hwgpe.c:85:3: note: in expansion of macro 'ACPI_SET8'
> 85 | ACPI_SET8(reg->address, value);
> | ^~~~~~~~~
> cc1: all warnings being treated as errors
This is what causes the build to terminate.
> make[3]: *** [scripts/Makefile.build:283: drivers/acpi/acpica/hwgpe.o]
> Error 1
> make[2]: *** [scripts/Makefile.build:500: drivers/acpi/acpica] Error 2
> make[1]: *** [scripts/Makefile.build:500: drivers/acpi] Error 2
> make: *** [Makefile:1777: drivers] Error 2
> ================================================
>
>
> > + return_ACPI_STATUS(AE_OK);
> > +#else
> > return acpi_os_read_memory((acpi_physical_address)reg->address,
> > value, ACPI_GPE_REGISTER_WIDTH);
> > +#endif
> > }
> >
> > status = acpi_os_read_port((acpi_io_address)reg->address,
> > @@ -76,8 +81,13 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg)
> > acpi_status acpi_hw_gpe_write(u64 value, struct acpi_gpe_address *reg)
> > {
> > if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > +#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
> > + ACPI_SET8(reg->address, value);
>
> (and also because of this line)
>
> By chance, do you already have a fix for that?
Can you please try the appended patch?
> I didn't see any other
> email related to this issue, I am surprised no bot already reported the
> problem but maybe I didn't look everywhere :)
No, they didn't AFAICS.
---
drivers/acpi/acpica/hwgpe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/acpi/acpica/hwgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwgpe.c
+++ linux-pm/drivers/acpi/acpica/hwgpe.c
@@ -47,7 +47,7 @@ acpi_status acpi_hw_gpe_read(u64 *value,
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
- *value = (u64)ACPI_GET8(reg->address);
+ *value = (u64)ACPI_GET8((unsigned long)reg->address);
return_ACPI_STATUS(AE_OK);
#else
return acpi_os_read_memory((acpi_physical_address)reg->address,
@@ -82,7 +82,7 @@ acpi_status acpi_hw_gpe_write(u64 value,
{
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
- ACPI_SET8(reg->address, value);
+ ACPI_SET8((unsigned long)reg->address, value);
return_ACPI_STATUS(AE_OK);
#else
return acpi_os_write_memory((acpi_physical_address)reg->address,
next prev parent reply other threads:[~2020-10-16 17:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 17:19 [PATCH 0/6] ACPICA / ACPI: OSL: Rework GPE registers access code Rafael J. Wysocki
2020-09-04 17:21 ` [PATCH 1/6] ACPICA: Validate GPE blocks at init time Rafael J. Wysocki
2020-09-04 17:22 ` [PATCH 2/6] ACPICA: Introduce acpi_hw_gpe_read() and acpi_hw_gpe_write() Rafael J. Wysocki
2020-09-04 17:23 ` [PATCH 3/6] ACPICA: Introduce special struct type for GPE register addresses Rafael J. Wysocki
2020-09-04 17:24 ` [PATCH 4/6] ACPICA: Add support for using logical addresses of GPE blocks Rafael J. Wysocki
2020-10-16 14:30 ` Matthieu Baerts
2020-10-16 17:15 ` Rafael J. Wysocki [this message]
2020-10-16 18:12 ` Matthieu Baerts
2020-09-04 17:24 ` [PATCH 5/6] ACPI: OSL: Change the type of acpi_os_map_generic_address() return value Rafael J. Wysocki
2020-09-04 17:25 ` [PATCH 6/6] ACPI: OSL: Make ACPICA use logical addresses of GPE blocks Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1735226.fPvPZg4QOa@kreacher \
--to=rjw@rjwysocki.net \
--cc=erik.kaneda@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matthieu.baerts@tessares.net \
--cc=robert.moore@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).