On Wed, 13 Jun 2012, Greg KH wrote:
> On Wed, Jun 13, 2012 at 11:07:17AM +0800, Lan Tianyu wrote:
> > Add struct usb_hub_port pointer port_data in the struct usb_hub and allocate
> > struct usb_hub_port perspectively for each ports to store private data.
> >
> > Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> > ---
> > drivers/usb/core/hub.c | 29 ++++++++++++++++-------------
> > 1 files changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 04fb834..512f0d5 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -37,6 +37,10 @@
> > #endif
> > #endif
> >
> > +struct usb_hub_port {
> > + void *port_owner;
>
> What? Why a void pointer? You should almost _never_ use a void pointer
> in kernel code, it's bad form, and it means you are trying to hide
> something here, when it shouldn't be hidden, as we should know the real
> type, right?
That's my fault; Lan is simply moving code that I wrote.
The void * values are simply cookies as far as the hub driver is
concerned -- the meaning is _intended_ to be hidden and the pointers
are never dereferenced. All that matters about them is whether two
values are the same or different. The actual pointers stored in
port_owner are created by usbfs; they are pointers to the internal data
structures defined in devio.c.
If you really want, we could change void * to struct dev_state *. I'm
not sure this would make things any better or clearer.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
