Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> While binding drivers directly to struct acpi_device objects allows
> basic functionality to be provided, at least in the majority of cases,
> there are some problems with it, related to general consistency, sysfs
> layout, power management operation ordering, and code cleanliness.
>
> Overall, it is better to bind drivers to platform devices than to their
> ACPI companions, so convert the ACPI NFIT core driver to a platform one.
>
> While this is not expected to alter functionality, it changes sysfs
> layout and so it will be visible to user space.
I'm not sure right off why but when I run the libndctl test with this patch I
get the following panic.
[ 17.483472] BUG: kernel NULL pointer dereference, address: 0000000000000008
[ 17.484116] #PF: supervisor read access in kernel mode
[ 17.484593] #PF: error_code(0x0000) - not-present page
[ 17.485056] PGD 9def067 P4D 9def067 PUD 9df3067 PMD 0
[ 17.485516] Oops: Oops: 0000 [#1] SMP NOPTI
[ 17.485886] CPU: 2 UID: 0 PID: 1191 Comm: libndctl Tainted: G O
6.18.0ira+ #34 PREEMPT(voluntary)
[ 17.486804] Tainted: [O]=OOT_MODULE
[ 17.487125] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
edk2-20250812-18.fc42 08/12/2025
[ 17.487749] RIP: 0010:acpi_nfit_ctl+0x40b/0xa00 [nfit]
[ 17.488222] Code: 48 48 c7 44 24 28 28 f1 8c a1 48 8b 83 c8 01 00 00 44 89
e7 48 89 44 24 50 e8 01 83 fd ff 48 c7 44 24 40 10 58 8c a1 48 89 c3 <49> 8b 47
08 48 c7 44 24 30 30 f1 8c a1 48 89 44 24 18 e9 24 fd
ff
[ 17.491668] RSP: 0018:ffffc9000f11ba28 EFLAGS: 00010286
[ 17.492422] RAX: ffffffffa18679f0 RBX: ffffffffa18679f0 RCX: ffffc9000f11bb40
[ 17.492903] RDX: 000000000000041e RSI: ffffffffa18cf116 RDI: 0000000000000003
[ 17.493408] RBP: ffffc9000f11bb40 R08: 0000000000000008 R09: ffffc9000f11bafc
[ 17.493888] R10: ffffc9000f11bae0 R11: 0000000000004019 R12: 0000000000000003
[ 17.494396] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 17.494878] FS: 00007f432f5fd7c0(0000) GS:ffff8880f9fdd000(0000)
knlGS:0000000000000000
[ 17.495436] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 17.495826] CR2: 0000000000000008 CR3: 0000000009e0c005 CR4: 0000000000770ef0
[ 17.496324] PKRU: 55555554
[ 17.496516] Call Trace:
[ 17.496691] <TASK>
[ 17.496844] ? __kmalloc_noprof+0x410/0x650
[ 17.497138] ? setup_result+0x1b/0xa0 [nfit_test]
[ 17.497474] nfit_ctl_test+0x21a/0x780 [nfit_test]
[ 17.497803] ? preempt_count_add+0x51/0xd0
[ 17.498086] ? up_write+0x13/0x60
[ 17.498333] ? up_write+0x35/0x60
[ 17.498565] ? preempt_count_add+0x51/0xd0
[ 17.498846] ? kernfs_next_descendant_post+0x1b/0xe0
[ 17.499196] nfit_test_probe+0x350/0x4d0 [nfit_test]
[ 17.499535] platform_probe+0x38/0x70
[ 17.499791] really_probe+0xde/0x380
[ 17.500039] ? _raw_spin_unlock_irq+0x18/0x40
[ 17.500354] __driver_probe_device+0xc0/0x150
[ 17.500656] driver_probe_device+0x1f/0xa0
[ 17.500939] ?
__pfx___driver_attach+0x10/0x10
[ 17.501263] __driver_attach+0xc7/0x200
[ 17.501529] bus_for_each_dev+0x63/0xa0
[ 17.501794] bus_add_driver+0x114/0x200
[ 17.502059] driver_register+0x71/0xe0
[ 17.502480] nfit_test_init+0x24e/0xff0 [nfit_test]
[ 17.502956] ? __pfx_nfit_test_init+0x10/0x10 [nfit_test]
[ 17.503483] do_one_initcall+0x42/0x210
[ 17.503891] do_init_module+0x62/0x230
[ 17.504296] init_module_from_file+0xb1/0xe0
[ 17.504732] idempotent_init_module+0xed/0x2d0
[ 17.505184] __x64_sys_finit_module+0x6d/0xe0
[ 17.505626] do_syscall_64+0x62/0x390
[ 17.506016] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 17.506563] RIP: 0033:0x7f432f8920cd
[ 17.506946] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89
f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d 03 4d 0f 00 f7 d8 64 89 01
48
[ 17.508548] RSP: 002b:00007fff0a6ccd98 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 17.509209] RAX: ffffffffffffffda RBX: 000000001f5def50 RCX: 00007f432f8920cd
[ 17.509831] RDX: 0000000000000000 RSI: 00007f432f9aa965 RDI:
0000000000000003
[
17.510472] RBP: 00007fff0a6cce50 R08: 0000000000000000 R09: 00007fff0a6cce00
[ 17.511091] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000020000
[ 17.511727] R13: 000000001f5deb60 R14: 00007f432f9aa965 R15: 0000000000000000
[ 17.512353] </TASK>
[ 17.512638] Modules linked in: nfit_test(O+) device_dax(O) nd_pmem(O)
dax_pmem(O) kmem nd_btt(O) nfit(O) dax_cxl cxl_pci nd_e820(O) cxl_mock_mem(O)
cxl_test(O) cxl_mem(O) cxl_pmem(O) cxl_acpi(O) cxl_port(O) cx
l_mock(O) libnvdimm(O) nfit_test_iomap(O) cxl_core(O) fwctl
[ 17.514512] CR2: 0000000000000008
[ 17.514878] ---[ end trace 0000000000000000 ]---
I'll try and find some time to dig into it but perhaps yall have a quick
idea of what it could be?
Ira
>
> This change was mostly developed by Michal Wilczynski [1].
>
> Linu:
> https://lore.kernel.org/linux-acpi/[email protected]/
> [1]
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/nfit/core.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2,6 +2,7 @@
> /*
> * Copyright(c) 2013-2015 Intel Corporation. All rights reserved.
> */
> +#include <linux/platform_device.h>
> #include <linux/list_sort.h>
> #include <linux/libnvdimm.h>
> #include <linux/module.h>
> @@ -98,7 +99,7 @@ static struct acpi_device *to_acpi_dev(s
> || strcmp(nd_desc->provider_name, "ACPI.NFIT") != 0)
> return NULL;
>
> - return to_acpi_device(acpi_desc->dev);
> + return ACPI_COMPANION(acpi_desc->dev);
> }
>
> static int xlat_bus_status(void *buf, unsigned int cmd, u32 status)
> @@ -3283,11 +3284,11 @@ static void acpi_nfit_put_table(void *ta
>
> static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
> {
> - struct acpi_device *adev = data;
> + struct device *dev = data;
>
> - device_lock(&adev->dev);
> - __acpi_nfit_notify(&adev->dev, handle, event);
> - device_unlock(&adev->dev);
> + device_lock(dev);
> + __acpi_nfit_notify(dev, handle, event);
> + device_unlock(dev);
> }
>
> static void acpi_nfit_remove_notify_handler(void *data)
> @@ -3328,18 +3329,19 @@ void acpi_nfit_shutdown(void *data)
> }
> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
>
> -static int acpi_nfit_add(struct acpi_device *adev)
> +static int acpi_nfit_probe(struct platform_device *pdev)
> {
> struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> struct acpi_nfit_desc *acpi_desc;
> - struct device *dev = &adev->dev;
> + struct device *dev = &pdev->dev;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> struct acpi_table_header *tbl;
> acpi_status status = AE_OK;
> acpi_size sz;
> int rc = 0;
>
> rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> - acpi_nfit_notify, adev);
> + acpi_nfit_notify, dev);
> if (rc)
> return rc;
>
> @@ -3369,7 +3371,7 @@ static int acpi_nfit_add(struct acpi_dev
> acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL);
> if (!acpi_desc)
> return -ENOMEM;
> - acpi_nfit_desc_init(acpi_desc, &adev->dev);
> + acpi_nfit_desc_init(acpi_desc, dev);
>
> /* Save the acpi header for exporting the revision via sysfs */
> acpi_desc->acpi_header = *tbl;
> @@ -3474,11 +3476,11 @@ static const struct acpi_device_id acpi_
> };
> MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
>
> -static struct acpi_driver acpi_nfit_driver = {
> - .name = KBUILD_MODNAME,
> - .ids = acpi_nfit_ids,
> - .ops = {
> - .add = acpi_nfit_add,
> +static struct platform_driver acpi_nfit_driver = {
> + .probe = acpi_nfit_probe,
> + .driver = {
> + .name = "acpi-nfit",
> + .acpi_match_table = acpi_nfit_ids,
> },
> };
>
> @@ -3516,7 +3518,7 @@ static __init int nfit_init(void)
> return -ENOMEM;
>
> nfit_mce_register();
> - ret = acpi_bus_register_driver(&acpi_nfit_driver);
> + ret = platform_driver_register(&acpi_nfit_driver);
> if (ret) {
> nfit_mce_unregister();
> destroy_workqueue(nfit_wq);
> @@ -3529,7 +3531,7 @@ static __init int nfit_init(void)
> static __exit void nfit_exit(void)
> {
> nfit_mce_unregister();
> - acpi_bus_unregister_driver(&acpi_nfit_driver);
> + platform_driver_unregister(&acpi_nfit_driver);
> destroy_workqueue(nfit_wq);
> WARN_ON(!list_empty(&acpi_descs));
> }
>
>
>
