From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory CLEMENT Date: Wed, 16 Dec 2015 11:56:30 +0100 Subject: [U-Boot] [PATCH] ARM: Add Support for the VInCo platform In-Reply-To: <20151216114032.08699928@free-electrons.com> (Thomas Petazzoni's message of "Wed, 16 Dec 2015 11:40:32 +0100") References: <1450262040-17734-1-git-send-email-gregory.clement@free-electrons.com> <20151216114032.08699928@free-electrons.com> Message-ID: <87k2oeu0f5.fsf@free-electrons.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Thomas, On mer., d?c. 16 2015, Thomas Petazzoni wrote: > Dear Gregory CLEMENT, > > On Wed, 16 Dec 2015 11:34:00 +0100, Gregory CLEMENT wrote: > >> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig >> index c333647..d7e36cb 100644 >> --- a/arch/arm/mach-at91/Kconfig >> +++ b/arch/arm/mach-at91/Kconfig >> @@ -114,6 +114,12 @@ config TARGET_SMARTWEB >> select CPU_ARM926EJS >> select SUPPORT_SPL >> >> +config TARGET_VINCO >> + bool "Support VINCO" >> + select CPU_V7 >> + select SUPPORT_SPL >> + >> + > > One too many empty line. > >> endchoice >> >> config SYS_SOC >> @@ -143,5 +149,6 @@ source "board/ronetix/pm9g45/Kconfig" >> source "board/siemens/corvus/Kconfig" >> source "board/siemens/taurus/Kconfig" >> source "board/siemens/smartweb/Kconfig" >> +source "board/l+g/vinco/Kconfig" > > Alphabetic ordering should be respected here. > >> diff --git a/board/l+g/vinco/Makefile b/board/l+g/vinco/Makefile >> new file mode 100644 >> index 0000000..d68b3e4 >> --- /dev/null >> +++ b/board/l+g/vinco/Makefile >> @@ -0,0 +1,2 @@ >> + > > Useless newline. > >> +obj-y += vinco.o >> diff --git a/board/l+g/vinco/vinco.c b/board/l+g/vinco/vinco.c >> new file mode 100644 >> index 0000000..d9ba987 >> --- /dev/null >> +++ b/board/l+g/vinco/vinco.c >> @@ -0,0 +1,212 @@ >> +/* >> + * Board file for the VinCo platform >> + * Based on the the SAMA5-EK board file >> + * Configuration settings for the VinCo platform. >> + * Copyright (C) 2014 Atmel >> + * Bo Shen >> + * Copyright (C) 2015 Free Electrons >> + * Gregory CLEMENT gregory.clement at free-electrons.com > > E-mail address should be enclosed in <...>. > > >> +#ifdef CONFIG_ATMEL_SPI >> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >> +{ >> + return bus == 0 && cs == 0; >> +} >> + >> +void spi_cs_activate(struct spi_slave *slave) >> +{ >> + at91_set_pio_output(AT91_PIO_PORTC, 3, 0); >> +} >> + >> +void spi_cs_deactivate(struct spi_slave *slave) >> +{ >> + at91_set_pio_output(AT91_PIO_PORTC, 3, 1); >> +} >> + >> +static void sama5d4ek_spi0_hw_init(void) > > A function named sama5d4ek in the support for a board named L+G VInCo ? > >> +{ >> + at91_set_a_periph(AT91_PIO_PORTC, 0, 0); /* SPI0_MISO */ >> + at91_set_a_periph(AT91_PIO_PORTC, 1, 0); /* SPI0_MOSI */ >> + at91_set_a_periph(AT91_PIO_PORTC, 2, 0); /* SPI0_SPCK */ >> + >> + at91_set_pio_output(AT91_PIO_PORTC, 3, 1); /* SPI0_CS0 */ >> + >> + /* Enable clock */ >> + at91_periph_clk_enable(ATMEL_ID_SPI0); >> +} >> +#endif /* CONFIG_ATMEL_SPI */ >> + >> + >> +#ifdef CONFIG_CMD_USB >> +static void sama5d4ek_usb_hw_init(void) > > Ditto. > >> +{ >> + at91_set_pio_output(AT91_PIO_PORTE, 11, 0); >> + at91_set_pio_output(AT91_PIO_PORTE, 12, 0); >> + at91_set_pio_output(AT91_PIO_PORTE, 10, 0); >> +} >> +#endif >> + >> + >> +#ifdef CONFIG_GENERIC_ATMEL_MCI >> +void sama5d4ek_mci0_hw_init(void) > > Ditto (and multiple times later). > > >> diff --git a/include/configs/vinco.h b/include/configs/vinco.h >> new file mode 100644 >> index 0000000..678b04b >> --- /dev/null >> +++ b/include/configs/vinco.h >> @@ -0,0 +1,172 @@ >> +/* >> + * Configuration settings for the VInCo platform. >> + * >> + * Based on the settings for the SAMA5-EK board >> + * Copyright (C) 2014 Atmel >> + * Bo Shen >> + * Copyright (C) 2015 Free Electrons >> + * Gregory CLEMENT gregory.clement at free-electrons.com > > <...> for e-mail address > >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef __CONFIG_H >> +#define __CONFIG_H >> + >> +/* No NOR flash, this definition should put before common header */ > > "should put" ? "should _be_ put" I guess, I dumbly copy and paste this one. > >> +#define CONFIG_SYS_NO_FLASH >> + >> +#ifdef CONFIG_SYS_TEXT_BASE >> +#undef CONFIG_SYS_TEXT_BASE >> +#endif > > Why here? Nothing has been included so far, I guess this is going to be > defined by the next line, no? > >> +#include "at91-sama5_common.h" >> + >> +/* The value in the common file is too far away for the VinCo platform */ >> +#ifdef CONFIG_SYS_TEXT_BASE >> +#undef CONFIG_SYS_TEXT_BASE >> +#endif >> +#define CONFIG_SYS_TEXT_BASE 0x20f00000 Thanks for your review, I agree with all your comment and will send a v2 soon. Gregory > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com