From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 14 Jan 2013 11:49:53 -0700 Subject: [U-Boot] [PATCH v2 8/9] tegra: add SPI SLINK driver In-Reply-To: References: <1357981631-21245-1-git-send-email-amartin@nvidia.com> <1357981631-21245-9-git-send-email-amartin@nvidia.com> Message-ID: <50F45351.7030802@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/12/2013 09:56 AM, Simon Glass wrote: > Hi, > > On Sat, Jan 12, 2013 at 1:07 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 >> +#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:-) >> +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.