Patchwork [PATCHv2,07/10] drivers: mtd: m25p80: Adapt driver to support memory mapped read.

login
register
mail settings
Submitter Poddar, Sourav
Date Dec. 6, 2013, 2:24 p.m.
Message ID <1386339891-32717-8-git-send-email-sourav.poddar@ti.com>
Download mbox | patch
Permalink /patch/298031/
State New
Headers show

Comments

Poddar, Sourav - Dec. 6, 2013, 2:24 p.m.
Adapt driver to do a memory mapped read.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
v1->v2:
 - Add a check to Wait for the previous erase/write to
   finish.
 - Ensure proper locking
 drivers/mtd/devices/m25p80.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
Huang Shijie - Dec. 12, 2013, 7:55 a.m.
On Fri, Dec 06, 2013 at 07:54:48PM +0530, Sourav Poddar wrote:
> Adapt driver to do a memory mapped read.
> @@ -109,6 +109,7 @@ struct m25p {
>  	u8			program_opcode;
>  	u8			*command;
>  	enum read_type		flash_read;
> +	void __iomem *mem_addr;

I think we can remove this field.
You can use a local variable in the m25p80_read.

>  };
>  
>  static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
> @@ -515,6 +516,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	size_t *retlen, u_char *buf)
>  {
>  	struct m25p *flash = mtd_to_m25p(mtd);
> +	struct spi_master *master = flash->spi->master;
>  	struct spi_transfer t[2];
>  	struct spi_message m;
>  	uint8_t opcode;
> @@ -523,6 +525,20 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	pr_debug("%s: %s from 0x%08x, len %zd\n", dev_name(&flash->spi->dev),
>  			__func__, (u32)from, len);
>  
> +	if (master->mmap) {
> +		mutex_lock(&flash->lock);
> +		/* Wait till previous write/erase is done. */
> +		if (wait_till_ready(flash)) {
> +			mutex_unlock(&flash->lock);
> +			return 1;
> +		}
> +		flash->mem_addr = master->get_buf(master);
> +		memcpy(buf, flash->mem_addr + from, len);
> +		master->put_buf(master);
> +		*retlen = len;
> +		goto out;
> +	}
> +
>  	spi_message_init(&m);
>  	memset(t, 0, (sizeof t));
>  
> @@ -558,6 +574,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>  
>  	*retlen = m.actual_length - m25p_cmdsz(flash) - dummy;
>  
> +out:
>  	mutex_unlock(&flash->lock);
>  
>  	return 0;
> @@ -1286,6 +1303,9 @@ static int m25p_probe(struct spi_device *spi)
>  		flash->addr_width = 3;
>  	}
>  
> +	if (spi->master->configure_from_slave)
> +		m25p80_fill_flash_information(flash);
> +
You have add a configure_from_slave hook in the SPI, why you also need 
a same hook in the SPI-NOR framework?

And i think the enable_mmap/disable_mmap is not needed too.

All the three hooks are used to set the SPI bus controller.

And the SPI-NOR framework only handles the issues between the 
SPI bus controller and the SPI-NOR, or the SPI-NOR controller and the
SPI-NOR.

thanks
Huang Shijie
Poddar, Sourav - Dec. 12, 2013, 8:15 a.m.
On Thursday 12 December 2013 01:25 PM, Huang Shijie wrote:
> On Fri, Dec 06, 2013 at 07:54:48PM +0530, Sourav Poddar wrote:
>> Adapt driver to do a memory mapped read.
>> @@ -109,6 +109,7 @@ struct m25p {
>>   	u8			program_opcode;
>>   	u8			*command;
>>   	enum read_type		flash_read;
>> +	void __iomem *mem_addr;
> I think we can remove this field.
> You can use a local variable in the m25p80_read.
>
>>   };
>>
>>   static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
>> @@ -515,6 +516,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>>   	size_t *retlen, u_char *buf)
>>   {
>>   	struct m25p *flash = mtd_to_m25p(mtd);
>> +	struct spi_master *master = flash->spi->master;
>>   	struct spi_transfer t[2];
>>   	struct spi_message m;
>>   	uint8_t opcode;
>> @@ -523,6 +525,20 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>>   	pr_debug("%s: %s from 0x%08x, len %zd\n", dev_name(&flash->spi->dev),
>>   			__func__, (u32)from, len);
>>
>> +	if (master->mmap) {
>> +		mutex_lock(&flash->lock);
>> +		/* Wait till previous write/erase is done. */
>> +		if (wait_till_ready(flash)) {
>> +			mutex_unlock(&flash->lock);
>> +			return 1;
>> +		}
>> +		flash->mem_addr = master->get_buf(master);
>> +		memcpy(buf, flash->mem_addr + from, len);
>> +		master->put_buf(master);
>> +		*retlen = len;
>> +		goto out;
>> +	}
>> +
>>   	spi_message_init(&m);
>>   	memset(t, 0, (sizeof t));
>>
>> @@ -558,6 +574,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>>
>>   	*retlen = m.actual_length - m25p_cmdsz(flash) - dummy;
>>
>> +out:
>>   	mutex_unlock(&flash->lock);
>>
>>   	return 0;
>> @@ -1286,6 +1303,9 @@ static int m25p_probe(struct spi_device *spi)
>>   		flash->addr_width = 3;
>>   	}
>>
>> +	if (spi->master->configure_from_slave)
>> +		m25p80_fill_flash_information(flash);
>> +
> You have add a configure_from_slave hook in the SPI, why you also need
> a same hook in the SPI-NOR framework?
>
We need a way to provoke this, currently it is placed in m25p80. If m25p80
is removed, for where should I handle these in nor framework?
> And i think the enable_mmap/disable_mmap is not needed too.
>
> All the three hooks are used to set the SPI bus controller.

