From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Fri, 10 Aug 2012 03:12:12 +0200
Subject: [U-Boot] [PATCH v3 1/8] ehci: cosmetic: Define used constants
In-Reply-To: <1623098771.2239033.1344560024838.JavaMail.root@advansee.com>
References: <1623098771.2239033.1344560024838.JavaMail.root@advansee.com>
Message-ID: <201208100312.13124.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Beno?t Th?baudeau,

> 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.
> > 
> > Oh, there're places where the offset is needed?
> 
> Yes, I think.
> 
> > What about adding
> > both BIT(x)
> > and BIT_OFFSET in such cases ? And BIT(x) (x << BIT_OFFSET) in that
> > case.
> 
> I would prefer:
> #define FIELD(fldval)     (((fldval) & MASK) << OFFSET)
> #define GET_FIELD(regval) (((regval) >> OFFSET) & MASK)

Yes of course.

> Are you OK with that?
> 
> Best regards,
> Beno?t

Best regards,
Marek Vasut