linux-kernel - Re: [PATCH] extcon: Drop explicit initialization of struct i2c_device_id::driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3dgr7brrcsux4bhywmdu7v4ibmieb3wotb7t5qlent64su7z4x@qapri6zyjfbe>
Date: Thu, 5 Dec 2024 08:50:55 +0100
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: MyungJoo Ham <myungjoo.ham@...sung.com>
Cc: Chanwoo Choi <cw00.choi@...sung.com>, 
	Krzysztof Kozlowski <krzk@...nel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] extcon: Drop explicit initialization of struct
 i2c_device_id::driver_data to 0

Hello MyungJoo,

On Thu, Dec 05, 2024 at 12:58:14PM +0900, MyungJoo Ham wrote:
> >On Wed, Sep 18, 2024 at 02:31:48PM +0200, Uwe Kleine-König wrote:
> >> These drivers don't use the driver_data member of struct i2c_device_id,
> >> so don't explicitly initialize this member.
> >> 
> >> This prepares putting driver_data in an anonymous union which requires
> >> either no initialization or named designators. But it's also a nice
> >> cleanup on its own.
> >> 
> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...libre.com>
> >
> >That patch got some positive feedback by Krzysztof but wasn't applied.
> >Is this still on someone's radar for application?
> 
> Can you provide a link to a commit that requires this change?
> 
> For now, I don't see any benefit from this change.

So the explicit initialisation of .driver_data that then isn't used
doesn't look strange enough to you to drop it?

Would you prefer 

--- a/drivers/extcon/extcon-fsa9480.c
+++ b/drivers/extcon/extcon-fsa9480.c
@@ -350,7 +350,7 @@ static const struct dev_pm_ops fsa9480_pm_ops = {
 };

 static const struct i2c_device_id fsa9480_id[] = {
-	{ "fsa9480", 0 },
+	{ .name = "fsa9480", .driver_data = 0, },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, fsa9480_id);

then?

Anyhow: The most recent presentation of the quest is at
https://lore.kernel.org/linux-iio/20241204150036.1695824-2-u.kleine-koenig@baylibre.com.

For drivers/extcon/extcon-sm5502.c this would allow the following
cleanup:

diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index 3f8cca528284..3eb8fe4f4af5 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -827,9 +827,9 @@ static SIMPLE_DEV_PM_OPS(sm5502_muic_pm_ops,
 			 sm5502_muic_suspend, sm5502_muic_resume);
 
 static const struct i2c_device_id sm5502_i2c_id[] = {
-	{ .name = "sm5502", .driver_data = (kernel_ulong_t)&sm5502_data, },
-	{ .name = "sm5504", .driver_data = (kernel_ulong_t)&sm5504_data, },
-	{ .name = "sm5703-muic", .driver_data = (kernel_ulong_t)&sm5502_data, },
+	{ .name = "sm5502", .driver_data_ptr = &sm5502_data, },
+	{ .name = "sm5504", .driver_data_ptr = &sm5504_data, },
+	{ .name = "sm5703-muic", .driver_data_ptr = &sm5502_data, },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, sm5502_i2c_id);

It doesn't benefit from further cleanups because the ugly cast it relies
on is done in i2c_get_match_data(), which can then be modified to:

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 7c810893bfa3..a050ead0b7d2 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -129,7 +129,7 @@ const void *i2c_get_match_data(const struct i2c_client *client)
 		if (!match)
 			return NULL;
 
-		data = (const void *)match->driver_data;
+		data = match->driver_data_ptr;
 	}
 
 	return data;

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)