[U-Boot] [PATCH v2 8/9] tegra: add SPI SLINK driver

[Allen Martin]

On Mon, Jan 14, 2013 at 10:49:53AM -0800, Stephen Warren wrote:
> On 01/12/2013 09:56 AM, Simon Glass wrote:
> > Hi,
> > 
> > On Sat, Jan 12, 2013 at 1:07 AM, Allen Martin <amartin at nvidia.com> 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
> 
> >> +#ifdef CONFIG_OF_CONTROL
> > 
> > You probably don't need this ifdef
> 
> If we did assume CONFIG_OF_CONTROL was on, then we could get rid of the
> previous patch; all the device addresses would come from device tree, so
> there would be no need to add them to any header file:-)

I took Simon's comment to mean it's safe to include fdtdec.h even if
the fdt code is ifdef'ed out.

> 
> >> +void spi_init(void)
> >> +{
> >> +       int node = 0, i;
> >> +       struct tegra_spi_ctrl *ctrl;
> > 
> > blank line here
> > 
> >> +       for (i = 0; i < CONFIG_TEGRA_SLINK_CTRLS; i++) {
> >> +               ctrl = &spi_ctrls[i];
> >> +#ifdef CONFIG_OF_CONTROL
> >> +               node = fdtdec_next_compatible(gd->fdt_blob, node,
> >> +                                             COMPAT_NVIDIA_TEGRA20_SLINK);
> >> +               if (!node)
> >> +                       break;
> > 
> > I think you should be using fdtdec_find_aliases_for_id() so that aliases work.
> 
> I strongly believe we shouldn't be using aliases for enumeration, which
> is what fdtdec_find_aliases_for_id() does, IIRC. Instead, we should
> enumerate all the devices in the correct fashion, and then apply aliases
> as a separate step if they exists. Hence, I believe it's not correct to
> use fdtdec_find_aliases_for_id() anywhere.

I'm a bit confused about the usage of fdt aliases in u-boot.  My
understanding of aliases is that they are intended to give drivers
hints about device naming.  Since u-boot doesn't really name devices,
what are aliases supposed to do?

-Allen
-- 
nvpublic