[U-Boot] [PATCH v3 1/8] ehci: cosmetic: Define used constants

Marek Vasut
2012-08-09T23:14:35Z

Dear Beno?t Th?baudeau,

[...]

> > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev,
> > > unsigned
> > > long pipe, void *buffer, */
> > >
> > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
> > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
> > >
> > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0;
> > > - endpt = (8 << 28) |
> > > - (c << 27) |
> > > - (usb_maxpacket(dev, pipe) << 16) |
> > > - (0 << 15) |
> > > - (1 << 14) |
> > > - (usb_pipespeed(pipe) << 12) |
> > > - (usb_pipeendpoint(pipe) << 8) |
> > > - (0 << 7) | (usb_pipedevice(pipe) << 0);
> > > + usb_pipeendpoint(pipe) == 0);
> > > + endpt = (8 << QH_ENDPT1_RL) |
> > > + (c << QH_ENDPT1_C) |
> >
> > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ?
> > [...]
>
> For all of these?

Yes, do you not think it's better than define the offsets only?

> > > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev,
> > > unsigned
> > > long pipe, void *buffer, ALIGN((uint32_t)buffer + length,
> > > ARCH_DMA_MINALIGN));
> > >
> > > /* Check that the TD processing happened */
> > >
> > > - if (token & 0x80) {
> > > + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS))
> > >
> > > printf("EHCI timed out on TD - token=%#x\n", token);
> > >
> > > - }
> > >
> > > /* Disable async schedule. */
> > > cmd = ehci_readl(&hcor->or_usbcmd);
> > > cmd &= ~CMD_ASE;
> > > ehci_writel(&hcor->or_usbcmd, cmd);
> > >
> > > - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0,
> > > + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0,
> >
> > Ooooh, nice catch :)
> >
> > [...]
> > The rest is cool.
> >
> > btw when (I hope you will) resubmitting next time, just submit the
> > whole series
> > under 0/8 patch (or 1/8) of the old one to make it a nice thread.
>
> OK, so with 2/8 removed since you have applied it.

Yes

> What is the rule here? I
> thought that a new version of a patch should be posted as a reply to the
> previous version so that patchwork could mark the old version as superseded

Yes, unless the series changed so much (aka. too much reordering etc), that it's
easier to repost it in reply to the first patch in the series.

> (even if this feature is not yet available for U-Boot). Does it apply only
> to single patches while series should be replied to the previous 0/n?

I didn't figure out these gems in patchwork myself. I'm not a big fan of PW
either.

> And should 3/n be a reply to 2/n, to 0/n (or to 1/n if no 0/n), or to
> nothing?

Just use the standard threading git send-email does.

> I will wait for your review of 8/8.

I think it's fine to just resend it so I'll either pick first 7 or all 8. First
7 are certain once we agree on the above stuff :)

> Best regards,
> Beno?t

Best regards,
Marek Vasut