[U-Boot] [PATCH v3 3/3] usb: tegra: combine header file

[Stephen Warren <swarren@wwwdotorg.org>]

On 02/16/2014 12:50 PM, Stefan Agner wrote:
> Combine the Tegra USB header file into one header file for all SoCs.
> Use ifdef to account for the difference, especially Tegra20 is quite
> different from newer SoCs. This avoids duplication especially
> between Tegra30 and newer devices.

Aside from a few minor comments below,
Reviewed-by: Stephen Warren <swarren@nvidia.com>

I think this should be extended to Tegra124 too.

> diff --git a/arch/arm/include/asm/arch-tegra/usb.h b/arch/arm/include/asm/arch-tegra/usb.h

>  #ifndef _TEGRA_USB_H_
>  #define _TEGRA_USB_H_
>  
> +
> +/* USB Controller (USBx_CONTROLLER_) regs */

Why two blank lines there?

> +struct usb_ctlr {
...
> +	/* 0x200 */
> +	uint reserved11[0x80];
> +
> +#else
> +
> +	/* 0x130 */

I don't think there should be blank lines before/after the #else; there
aren't after the #if or before the #endif.

...
> +};
> +
> +
>  /* USB1_LEGACY_CTRL */

Why two blank lines there?

> +#if defined(CONFIG_TEGRA20) || defined(CONFIG_TEGRA30)
> +/* USB2_IF_ULPI_TIMING_CTRL_0 */
> +#define ULPI_OUTPUT_PINMUX_BYP			(1 << 10)
> +#define ULPI_CLKOUT_PINMUX_BYP			(1 << 11)
> +
> +/* USB2_IF_ULPI_TIMING_CTRL_1 */
> +#define ULPI_DATA_TRIMMER_LOAD			(1 << 0)
> +#define ULPI_DATA_TRIMMER_SEL(x)		(((x) & 0x7) << 1)
> +#define ULPI_STPDIRNXT_TRIMMER_LOAD		(1 << 16)
> +#define ULPI_STPDIRNXT_TRIMMER_SEL(x)	(((x) & 0x7) << 17)
> +#define ULPI_DIR_TRIMMER_LOAD			(1 << 24)
> +#define ULPI_DIR_TRIMMER_SEL(x)			(((x) & 0x7) << 25)
> +#endif

Are those defines really Tegra20/30-only, or were they simply omitted
from the Tegra114 header because they aren't used by the driver?

Actually, if these aren't used, is it even worth including this text in
the header? Perhaps delete unused text in a separate patch before this
one, so that this patch simply moves text between files?