[PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Andy Shevchenko]

Replace open coded device_get_match_data() in acpi_smbus_cmi_add().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: used device_get_match_data()
v3: fixed couple of warnings (LKP)
 drivers/i2c/busses/i2c-scmi.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index 6746aa46d96c..79798fc7462a 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -30,7 +30,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
-	struct smbus_methods_t *methods;
+	const struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -361,7 +361,6 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
-	const struct acpi_device_id *id;
 	int ret;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
@@ -369,6 +368,7 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 		return -ENOMEM;
 
 	smbus_cmi->handle = device->handle;
+	smbus_cmi->methods = device_get_match_data(&device->dev);
 	strcpy(acpi_device_name(device), ACPI_SMBUS_HC_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_SMBUS_HC_CLASS);
 	device->driver_data = smbus_cmi;
@@ -376,11 +376,6 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
-	for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
-		if (!strcmp(id->id, acpi_device_hid(device)))
-			smbus_cmi->methods =
-				(struct smbus_methods_t *) id->driver_data;
-
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, NULL, smbus_cmi, NULL);
 
-- 
2.35.1

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

On Fri, Jul 08, 2022 at 03:09:58PM +0300, Andy Shevchenko wrote:
> Replace open coded device_get_match_data() in acpi_smbus_cmi_add().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Josef Johansson]

On 8/25/22 19:41, Andy Shevchenko wrote:
> On Thu, Aug 25, 2022 at 06:54:29PM +0200, Josef Johansson wrote:
>> On 8/25/22 17:14, Andy Shevchenko wrote:
>>> On Tue, Aug 16, 2022 at 08:03:33AM +0200, Josef Johansson wrote:
>>>> On 7/8/22 14:09, Andy Shevchenko wrote:
>>>> I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
>>>>
>>>> Since I do not have any way of capturing the error except camera (it's a long message), here's my best try.
>>>>
>>>> BUG: kernel NULL pointer dereference, address: 0000000000000000
>>> I think I know what happens there. Can you test a hackpatch (below)
>>> with my patch _not being reverted_?
>>>
>>> diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
>>> index 79798fc7462a..3ed1758706b0 100644
>>> --- a/drivers/i2c/busses/i2c-scmi.c
>>> +++ b/drivers/i2c/busses/i2c-scmi.c
>>> @@ -426,6 +426,9 @@ static struct acpi_driver acpi_smbus_cmi_driver = {
>>>    		.add = acpi_smbus_cmi_add,
>>>    		.remove = acpi_smbus_cmi_remove,
>>>    	},
>>> +	.drv = {
>>> +		.acpi_match_table = acpi_smbus_cmi_ids,
>>> +	},
>>>    };
>>>    module_acpi_driver(acpi_smbus_cmi_driver);
>>>
>> Hi,
>>
>> I applied your patch and the system boots now. Good job :)
> Yeah, but the initial conversion will be transformed to much bigger since
> maintainer doesn't want the fix in the ACPI glue code for the (deprecated)
> ACPI drivers.
>
> So there will be no patches soon, but I will Cc you to anything
> I will come up with in the future. Thanks for testing and reporting!
>
Oh, ok! Yeah for sure, obviously I have a pretty broken setup :)

Good luck with the bigger transformation!

Regards
- Josef

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Andy Shevchenko]

On Thu, Aug 25, 2022 at 06:54:29PM +0200, Josef Johansson wrote:
> On 8/25/22 17:14, Andy Shevchenko wrote:
> > On Tue, Aug 16, 2022 at 08:03:33AM +0200, Josef Johansson wrote:
> > > On 7/8/22 14:09, Andy Shevchenko wrote:
> > > I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
> > > 
> > > Since I do not have any way of capturing the error except camera (it's a long message), here's my best try.
> > > 
> > > BUG: kernel NULL pointer dereference, address: 0000000000000000
> > I think I know what happens there. Can you test a hackpatch (below)
> > with my patch _not being reverted_?
> > 
> > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
> > index 79798fc7462a..3ed1758706b0 100644
> > --- a/drivers/i2c/busses/i2c-scmi.c
> > +++ b/drivers/i2c/busses/i2c-scmi.c
> > @@ -426,6 +426,9 @@ static struct acpi_driver acpi_smbus_cmi_driver = {
> >   		.add = acpi_smbus_cmi_add,
> >   		.remove = acpi_smbus_cmi_remove,
> >   	},
> > +	.drv = {
> > +		.acpi_match_table = acpi_smbus_cmi_ids,
> > +	},
> >   };
> >   module_acpi_driver(acpi_smbus_cmi_driver);
> > 
> Hi,
> 
> I applied your patch and the system boots now. Good job :)

