[flashrom] [PATCH] linux_spi.c: set SPI mode, word justification and bits per word on init.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri Mar 2 22:43:44 CET 2012
Am 02.03.2012 00:43 schrieb Stefan Tauner:
> Previously we relied on a correctly set up state.
>
> ---
> untested.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> ---
> linux_spi.c | 23 +++++++++++++++++++++++
> 1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/linux_spi.c b/linux_spi.c
> index d994389..d29c59a 100644
> --- a/linux_spi.c
> +++ b/linux_spi.c
> @@ -57,6 +57,8 @@ int linux_spi_init(void)
> {
> char *p, *endp, *dev;
> uint32_t speed = 0;
> + /* FIXME: make the following configurable by CLI options. */
> + uint8_t mode = SPI_MODE_0, lsb = 0, bits = 0; /* mode 0, msb first, 8 bits */
Can you move that comment above the variable definitions?
Where should we note that SPI_MODE_0 also implies CS# active low?
>
> dev = extract_programmer_param("dev");
> if (!dev || !strlen(dev)) {
> @@ -92,6 +94,27 @@ int linux_spi_init(void)
> msg_pdbg("Using %d kHz clock\n", speed);
> }
>
> + if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) {
> + msg_perr("%s: failed to set SPI mode to %u: %s\n",
> + __func__, mode, strerror(errno));
> + close(fd);
> + return 1;
> + }
> +
> + if (ioctl(fd, SPI_IOC_WR_LSB_FIRST, &lsb) == -1) {
> + msg_perr("%s: failed to set SPI justification to %u: %s\n",
> + __func__, lsb, strerror(errno));
This message would benefit from an explanation what SPI justification
is. Suggestion:
msg_perr("%s: failed to set SPI bit order to %s first: %s\n", __func_,
lsb ? "LSB" : "MSB", strerror(errno));
> + close(fd);
> + return 1;
> + }
> +
> + if (ioctl(fd, SPI_IOC_WR_BITS_PER_WORD, &bits) == -1) {
> + msg_perr("%s: failed to set the number of bits in an SPI word to %u: %s\n",
> + __func__, bits, strerror(errno));
bits is 0. The error message would suggest that we tried to set the
number of bits to 0. Does 0 also mean 8 bits, or would we have to set 8
bits with bits=8?
> + close(fd);
> + return 1;
> + }
> +
> if (register_shutdown(linux_spi_shutdown, NULL))
> return 1;
>
As an alternative, we could avoid the whole close(fd) dance by calling
register_shutdown() first, and then letting it do the work for us
automatically after we return 1.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the flashrom
mailing list