Dear Greg Kroah-Hartman,
> On Tue, May 15, 2012 at 06:10:19AM +0200, Marek Vasut wrote:
> > Add this function so for example USB Host driver can notify
> > the PHY about some event. This is useful for me on i.MX28, where
> > I need to tell the PHY, from the EHCI interrupt handler, to disable
> > disconnection detector (or reenable it).
> >
> > Signed-off-by: Marek Vasut <marex@xxxxxxx>
> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> > Cc: Felipe Balbi <balbi@xxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Linux USB <linux-usb@xxxxxxxxxxxxxxx>
> > ---
> >
> > include/linux/usb/otg.h | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> > index 38ab3f4..4a74618 100644
> > --- a/include/linux/usb/otg.h
> > +++ b/include/linux/usb/otg.h
> > @@ -117,6 +117,9 @@ struct usb_phy {
> >
> > int (*set_suspend)(struct usb_phy *x,
> >
> > int suspend);
> >
> > + /* for host driver to pass data to the PHY */
> > + int (*notify)(struct usb_phy *x,
> > + void *data);
>
> Wow that is vague.
Like the rest of the comments in that structure ... I agree it might use some
further explanation.
> Notify what? Of what? With what? About what?
I tried to make it as generic as possible, so the user can pass whatever kind of
message he needs. I went on explaining this stuff starting here onwards:
http://www.spinics.net/lists/linux-usb/msg63791.html
>
> Ick ick ick.
>
> Please make this specific as to exactly what you are wanting to do, this
> patch is not specific at all.
All right, can you check the discussion above for the details please? I'll
eventually rework this patch once we come to some conclusion.
> Hint, if you are ever passing in a void *,
> you are probably doing something wrong...
That's true, but the intention was to allow passing any kind of data.
>
> > +static inline int
> > +usb_phy_notify(struct usb_phy *x, void *data)
> > +{
> > + if (x->notify)
> > + return x->notify(x, data);
> > +
> > + return 0;
> > +}
> > +
>
> No one is calling this function, so why add it? You just created a
> complier warning here, right?
It's then used in here:
http://www.spinics.net/lists/arm-kernel/msg175152.html
>
> greg k-h
Thanks for the review, Greg.
Best regards,
Marek Vasut
--
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
