[PATCH v2] usb: ehci-orion: add more constants for register values
Alan Stern
stern at rowland.harvard.edu
Thu Mar 19 07:27:18 PDT 2015
On Thu, 19 Mar 2015, Thomas Petazzoni wrote:
> This commit adds new register values for the USB_CMD and USB_MODE
> registers, which allows to avoid the usage of a number of magic values
> in orion_usb_phy_v1_setup().
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
The new symbol values are fine. But (side issue) ...
> @@ -69,8 +75,8 @@ static void orion_usb_phy_v1_setup(struct usb_hcd *hcd)
> /*
> * Reset controller
> */
> - wrl(USB_CMD, rdl(USB_CMD) | 0x2);
> - while (rdl(USB_CMD) & 0x2);
> + wrl(USB_CMD, rdl(USB_CMD) | USB_CMD_RESET);
> + while (rdl(USB_CMD) & USB_CMD_RESET);
For one thing, this use of whitespace does not make the syntax clear.
At the very least, it should be written as:
while (rdl(USB_CMD) & USB_CMD_RESET)
;
so that the reader can see this is a loop with an empty body. Right
now it looks like an ordinary, non-looping statement.
For another, have you considered what will happen if the hardware is
defective and never turns off the USB_CMD_RESET bit? This sort of
thing should always be implemented with some sort of timeout.
Alan Stern
More information about the linux-arm-kernel
mailing list