On Monday, May 02, 2011 11:33:24 Tom Warren wrote: > On Fri, Apr 29, 2011 at 4:21 PM, Mike Frysinger wrote: > > On Thursday, April 28, 2011 10:55:13 Tom Warren wrote: > >> This patch adds support for the SPIFLASH peripheral on Tegra2 (SPI 1). > >> Probe, erase, read and write are all supported, as well as low-level > >> SPI commands via 'sspi'. > >> > >> Note that, at this time, only Seaboard has a SPI flash part (Winbond). > >> With this code, I've written U-Boot to SPI from within U-Boot, and > >> booted the system from that SPI image (boot sel jumpers must be set > >> to SPI [1000], and the UART ENABLE jumper must be set to GPIO CTRL). > > > > so this is a driver for a SPI bus, and the board you're working on just > > happens to have some SPI flashes connected to it ? then the "SPIFLASH" > > part is irrelevant, as is the board in question. please remove > > references to both and just refer to this as "a SPI driver for Tegra2 > > processors". > > No, there are 4 general purpose SPI interfaces in Tegra2 (SLINK), and > 1 dedicated SPI FLASH controller. From the Tegra2 TRM: > > "The Serial Flash interface (SFLASH) Controller interfaces Tegra 2 to > SPI-capable devices such as Flash > memories and ADC/DAC devices. Tegra 2 supports only master mode of SPI > operation on this interface. > This interface is specifically intended for serial flash and similar > devices. For a general purpose SPI > interface, use the SLINK serial interface." > > The register sets are similar, but not identical.
do you plan on writing a driver for the SLINK logic ? would it be a new file, or would you keep the two integrated ? if diff files, i'd name this one "tegra2_sflash_spi.c". if you keep them merged, then "tegra2_spi.c" is fine. > As to the board reference, it's in my comments so people wishing to > use this will know that only Seaboard is available w/a SPI chip - > there's no provision for that on the Harmony board. generally, processor drivers are not concerned with board issues. if you want to document board-specific issues somewhere, add a doc/README.<boardname>. > >> --- a/arch/arm/lib/board.c > >> +++ b/arch/arm/lib/board.c > >> +#if defined(CONFIG_CMD_SPI) > > I'm going to remove these changes from arm/lib/board.c and move > spi_init() to our common board file (nvidia/common/board.c. i dont think you want to do this. other than the style changes, and needing a sep changeset, having the arm board.c file call spi_init() is desirable. > >> + * On Seaboard, MOSI/MISO are shared w/UART. > >> + * Use GPIO I3 (UART_DISABLE) to tristate UART during SPI > >> activity. > >> + * Enable UART later (cs_deactivate) so we can use it > >> for U-Boot comms. > >> + gpio_direction_output(UART_DISABLE_GPIO, 1); > > > > board specific issues shouldnt be in a processor driver. > > Please explain what you mean by a processor driver. you're writing a driver for the Tegra2 SoC. that is the only thing this driver should be doing. that means no board-specific logic is allowed. > This does need an > ifdef, so any future board that doesn't have Seaboard's defiency of a > muxed SPIFLASH/UART won't have to mess with a GPIO, but for Seaboard, > we have to dynamically change GPIO_PI3 every SPI transaction to ensure > UART spew can continue (in DEBUG builds, etc.). right, this is a board-specific issue. thus it doesnt belong in a driver that is only concerned with the Tegra2 SoC. what you could do is: void spi_init_common(void) { ... only tegra2 code ... } void spi_init(void) __attribute__((weak, alias("spi_init_common"))); and then in the seaboard directory, add: void spi_init(void) { ... do random gpio/portmux stuff ... spi_init_common(); } or another style would be to: extern void spi_board_init(void) __weak; void spi_init(void) { if (spi_board_init()) spi_board_init(); ... only tegra2 code ... } and then in the seaboard directory, add: void spi_board_init(void) { ... do random gpio/portmux stuff ... } -mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot