diff mbox series

usb: typec: ucsi: Changing the driver data type to void

Message ID 20240618120024.3384047-1-heikki.krogerus@linux.intel.com (mailing list archive)
State Accepted
Commit 3560b37525d3c9059c62da7f4d74daa2d5338626
Headers show
Series usb: typec: ucsi: Changing the driver data type to void | expand

Commit Message

Heikki Krogerus June 18, 2024, noon UTC
Let's use the same data type as struct device.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg KH June 18, 2024, 12:20 p.m. UTC | #1
On Tue, Jun 18, 2024 at 03:00:24PM +0300, Heikki Krogerus wrote:
> Let's use the same data type as struct device.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/ucsi/ucsi.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index 97eda8cd63df..1280cac314fe 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -395,7 +395,7 @@ struct ucsi_debugfs_entry {
>  struct ucsi {
>  	u16 version;
>  	struct device *dev;
> -	struct driver_data *driver_data;
> +	void *driver_data;

Nice, but why?

And how was this building?  I don't see a "struct driver_data" defined
anywhere here.

thanks,

greg k-h
Heikki Krogerus June 18, 2024, 1:14 p.m. UTC | #2
On Tue, Jun 18, 2024 at 02:20:03PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 18, 2024 at 03:00:24PM +0300, Heikki Krogerus wrote:
> > Let's use the same data type as struct device.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/usb/typec/ucsi/ucsi.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> > index 97eda8cd63df..1280cac314fe 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.h
> > +++ b/drivers/usb/typec/ucsi/ucsi.h
> > @@ -395,7 +395,7 @@ struct ucsi_debugfs_entry {
> >  struct ucsi {
> >  	u16 version;
> >  	struct device *dev;
> > -	struct driver_data *driver_data;
> > +	void *driver_data;
> 
> Nice, but why?
> 
> And how was this building?  I don't see a "struct driver_data" defined
> anywhere here.

I had not even thought about that, but that's weird.

I can write anything after "struct" there, and it will just build
without any warnings or errors. The complier does not seem to even
check the type in this case, since the member is always assigned a
void pointer.

So this is actually a bug, no?

thanks,
Greg KH June 18, 2024, 1:58 p.m. UTC | #3
On Tue, Jun 18, 2024 at 04:14:57PM +0300, Heikki Krogerus wrote:
> On Tue, Jun 18, 2024 at 02:20:03PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 18, 2024 at 03:00:24PM +0300, Heikki Krogerus wrote:
> > > Let's use the same data type as struct device.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > >  drivers/usb/typec/ucsi/ucsi.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> > > index 97eda8cd63df..1280cac314fe 100644
> > > --- a/drivers/usb/typec/ucsi/ucsi.h
> > > +++ b/drivers/usb/typec/ucsi/ucsi.h
> > > @@ -395,7 +395,7 @@ struct ucsi_debugfs_entry {
> > >  struct ucsi {
> > >  	u16 version;
> > >  	struct device *dev;
> > > -	struct driver_data *driver_data;
> > > +	void *driver_data;
> > 
> > Nice, but why?
> > 
> > And how was this building?  I don't see a "struct driver_data" defined
> > anywhere here.
> 
> I had not even thought about that, but that's weird.
> 
> I can write anything after "struct" there, and it will just build
> without any warnings or errors. The complier does not seem to even
> check the type in this case, since the member is always assigned a
> void pointer.

Ah, I guess the compiler never actually has to look up what 'struct
driver_data' is if it is never referenced (i.e. you are only casting
void * to it.)

> So this is actually a bug, no?

Kind of?  But it works, so let's just leave it alone :)

I'll take this patch as it is "obviously correct" now, but sure looked
odd to start with.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 97eda8cd63df..1280cac314fe 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -395,7 +395,7 @@  struct ucsi_debugfs_entry {
 struct ucsi {
 	u16 version;
 	struct device *dev;
-	struct driver_data *driver_data;
+	void *driver_data;
 
 	const struct ucsi_operations *ops;