[U-Boot] [PATCH 6/7] tegra: add SPI SLINK driver

[Allen Martin <amartin@nvidia.com>]

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