On Fri, Jan 11, 2013 at 04:22:46PM -0800, Stephen Warren wrote: > On 01/11/2013 11:44 AM, Allen Martin wrote: > > Add driver for tegra SPI "SLINK" style driver. This controller is > > similar to the tegra20 SPI "SFLASH" controller. The difference is > > that the SLINK controller is a genernal purpose SPI controller and the > > SFLASH controller is special purpose and can only talk to FLASH > > devices. In addition there are potentially many instances of an SLINK > > controller on tegra and only a single instance of SFLASH. Tegra20 is > > currently ths only version of tegra that instantiates an SFLASH > > controller. > > > > This driver supports basic PIO mode of operation and is configurable > > (CONFIG_OF_CONTROL) to be driven off devicetree bindings. Up to 4 > > devices per controller may be attached, although typically only a > > single chip select line is exposed from tegra per controller so in > > reality this is usually limited to 1. > > > > To enable this driver, use CONFIG_TEGRA_SLINK > > > diff --git a/drivers/spi/tegra_slink.c b/drivers/spi/tegra_slink.c > > > +void spi_init(void) > > +{ > > + int node = 0, i; > > + struct tegra_spi_ctrl *ctrl; > > + for (i = 0; i < CONFIG_TEGRA_SLINK_CTRLS; i++) { > > + ctrl = &spi_ctrls[i]; > > +#ifdef CONFIG_OF_CONTROL > > Is this ever false for Tegra upstream? I don't think so.
No, but I think it's worthwhile to keep it under #ifdef so this driver could be used from the SPL if the need ever comes up. I tested by forcing it off for this driver and it works. > > > + node = fdtdec_next_compatible(gd->fdt_blob, node, > > + COMPAT_NVIDIA_TEGRA20_SLINK); > > + if (!node) > > + break; > > + ctrl->regs = (struct slink_tegra *)fdtdec_get_addr(gd->fdt_blob, > > + node, "reg"); > > + if ((fdt_addr_t)ctrl->regs == FDT_ADDR_T_NONE) { > > + debug("%s: no slink register found\n", __func__); > > + break; > > Shouldn't most of the "break"s in this loop be "continue"s, so that any > other nodes can be parsed? After all, this loop is trying to initialize > all SLINK controllers in the DT right? True, and looking at this code I think the driver is going to do something bad if any of the controllers are not fully specified and you go to use it later. I think the controller structure needs a "valid" flag. -Allen -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot