Re: [PATCH] extcon: Drop explicit initialization of struct i2c_device_id::driver_data to 0

[Uwe Kleine-König]

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@xxxxxxxxxxxx>
> >
> >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@xxxxxxxxxxxx.

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

Attachment: signature.asc
Description: PGP signature