Yeah, but the initial conversion will be transformed to much bigger since
maintainer doesn't want the fix in the ACPI glue code for the (deprecated)
ACPI drivers.

So there will be no patches soon, but I will Cc you to anything
I will come up with in the future. Thanks for testing and reporting!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Josef Johansson]

On 8/25/22 17:14, Andy Shevchenko wrote:
> On Tue, Aug 16, 2022 at 08:03:33AM +0200, Josef Johansson wrote:
>> On 7/8/22 14:09, Andy Shevchenko wrote:
>> I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
>>
>> Since I do not have any way of capturing the error except camera (it's a long message), here's my best try.
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000000
> I think I know what happens there. Can you test a hackpatch (below)
> with my patch _not being reverted_?
>
> diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
> index 79798fc7462a..3ed1758706b0 100644
> --- a/drivers/i2c/busses/i2c-scmi.c
> +++ b/drivers/i2c/busses/i2c-scmi.c
> @@ -426,6 +426,9 @@ static struct acpi_driver acpi_smbus_cmi_driver = {
>   		.add = acpi_smbus_cmi_add,
>   		.remove = acpi_smbus_cmi_remove,
>   	},
> +	.drv = {
> +		.acpi_match_table = acpi_smbus_cmi_ids,
> +	},
>   };
>   module_acpi_driver(acpi_smbus_cmi_driver);
>   
>
Hi,

I applied your patch and the system boots now. Good job :)

Regards
- Josef

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Andy Shevchenko]

On Tue, Aug 16, 2022 at 08:03:33AM +0200, Josef Johansson wrote:
> On 7/8/22 14:09, Andy Shevchenko wrote:

> I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
> 
> Since I do not have any way of capturing the error except camera (it's a long message), here's my best try.
> 
> BUG: kernel NULL pointer dereference, address: 0000000000000000

I think I know what happens there. Can you test a hackpatch (below)
with my patch _not being reverted_?

diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index 79798fc7462a..3ed1758706b0 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -426,6 +426,9 @@ static struct acpi_driver acpi_smbus_cmi_driver = {
 		.add = acpi_smbus_cmi_add,
 		.remove = acpi_smbus_cmi_remove,
 	},
+	.drv = {
+		.acpi_match_table = acpi_smbus_cmi_ids,
+	},
 };
 module_acpi_driver(acpi_smbus_cmi_driver);
 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Andy Shevchenko]

On Tue, Aug 16, 2022 at 12:18:03PM +0200, Wolfram Sang wrote:
> Hi,
> 
> > I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
> 
> Thank you for the report and the provided information. I will give Andy
> a bit of time to come up with a fix. If a fix is not easily possible,
> this will get reverted for rc2.

Haven't seen in the thread information about revert done, hence if nobody
respond in this way, I will look into this starting from tomorrow (sorry,
was on a sick leave).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]


> Not verified by anyone but me so it could be me just having odd kernel
> config.

I also love odd kernel configs to find bugs :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Josef Johansson]

On 8/16/22 12:18, Wolfram Sang wrote:
> Hi,
>
>> I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.
> Thank you for the report and the provided information. I will give Andy
> a bit of time to come up with a fix. If a fix is not easily possible,
> this will get reverted for rc2.
>
> Thanks again,
>
>     Wolfram
>
No problem, I'm just testing out tip.

Not verified by anyone but me so it could be me just having odd kernel 
config.

Regards
- Josef

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

Hi,

> I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.

Thank you for the report and the provided information. I will give Andy
a bit of time to come up with a fix. If a fix is not easily possible,
this will get reverted for rc2.

Thanks again,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] i2c: scmi: Replace open coded device_get_match_data()

[Josef Johansson]

On 7/8/22 14:09, Andy Shevchenko wrote:

