From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Frysinger Date: Sat, 5 Nov 2011 13:13:46 -0400 Subject: [U-Boot] [PATCH v2 3/8] tegra2: spi: Add SPI driver for Tegra2 SOC In-Reply-To: References: <1320360099-28227-1-git-send-email-sjg@chromium.org> <201111032136.42310.vapier@gentoo.org> Message-ID: <201111051313.47495.vapier@gentoo.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 Saturday 05 November 2011 10:36:30 Simon Glass wrote: > On Thu, Nov 3, 2011 at 6:36 PM, Mike Frysinger wrote: > > On Thursday 03 November 2011 18:41:34 Simon Glass wrote: > >> --- /dev/null > >> +++ b/drivers/spi/tegra2_spi.c > >> > >> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) > >> +{ > >> + /* Tegra2 SPI-Flash - only 1 device ('bus/cs') */ > >> + if (bus > 0 && cs != 0) > >> + return 0; > >> + else > >> + return 1; > >> +} > > > > shouldn't that be "||" and not "&&" ? > > This function should be removed as it doesn't print enough errors. this func is part of the SPI API. you can't remove it ;). > >> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > >> + unsigned int max_hz, unsigned int mode) > >> +{ > >> + struct tegra_spi_slave *spi; > >> + > >> + if (!spi_cs_is_valid(bus, cs)) > >> + return NULL; > >> + > >> + if (bus != 0) { > >> + printf("SPI error: unsupported bus %d\n", bus); > >> + return NULL; > >> + } > >> + if (cs != 0) { > >> + printf("SPI error: unsupported chip select %d on bus > >> %d\n", + cs, bus); > >> + return NULL; > >> + } > > > > doesn't spi_cs_is_valid() make these two later checks redundant ? > > Yes - have removed the function. i think this is the wrong direction ... other SPI buses are not warning about invalid bus/cs combos and that seems to be fine. i don't think the tegra spi bus needs to be uniquely verbose. > >> + reg = readl(®s->status); > >> + writel(reg, ®s->status); /* Clear all SPI events via R/W */ > > > > are these R1C or W1C bits ? if the latter, you could just write -1 and > > avoid the read altogether ... > > The next line is: > > debug("spi_xfer entry: STATUS = %08x\n", reg); > > and I didn't want to remove that, so I need to keep the read > unfortunately. It could perhaps be this if you are keen: > > writel(-1, ®s->status); /* Clear all SPI events via R/W */ > debug("spi_xfer entry: STATUS = %08x\n", readl(®->status)); what you have now is fine if you prefer it that way -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111105/45020a50/attachment.pgp