On Thu, Jul 12, 2018 at 09:03:30AM +0000, Pawel Laszczak wrote:
> > > +/* USB 2.0 hardware LMP capability*/
> > > +#define USBSSP_HLC (1 << 19)
> > > +#define USBSSP_BLC (1 << 20)
> >
> > Again, BIT() please.
> >
> > > +int usbssp_handshake(void __iomem *ptr, u32 mask, u32 done, int usec)
> > > +{
> > > + u32 result;
> >
> > Some places you use tabs for the variable declarations, and some you do
> > not. Pick a single style and stick to it please.
> >
> > > +
> > > + do {
> > > + result = readl(ptr);
> > > + if (result == ~(u32)0) /* card removed */
> > > + return -ENODEV;
> > > + result &= mask;
> > > + if (result == done)
> > > + return 0;
> > > + udelay(1);
> > > + usec--;
> > > + } while (usec > 0);
> > > + return -ETIMEDOUT;
> >
> > We don't have a built-in kernel function to do this type of thing already?
> > That's sad. Oh well...
> >
> > > +int usbssp_init(struct usbssp_udc *usbssp_data) {
> > > + int retval = 0;
> > > +
> > > + usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
> > "usbssp_init");
> > > +
> > > + spin_lock_init(&usbssp_data->lock);
> > > + spin_lock_init(&usbssp_data->irq_thread_lock);
> > > +
> > > + //TODO: memory initialization
> > > + //retval = usbssp_mem_init(usbssp_data, GFP_KERNEL);
> > > +
> > > + usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
> > > + "Finished usbssp_init");
> >
> > When your trace functions do nothing but say "entered a function", and
> > "exited a function", why even have them? ftrace can provide that for you
> > already, no need to overload that on the tracing framework, right?
>
> Do you suggest to use only:
> trace_usbssp_dbg_init("Finished usbssp_init");
> instead:
> usbssp_dbg(usbssp_data, "%pV\n", "Finished usbssp_init");
> trace_usbssp_dbg_init("Finished usbssp_init");
> ?
>
> I'm simple re-used the code from XHCI driver. It's really redundant,
> but I don't know the intention of author 😊.
Why are any of those lines needed? Doesn't ftrace work properly for
you?
And yeah, if xhci has this it should be removed from there as well.
thanks,
greg k-h
--
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
