[U-Boot] [PATCH 02/10] drivers: spi: add config to consider command bytes when writting to flash

[Álvaro Fernández Rojas]

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 <noltari at gmail.com> 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 <noltari at gmail.com>
>> ---
>>  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.