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

["Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>]

Dear Marek Vasut,

> > > > @@ -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?

Yes, I hesitated with this solution when writing this code.

Where the offset is needed, I can instead write additional extraction macros.

> > > > @@ -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 :)

OK.

Best regards,
Beno?t