> And the SPI-NOR framework only handles the issues between the
> SPI bus controller and the SPI-NOR, or the SPI-NOR controller and the
> SPI-NOR.
My controller can be used as spi flash controller as well as a spi
controller.
> thanks
> Huang Shijie
>
>
Huang Shijie - Dec. 12, 2013, 8:31 a.m.
On Thu, Dec 12, 2013 at 01:45:24PM +0530, Sourav Poddar wrote:
> On Thursday 12 December 2013 01:25 PM, Huang Shijie wrote:
> >>
> >>+	if (spi->master->configure_from_slave)
> >>+		m25p80_fill_flash_information(flash);
> >>+
> >You have add a configure_from_slave hook in the SPI, why you also need
> >a same hook in the SPI-NOR framework?
> >
> We need a way to provoke this, currently it is placed in m25p80. If m25p80
> is removed, for where should I handle these in nor framework?

The m25p80 is not removed. it is easy to move your code in the
m25p_read.

Please read the patch 4.

> >And i think the enable_mmap/disable_mmap is not needed too.
> >
> >All the three hooks are used to set the SPI bus controller.
> 
> >And the SPI-NOR framework only handles the issues between the
> >SPI bus controller and the SPI-NOR, or the SPI-NOR controller and the
> >SPI-NOR.
> My controller can be used as spi flash controller as well as a spi
> controller.
> 
If you really think we need to do something before the do the real work.
we can add two hooks : prepare/unprepare.

   for you, you can put the the enable_mmap/disable_mmap in the
   prepare/unprepare separately.

   for other driver, we can put the enable/disable clocks in the
   prepare/unprepare hooks.

thanks
Huang Shijie

Patch

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index b90c7e5..eb75d84 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -109,6 +109,7 @@  struct m25p {
 	u8			program_opcode;
 	u8			*command;
 	enum read_type		flash_read;
+	void __iomem *mem_addr;
 };
 
 static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
@@ -515,6 +516,7 @@  static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	size_t *retlen, u_char *buf)
 {
 	struct m25p *flash = mtd_to_m25p(mtd);
+	struct spi_master *master = flash->spi->master;
 	struct spi_transfer t[2];
 	struct spi_message m;
 	uint8_t opcode;
@@ -523,6 +525,20 @@  static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	pr_debug("%s: %s from 0x%08x, len %zd\n", dev_name(&flash->spi->dev),
 			__func__, (u32)from, len);
 
+	if (master->mmap) {
+		mutex_lock(&flash->lock);
+		/* Wait till previous write/erase is done. */
+		if (wait_till_ready(flash)) {
+			mutex_unlock(&flash->lock);
+			return 1;
+		}
+		flash->mem_addr = master->get_buf(master);
+		memcpy(buf, flash->mem_addr + from, len);
+		master->put_buf(master);
+		*retlen = len;
+		goto out;
+	}
+
 	spi_message_init(&m);
 	memset(t, 0, (sizeof t));
 
@@ -558,6 +574,7 @@  static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 
 	*retlen = m.actual_length - m25p_cmdsz(flash) - dummy;
 
+out:
 	mutex_unlock(&flash->lock);
 
 	return 0;
@@ -1286,6 +1303,9 @@  static int m25p_probe(struct spi_device *spi)
 		flash->addr_width = 3;
 	}
 
+	if (spi->master->configure_from_slave)
+		m25p80_fill_flash_information(flash);
+
 	dev_info(&spi->dev, "%s (%lld Kbytes)\n", id->name,
 			(long long)flash->mtd.size >> 10);