From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= Date: Sat, 20 May 2017 10:06:06 +0200 Subject: [U-Boot] [PATCH 02/10] drivers: spi: add config to consider command bytes when writting to flash In-Reply-To: References: <1495135788-9152-1-git-send-email-noltari@gmail.com> <1495135788-9152-3-git-send-email-noltari@gmail.com> Message-ID: <2e4e1f83-e0a9-e617-03a5-e469862d9f0a@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Simon, El 20/05/2017 a las 4:29, Simon Glass escribió: > Hi Alvaro, > > On 18 May 2017 at 13:29, Álvaro Fernández Rojas wrote: >> Command bytes are part of the written bytes and they should be taken into >> account when sending a spi transfer. >> >> Signed-off-by: Álvaro Fernández Rojas >> --- >> drivers/mtd/spi/spi_flash.c | 2 +- >> drivers/spi/Kconfig | 3 +++ >> include/spi.h | 8 +++++++- >> 3 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >> index e44c10f..748cc32 100644 >> --- a/drivers/mtd/spi/spi_flash.c >> +++ b/drivers/mtd/spi/spi_flash.c >> @@ -380,7 +380,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset, >> >> if (spi->max_write_size) >> chunk_len = min(chunk_len, >> - (size_t)spi->max_write_size); >> + spi_max_write(spi, sizeof(cmd))); >> >> spi_flash_addr(write_addr, cmd); >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> index f3f7dbe..a00d401 100644 >> --- a/drivers/spi/Kconfig >> +++ b/drivers/spi/Kconfig >> @@ -13,6 +13,9 @@ config DM_SPI >> typically use driver-private data instead of extending the >> spi_slave structure. >> >> +config SPI_MAX_WRITE_CMD_BYTES >> + bool "Include command bytes when determining max write size" > > Do you really need this, or can you just always do this? If you need > it, please add detailed help. Actually we don't need this at all and we could do it always from a BCM63xx point of view, but there are drivers that are hacking this at a lower level, like the ich one: https://github.com/Noltari/u-boot/blob/master/drivers/spi/ich.c#L355 In my opinion this is wrong, but I added that new kconfig because I didn't want to break other SPI drivers... Maybe I should reverse it and make this the default, adding a kconfig option option which doesn't include command bytes for those drivers (fsl_qspi also uses max_write_size, but it's quite complex and didn't check if it would break it...). > >> + >> if DM_SPI >> >> config ALTERA_SPI >> diff --git a/include/spi.h b/include/spi.h >> index 75a994c..310fa4d 100644 >> --- a/include/spi.h >> +++ b/include/spi.h >> @@ -63,6 +63,12 @@ struct dm_spi_slave_platdata { >> >> #endif /* CONFIG_DM_SPI */ >> >> +#ifdef CONFIG_SPI_MAX_WRITE_CMD_BYTES >> +#define spi_max_write(spi, len) (spi->max_write_size - len) >> +#else >> +#define spi_max_write(spi, len) (spi->max_write_size) >> +#endif >> + >> /** >> * struct spi_slave - Representation of a SPI slave >> * >> @@ -89,7 +95,7 @@ struct dm_spi_slave_platdata { >> * @max_read_size: If non-zero, the maximum number of bytes which can >> * be read at once. >> * @max_write_size: If non-zero, the maximum number of bytes which can >> - * be written at once, excluding command bytes. >> + * be written at once. >> * @memory_map: Address of read-only SPI flash access. >> * @flags: Indication of SPI flags. >> */ >> -- >> 2.1.4 >> > > Regards, > Simon > Regards, Álvaro.