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

login
register
mail settings
Submitter Stanimir Varbanov
Date Aug. 25, 2011, 1:29 p.m.
Message ID <b7d4ea5c5ca058568f4d561914fa8becfa9324e1.1314278151.git.svarbanov@mm-sol.com>
Download mbox | patch
Permalink /patch/111570/
State RFC
Headers show

Comments

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(-)
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

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).