> Replace open coded device_get_match_data() in acpi_smbus_cmi_add().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: used device_get_match_data()
> v3: fixed couple of warnings (LKP)
>   drivers/i2c/busses/i2c-scmi.c | 9 ++-------
>   1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
> index 6746aa46d96c..79798fc7462a 100644
> --- a/drivers/i2c/busses/i2c-scmi.c
> +++ b/drivers/i2c/busses/i2c-scmi.c
> @@ -30,7 +30,7 @@ struct acpi_smbus_cmi {
>   	u8 cap_info:1;
>   	u8 cap_read:1;
>   	u8 cap_write:1;
> -	struct smbus_methods_t *methods;
> +	const struct smbus_methods_t *methods;
>   };
>   
>   static const struct smbus_methods_t smbus_methods = {
> @@ -361,7 +361,6 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
>   static int acpi_smbus_cmi_add(struct acpi_device *device)
>   {
>   	struct acpi_smbus_cmi *smbus_cmi;
> -	const struct acpi_device_id *id;
>   	int ret;
>   
>   	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
> @@ -369,6 +368,7 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
>   		return -ENOMEM;
>   
>   	smbus_cmi->handle = device->handle;
> +	smbus_cmi->methods = device_get_match_data(&device->dev);
>   	strcpy(acpi_device_name(device), ACPI_SMBUS_HC_DEVICE_NAME);
>   	strcpy(acpi_device_class(device), ACPI_SMBUS_HC_CLASS);
>   	device->driver_data = smbus_cmi;
> @@ -376,11 +376,6 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
>   	smbus_cmi->cap_read = 0;
>   	smbus_cmi->cap_write = 0;
>   
> -	for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
> -		if (!strcmp(id->id, acpi_device_hid(device)))
> -			smbus_cmi->methods =
> -				(struct smbus_methods_t *) id->driver_data;
> -
>   	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
>   			    acpi_smbus_cmi_query_methods, NULL, smbus_cmi, NULL);
>   

Hi,

I tried build Linux 6.0 and run under Xen 4.14.5. I got a kernel null pointer dereferenced while booting. Reverting this commit allowed the system to boot.

Since I do not have any way of capturing the error except camera (it's a long message), here's my best try.

BUG: kernel NULL pointer dereference, address: 0000000000000000

#PF: supervisor read access in kernel mode

#PF: error_code(0x0800) - not-present page

PGD 0 P4D 0

Oops: 0000 [#1] PREEMPT SMP NOPT

CPU: 3 PID: 1879 Comm: systemd-udevd Tainted: G  W 6.0.0-1.fc32.quebs.x86_64 #1

RIP: e838:acpi_smbus_cmi_add_cap.isra.0+0x45/0x1a0 [i2c_scmi]

Code: 84 00 00 4c 8b 37 4c 89 e7 65 48 8b 04 25 28 00 00 00 48 89 44 24 10 31 c0 48 c7 04 24 ff ff ff ff 48 c7 44 24 08 00 00 00 00 <4c>

RSP: e02b:ffffc98841d1bad8 EFLAGS: 00010246

RAX: 0000000000000000 RBX: ffff88810abb6000 RCX: 0000000000000000

RDX: ffffc90041d1bb23 RSI: ffffc90041d1bb33 RDI: ffffc90041d1bb33

RBP: 0000000000000000 R08: ffff88818127169c R09: 0000000000000000

R10: ffff8881020f56f0 R11: 0000000000000000 R12: ffffc90041d1bb33

R13: 0000000000000000 r14: fffff888181271cc0 R15: 0000000000000000

FS:  00007572507ceb00(0000) GS: ffff8881408c0000(0000) knlGS: 0000000000000000

CS:   e830 DS: 0000 ES: 0000 CR0: 0000000000050033

CR2: 0000000000000000 CR3: 000000010ab1e000 CR4: 0000000000010660

Call Trace:

   <TASK>

   acp_smbus_cmi_query_methods+0x78/0x7f [i2c_scmi]

   acpi_ns_walk_namespace+0xea/0x1cb

   ? acpi_smbus_cmi_add_cap.isra.0+0x1a8/0x1a0 [i2c_scmi]

   ? acpi_smbus_cmi_add_cap.isra.0+0x1a8/0x1a0 [i2c_scmi]

   acpi_ns_walk_namespace+0x99/0xcf

   acpi_smbus_cmi_add+0xad/0x150 [i2c_scmi]

   acpi_device_probe+0xx45/0x160

   call_driver-probe+0x24/0xe0

   really_probe+0xcf/0x2d0

   ? pm_runtime_barrier+0x50/0x90

   __driver_probe_device+0x70/0x180

   driver_probe_device+0x1e/0x90

   __driver_attach+0xc4/0x1e0

   ? __device_attach_driver+0x??

   ? __device_attach_driver+??

   bus_for-each_dev

   bus_add_driver

   driver_register

   ??

   do_one_initcall

   do_init_module

   __do_sys_finit_??

   do_syscall_64

   ? do_syscall_64

   ? syscall_exit_to_??

   ? do_syscall_64

   entry_SYSCALL_64_after??

RIP:

Code:

RSP:

RAX

RZBX

?

R10

R13

  </TASK>

Modules linked in:

CR2: 0000000000000000

---[ end trace

RIP: e830: acpi_smbis_cmi_add_cap.isra

Code:..

RSP..

..

CR2: ?

Kernel panic - not syncing: Fatal exception

Kernel Offset: disabled

   

Cheers

Josef