Re: [tegrarcm PATCH v1 1/8] tegrarcm: Add Tegra132 support

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Tue, Mar 17, 2015 at 05:38:53PM -0700, Jimmy Zhang wrote:
> From: Allen Martin <amartin-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Add Tegra132 USB device id, miniloader, and chip SKU information.
> 
> Signed-off-by: Allen Martin <amartin-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  src/Makefile.am                      |    1 +
>  src/main.c                           |   15 +-
>  src/miniloader/tegra132-miniloader.h | 5259 ++++++++++++++++++++++++++++++++++
>  src/nv3p.h                           |    3 +
>  src/tegrarcm.1.in                    |    4 +-
>  src/usb.c                            |    3 +-
>  src/usb.h                            |    1 +
>  7 files changed, 5283 insertions(+), 3 deletions(-)
>  create mode 100644 src/miniloader/tegra132-miniloader.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4b548859e075..d0d45cad4fee 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -18,6 +18,7 @@ tegrarcm_SOURCES = \
>  	miniloader/tegra30-miniloader.h \
>  	miniloader/tegra114-miniloader.h \
>  	miniloader/tegra124-miniloader.h \
> +	miniloader/tegra132-miniloader.h \
>  	usb.h
>  
>  man_MANS = tegrarcm.1
> diff --git a/src/main.c b/src/main.c
> index 50adc14f0b06..24d3bf81191f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -60,6 +60,9 @@
>  // tegra124 miniloader
>  #include "miniloader/tegra124-miniloader.h"
>  
> +// tegra132 miniloader
> +#include "miniloader/tegra132-miniloader.h"

I'd prefer if we stopped including miniloader (and other) blobs in the
tegrarcm binary. We already provide the MTS in a separate repository and
can reference it using command-line options. I think we should do the
same for the miniloader.

The ordering of patches here is somewhat confusing, too. This patch in
fact doesn't really add support for Tegra132 because we need MTS for
that to actually work. A more natural ordering would be to have the MTS
support go in first, followed by this patch to enable Tegra132 support.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]