From: "Thomas Weißschuh" <linux@weissschuh.net>
To: Arnd Bergmann <arnd@kernel.org>
Cc: "Srinivas Pandruvada" <srinivas.pandruvada@linux.intel.com>,
"Jiri Kosina" <jikos@kernel.org>,
"Benjamin Tissoires" <benjamin.tissoires@redhat.com>,
"Benson Leung" <bleung@chromium.org>,
"Sumesh K Naduvalath" <sumesh.k.naduvalath@intel.com>,
"Hans de Goede" <hdegoede@redhat.com>,
"Mark Gross" <markgross@kernel.org>,
"Nathan Chancellor" <nathan@kernel.org>,
"Nick Desaulniers" <ndesaulniers@google.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Guenter Roeck" <groeck@chromium.org>,
"Lee Jones" <lee.jones@linaro.org>,
"Uwe Kleine-König" <uwe@kleine-koenig.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
platform-driver-x86@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH] HID: intel-ish-hid: fix module device-id handling
Date: Thu, 11 Nov 2021 11:34:49 +0100 [thread overview]
Message-ID: <b87d00eb-1d89-4241-a631-1cb70ec7550e@t-8ch.de> (raw)
In-Reply-To: <20211111085842.2846422-1-arnd@kernel.org>
Hi,
On 2021-11-11 09:56+0100, Arnd Bergmann wrote:
> A late addititon to the intel-ish-hid framework caused a build failure
> with clang, and introduced an ABI to the module loader that stops working
> if any driver ever needs to bind to more than one UUID:
>
> drivers/hid/intel-ish-hid/ishtp-fw-loader.c:1067:4: error: initializer element is not a compile-time constant
>
> Change the ishtp_device_id to have correct documentation and a driver_data
> field like all the other ones, and change the drivers to use the ID table
> as the primary identification in a way that works with all compilers
> and avoids duplciating the identifiers.
I was not aware that the missing driver_data would actually be part of the ABI.
Thanks for noticing!
> Fixes: f155dfeaa4ee ("platform/x86: isthp_eclite: only load for matching devices")
> Fixes: facfe0a4fdce ("platform/chrome: chros_ec_ishtp: only load for matching devices")
> Fixes: 0d0cccc0fd83 ("HID: intel-ish-hid: hid-client: only load for matching devices")
> Fixes: 44e2a58cb880 ("HID: intel-ish-hid: fw-loader: only load for matching devices")
> Fixes: cb1a2c6847f7 ("HID: intel-ish-hid: use constants for modaliases")
> Fixes: fa443bc3c1e4 ("HID: intel-ish-hid: add support for MODULE_DEVICE_TABLE()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I did not see this reported on the list so far, but it has probably
> shown up in other build bots as well.
The warning/error was also reported by Nathan here:
https://lore.kernel.org/lkml/YYv22iWQ7yTfMNC5@archlinux-ax161/
> There are lots of ways to fix the warning, I picked this way to address
> the more urgent problem of fixing the mod_devicetable.h format before
> it is too late for that.
> ---
> drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 19 ++++++++-----------
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 19 ++++++++-----------
> drivers/hid/intel-ish-hid/ishtp/bus.c | 2 +-
> drivers/platform/chrome/cros_ec_ishtp.c | 19 ++++++++-----------
> drivers/platform/x86/intel/ishtp_eclite.c | 19 ++++++++-----------
> include/linux/intel-ish-client-if.h | 4 ++--
> include/linux/mod_devicetable.h | 5 +++--
> 7 files changed, 38 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ishtp_eclite.c b/drivers/platform/x86/intel/ishtp_eclite.c
> index b9fb8f28fd63..b2da3a4b3e1b 100644
> --- a/drivers/platform/x86/intel/ishtp_eclite.c
> +++ b/drivers/platform/x86/intel/ishtp_eclite.c
> @@ -93,9 +93,12 @@ struct ishtp_opregion_dev {
> };
>
> /* eclite ishtp client UUID: 6a19cc4b-d760-4de3-b14d-f25ebd0fbcd9 */
> -static const guid_t ecl_ishtp_guid =
> - GUID_INIT(0x6a19cc4b, 0xd760, 0x4de3,
> - 0xb1, 0x4d, 0xf2, 0x5e, 0xbd, 0xf, 0xbc, 0xd9);
> +static const struct ishtp_device_id ecl_ishtp_id_table[] = {
> + { .guid = GUID_INIT(0x6a19cc4b, 0xd760, 0x4de3,
> + 0xb1, 0x4d, 0xf2, 0x5e, 0xbd, 0xf, 0xbc, 0xd9), },
> + { }
> +};
> +MODULE_DEVICE_TABLE(ishtp, ecl_ishtp_id_table);
>
> /* ACPI DSM UUID: 91d936a7-1f01-49c6-a6b4-72f00ad8d8a5 */
> static const guid_t ecl_acpi_guid =
> @@ -462,7 +465,7 @@ static int ecl_ishtp_cl_init(struct ishtp_cl *ecl_ishtp_cl)
> ishtp_set_tx_ring_size(ecl_ishtp_cl, ECL_CL_TX_RING_SIZE);
> ishtp_set_rx_ring_size(ecl_ishtp_cl, ECL_CL_RX_RING_SIZE);
>
> - fw_client = ishtp_fw_cl_get_client(dev, &ecl_ishtp_guid);
> + fw_client = ishtp_fw_cl_get_client(dev, &ecl_ishtp_id_table[0].guid);
> if (!fw_client) {
> dev_err(cl_data_to_dev(opr_dev), "fw client not found\n");
> return -ENOENT;
> @@ -674,19 +677,13 @@ static const struct dev_pm_ops ecl_ishtp_pm_ops = {
>
> static struct ishtp_cl_driver ecl_ishtp_cl_driver = {
> .name = "ishtp-eclite",
> - .guid = &ecl_ishtp_guid,
> + .id = &ecl_ishtp_id_table,
This '&' should not be there.
drivers/platform/x86/intel/ishtp_eclite.c:680:15: error: initialization of ‘const struct ishtp_device_id *’ from incompatible pointer type ‘const struct ishtp_device_id (*)[2]’ [-Werror=incompatible-pointer-types]
680 | .id = &ecl_ishtp_id_table,
| ^
drivers/platform/x86/intel/ishtp_eclite.c:680:15: note: (near initialization for ‘ecl_ishtp_cl_driver.id’)
> .probe = ecl_ishtp_cl_probe,
> .remove = ecl_ishtp_cl_remove,
> .reset = ecl_ishtp_cl_reset,
> .driver.pm = &ecl_ishtp_pm_ops,
> };
>
> -static const struct ishtp_device_id ecl_ishtp_id_table[] = {
> - { ecl_ishtp_guid },
> - { }
> -};
> -MODULE_DEVICE_TABLE(ishtp, ecl_ishtp_id_table);
> -
> static int __init ecl_ishtp_init(void)
> {
> return ishtp_cl_driver_register(&ecl_ishtp_cl_driver, THIS_MODULE);
Thomas
next prev parent reply other threads:[~2021-11-11 10:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-11 8:56 [PATCH] HID: intel-ish-hid: fix module device-id handling Arnd Bergmann
2021-11-11 9:29 ` Hans de Goede
2021-11-11 10:34 ` Thomas Weißschuh [this message]
2021-11-11 16:25 ` Jiri Kosina
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=b87d00eb-1d89-4241-a631-1cb70ec7550e@t-8ch.de \
--to=linux@weissschuh.net \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=benjamin.tissoires@redhat.com \
--cc=bleung@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@chromium.org \
--cc=hdegoede@redhat.com \
--cc=jikos@kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=markgross@kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=sumesh.k.naduvalath@intel.com \
--cc=uwe@kleine-koenig.org \
/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).