Re: [PATCH] xhci: Merge and Update debugging for patches from 3.6 kernel tree

[Mateusz Guzik <mguzik@redhat.com>]

On Tue, Aug 05, 2014 at 01:55:34AM -0400, Nick Krause wrote:
> On Tue, Aug 5, 2014 at 1:45 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Aug 05, 2014 at 12:56:57AM -0400, Nicholas Krause wrote:
> >> I am adding the fixes to the tree send for adding debugging to the kernel
> >> tree from a patch sent in 2013 on the the 3.6 release. The patch adds
> >> debugging over xhci capable debugging usb ports and needed to be updated
> >> into the latest rc tree. The patch was first sent in this thread,
> >> http://marc.info/?l=linux-usb&m=135948845511047.
> >>
> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> >
> > If you send one more patch, I am going to have to ask vger.kernel.org to
> > ban you from their servers.  You are actively bothering people and
> > causing problems and wasting time.
> >
> > You have been told _numerous_ times to stop, yet you refuse to listen.
> > You hold the record for getting kicked out of the Eudyptula challenge in
> > a matter of hours, something no one else has ever had happen.  You
> > ignore lots of very valid comments and suggestions, for no good reason.
> > You flail about, making mistakes that are now starting to bother users,
> > which is not acceptable at all.
> >
> > I will not respond to any more of your emails, and ask everyone else to
> > now stop as well.
> >
> > good bye.
> >
> > *plonk*
> >
> > greg k-h
> 
> Greg,
> You haven't even checked my patch, our you just going to assume it's wrong?


I don't know this code and I'm not interested in changing that, so just I'm
just pointing out obvious problems.

1. You somehow managed to have changes like his all over the patch:
-#define TT_HS_OVERHEAD (31 + 94)
-#define TT_DMI_OVERHEAD (25 + 12)
+#define TT_HS_OVERHEAD (31  94)
+#define TT_DMI_OVERHEAD (25  12)

Clearly, this will cause compilation problems. But maybe this weird
corruption happened after you tested your patch?

2.  In the original patch there were some additions to struct xhci_hcd,
e.g. dbg_cap_reg.

Let's look at a random user:
+void xhci_teardown_dbg_cap(struct xhci_hcd *xhci, struct device *dev)
+{
+       u32 val;
+
+       xhci_dbg(xhci, "xhci_teardown_dbg_cap()\n");
+       if (!xhci->dbg_cap_regs)
+               return;

xhci is a pointer to struct xhci_hcd.

Except in your patch all these additions landed in struct xhci_dbg_cap_ctx,
thus this could not possibly compile.

But what's the most important thing here is this: writing a compilable
patch is not a problem, writing a working patch is.

Forward porting, like this one, demands that you:
- understand the patch as applied to the original tree
- understand current state of the tree and how it influences the patch

As such, foward porting is typically not a task for beginners.

I can only recommend one last time you leave the kernel alone for the
time being and focus on userspace.

There. This is my last reply, have fun.

-- 
Mateusz Guzik