[PATCH/RFC,2/2] mtd: m25p80: Call a platform power method in the driver

Submitted by Stanimir Varbanov on Aug. 25, 2011, 1:29 p.m.

Details

Message ID b7d4ea5c5ca058568f4d561914fa8becfa9324e1.1314278151.git.svarbanov@mm-sol.com
State RFC
Headers show

Commit Message

Stanimir Varbanov Aug. 25, 2011, 1:29 p.m.
On some devices the flash chip could be powered off when m25p driver
is probed. To avoid erroneous detection the power of the chip must be
turn on, add a power function in m25p_probe to switch on the power by
platform data. The power will be turned off at the probe end.

Also implement a get/put_device callbacks to power on/off the flash
chip.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 drivers/mtd/devices/m25p80.c |   41 +++++++++++++++++++++++++++++++++++++----
 1 files changed, 37 insertions(+), 4 deletions(-)

Comments

Baruch Siach Aug. 28, 2011, 5:22 a.m.
Hi Stanimir,

On Thu, Aug 25, 2011 at 04:29:12PM +0300, Stanimir Varbanov wrote:
> On some devices the flash chip could be powered off when m25p driver
> is probed. To avoid erroneous detection the power of the chip must be
> turn on, add a power function in m25p_probe to switch on the power by
> platform data. The power will be turned off at the probe end.

[snip]

> +	/* power on device while probing */
> +	m25p_power(data, 1);

[snip]

> +out:
> +	m25p_power(data, 0);

Shouldn't you power off in m25p_remove() as well?

baruch
Stanimir Varbanov Aug. 28, 2011, 10:29 a.m.
Hi, Baruch

Thanks for the comments !

Baruch Siach wrote:
> Hi Stanimir,
> 
> On Thu, Aug 25, 2011 at 04:29:12PM +0300, Stanimir Varbanov wrote:
>> On some devices the flash chip could be powered off when m25p driver
>> is probed. To avoid erroneous detection the power of the chip must be
>> turn on, add a power function in m25p_probe to switch on the power by
>> platform data. The power will be turned off at the probe end.
> 
> [snip]
> 
>> +	/* power on device while probing */
>> +	m25p_power(data, 1);
> 
> [snip]
> 
>> +out:
>> +	m25p_power(data, 0);
> 
> Shouldn't you power off in m25p_remove() as well?
> 

IMO, no, because m25p_power(off) is called from m25p_put_device(). Then 
m25p_put_device() is called from mtdchar::mtd_close(). m25p_remove must
fail if user didn't close mtdchar /dev/mtdX device.

The motivation to use power on/off in m25p_probe is because the flash 
chip might be not powered when driver probe is called. The chip is powered
on on the probe begging and powered off after JEDEC probe pass.

> baruch
> 

regards,
Stan

Patch hide | download patch | download mbox

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index e6ba034..6ab5896 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -806,6 +806,27 @@  static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	return ERR_PTR(-ENODEV);
 }
 
+static void m25p_power(struct flash_platform_data *pdata, int on)
+{
+	if (pdata && pdata->power)
+		pdata->power(on);
+}
+
+static int m25p_get_device(struct mtd_info *mtd)
+{
+	struct flash_platform_data *pdata = mtd->dev.parent->platform_data;
+
+	m25p_power(pdata, 1);
+
+	return 0;
+}
+
+static void m25p_put_device(struct mtd_info *mtd)
+{
+	struct flash_platform_data *pdata = mtd->dev.parent->platform_data;
+
+	m25p_power(pdata, 0);
+}
 
 /*
  * board specific setup should have ensured the SPI clock used here
@@ -820,6 +841,7 @@  static int __devinit m25p_probe(struct spi_device *spi)
 	struct flash_info		*info;
 	unsigned			i;
 	struct mtd_part_parser_data	ppdata;
+	int				ret;
 
 	/* Platform data helps sort out which chip type we have, as
 	 * well as how this board partitions it.  If we don't have
@@ -845,12 +867,16 @@  static int __devinit m25p_probe(struct spi_device *spi)
 
 	info = (void *)id->driver_data;
 
+	/* power on device while probing */
+	m25p_power(data, 1);
+
 	if (info->jedec_id) {
 		const struct spi_device_id *jid;
 
 		jid = jedec_probe(spi);
 		if (IS_ERR(jid)) {
-			return PTR_ERR(jid);
+			ret = PTR_ERR(jid);
+			goto out;
 		} else if (jid != id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
@@ -867,12 +893,15 @@  static int __devinit m25p_probe(struct spi_device *spi)
 	}
 
 	flash = kzalloc(sizeof *flash, GFP_KERNEL);
-	if (!flash)
-		return -ENOMEM;
+	if (!flash) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	flash->command = kmalloc(MAX_CMD_SIZE + FAST_READ_DUMMY_BYTE, GFP_KERNEL);
 	if (!flash->command) {
 		kfree(flash);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	flash->spi = spi;
@@ -902,6 +931,8 @@  static int __devinit m25p_probe(struct spi_device *spi)
 	flash->mtd.size = info->sector_size * info->n_sectors;
 	flash->mtd.erase = m25p80_erase;
 	flash->mtd.read = m25p80_read;
+	flash->mtd.get_device = m25p_get_device;
+	flash->mtd.put_device = m25p_put_device;
 
 	/* sst flash chips use AAI word program */
 	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_SST)
@@ -956,6 +987,8 @@  static int __devinit m25p_probe(struct spi_device *spi)
 				flash->mtd.eraseregions[i].erasesize / 1024,
 				flash->mtd.eraseregions[i].numblocks);
 
+out:
+	m25p_power(data, 0);
 
 	/* partitions should match sector boundaries; and it may be good to
 	 * use readonly partitions for writeprotected sectors (BP2..BP0).