Re: [PATCH REPOST v3] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>]

Hello!

On 12/23/2019 04:45 PM, Tudor.Ambarus@xxxxxxxxxxxxx wrote:

>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Make use of the spi-mem direct mapping API to let advanced controllers
>> optimize read/write operations when they support direct mapping.
>>
>> Based on the original patch by Boris Brezillon <boris.brezillon@xxxxxxxxxxx>.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
>>
>> ---
>> The patch is against the 'spi-nor/next' branch of the MTD 'linux.git repo...
>> Forgot to post to 'linux-mtd' 1st time... :-(
>>
>> Changes in version 3:
>> - simplified the way spi_mem_dirmap_{read|write}() are called;
>> - refreshed the patch;
>> - added Boris' tag.
>>
>> Changes in version 2:
>> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>>   spi_nor_spimem_{read|write}_data().
>>
>>  drivers/mtd/spi-nor/spi-nor.c |   66 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nor.h   |    5 +++
>>  2 files changed, 71 insertions(+)
>>
>> Index: linux/drivers/mtd/spi-nor/spi-nor.c
>> ===================================================================
>> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
>> +++ linux/drivers/mtd/spi-nor/spi-nor.c
>> @@ -311,6 +311,9 @@ static ssize_t spi_nor_spimem_read_data(
>>                            SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
>>                            SPI_MEM_OP_DATA_IN(len, buf, 1));
>>
>> +       if (nor->dirmap.rdesc)
>> +               return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);
>> +
>>         /* get transfer protocols. */
>>         op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>>         op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
>> @@ -360,6 +363,9 @@ static ssize_t spi_nor_spimem_write_data
>>                            SPI_MEM_OP_NO_DUMMY,
>>                            SPI_MEM_OP_DATA_OUT(len, buf, 1));
>>
>> +       if (nor->dirmap.wdesc)
> 
> This check is always true.

   Not really, the rdesc/wdesc pointers are NULL during probing:

spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi-nor spi0.0: Failed to parse optional parameter table: ff81                  
spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000                
spi-nor spi0.0: s25fs512s (65536 Kbytes)                                        
11 fixed-partitions partitions found on MTD device spi0.0                       
Creating 11 MTD partitions on "spi0.0":                                         
0x000000000000-0x000000040000 : "bootparam"                                     
0x000000040000-0x0000000c0000 : "cr7"                                           
0x0000000c0000-0x000000140000 : "cert_header_sa3"                               
0x000000140000-0x000000180000 : "bl2"                                           
0x000000180000-0x0000001c0000 : "cert_header_sa6"                               
0x0000001c0000-0x000000620000 : "bl31"                                          
0x000000640000-0x000000700000 : "uboot"                                         
0x000000700000-0x000000740000 : "uboot-env"                                     
0x000000740000-0x0000007c0000 : "dtb"                                           
0x0000007c0000-0x000001bc0000 : "kernel"                                        
0x000001bc0000-0x000004000000 : "user"                                          

   Perhaps the order of calls during probe should be changed?

> The dirmap API is available just for the spi-mem
> drivers, and we are already in the nor->spimem case.
> 
>> +               return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf);
>> +
> This doesn't guarantee that the buffer is DMA-able. You should instead modify
> the spi_nor_spimem_xfer_data() method.

   Still looking into this...
 
> Cheers,
> ta

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/