Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1267613

Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash devices

Show all headers | View raw

Hi Brian,

On 11/12/2015 12:54 AM, Brian Norris wrote:
> In addition to my other comments:
> 

[...]

>> +	int (*spi_mtd_mmap_read)(struct  spi_device *spi,
>> +				 loff_t from, size_t len,
>> +				 size_t *retlen, u_char *buf,
>> +				 u8 read_opcode, u8 addr_width,
>> +				 u8 dummy_bytes);
> 
> This is seeming to be a longer and longer list of arguments. I know MTD
> has a bad habit of long argument lists (which then cause a ton of
> unnecessary churn when things need changed in the API), but perhaps we
> can limit the damage to the SPI layer. Perhaps this deserves a struct to
> encapsulate all the flash read arguments? Like:
> 
> struct spi_flash_read_message {
> 	loff_t from;
> 	size_t len;
> 	size_t *retlen;
> 	void *buf;
> 	u8 read_opcode;
> 	u8 addr_width;
> 	u8 dummy_bits;
> 	// additional fields to describe rx_nbits for opcode/addr/data
> };
> 
> struct spi_master {
> 	...
> 	int (*spi_flash_read)(struct spi_device *spi,
> 			      struct spi_flash_message *msg);
> };


Yeah.. I think struct encapsulation helps, this can also be used to pass
sg lists for dma in future. I will rework the series with your
suggestion to include nbits for opcode/addr/data.
Also, will add validation logic (similar to __spi_validate()) to check
whether master supports dual/quad mode for opcode/addr/data. I am
planning to add this validation code to spi_flash_read_validate(in place
of spi_mmap_read_supported())
Thanks!


-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v3 1/5] spi: introduce mmap read support for spi flash devices Vignesh R <vigneshr@ti.com> - 2015-11-10 06:40 +0100
  Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Brian Norris <computersforpeace@gmail.com> - 2015-11-11 00:30 +0100
    Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices "R, Vignesh" <vigneshr@ti.com> - 2015-11-11 08:00 +0100
      Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Brian Norris <computersforpeace@gmail.com> - 2015-11-11 08:30 +0100
        Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Cyrille Pitchen <cyrille.pitchen@atmel.com> - 2015-11-13 17:10 +0100
      Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Mike Looijmans <mike.looijmans@topic.nl> - 2015-11-13 15:40 +0100
  Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Brian Norris <computersforpeace@gmail.com> - 2015-11-11 20:30 +0100
    Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash  devices Vignesh R <vigneshr@ti.com> - 2015-11-12 05:40 +0100

csiph-web