[U-Boot] [PATCH 12/23] ARM: tegra: Implement XUSB pad controller

[Stephen Warren <swarren@wwwdotorg.org>]

On 08/18/2014 01:16 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> This controller was introduced on Tegra114 to handle XUSB pads. On
> Tegra124 it is also used for PCIe and SATA pin muxing and PHY control.
> Only the Tegra124 PCIe and SATA functionality is currently implemented,
> with weak symbols on Tegra114.
>
> Tegra20 and Tegra30 also provide weak symbols for these functions so
> that drivers can use the same API irrespective of which SoC they're
> being built for.

>   arch/arm/include/asm/arch-tegra/xusb-padctl.h    |  14 +
>   arch/arm/include/asm/arch-tegra114/xusb-padctl.h |   6 +
>   arch/arm/include/asm/arch-tegra124/xusb-padctl.h |   6 +
>   arch/arm/include/asm/arch-tegra20/xusb-padctl.h  |   6 +
>   arch/arm/include/asm/arch-tegra30/xusb-padctl.h  |   6 +

These per-SoC headers do nothing but include the common header. Why not 
just have the code include the common header directly? IIRC, that's what 
we do now for some other modules that are identical. We only need 
SoC-specific headers if we have SoC-specific information to represent.

> diff --git a/arch/arm/cpu/tegra124-common/Makefile b/arch/arm/cpu/tegra124-common/Makefile

>   obj-y	+= clock.o
>   obj-y	+= funcmux.o
>   obj-y	+= pinmux.o
> +obj-y	+= xusb-padctl.o

It looks like there's indentation inconsistency there?

> diff --git a/arch/arm/cpu/tegra124-common/xusb-padctl.c b/arch/arm/cpu/tegra124-common/xusb-padctl.c

> +int fdtdec_count_strings(const void *fdt, int node, const char *prop_name)

> +int fdtdec_get_string_index(const void *fdt, int node, const char *prop_name,
> +			    int index, const char **output)

> +int fdtdec_get_string(const void *fdt, int node, const char *prop_name,

Shouldn't those be in libfdt or similar? I thought I saw a patch to do that.

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

> +struct tegra_xusb_phy *tegra_xusb_phy_get(unsigned int type);

What is "type"? A comment would be useful, or using an enum type instead.

With those fixed, I don't see anything obviously wrong, so,
Acked-by: Stephen Warren <swarren@nvidia.com>