* [PATCH v4 0/2] Added support for spi-nor device pm in m25p80
@ 2017-02-14 15:32 Kamal Dasu
[not found] ` <1487086368-4118-1-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Kamal Dasu @ 2017-02-14 15:32 UTC (permalink / raw)
To: linux-spi-u79uwXL29TY76Z2rM5mHXA,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w, marex-ynQEQJNshbs,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Kamal Dasu
Changes since v3 based on review comments:
spi-nor.h
- Moved flash_info field to end of structure
- Added flash_info entry to the spi_nor structure description
- Reworded the function description
- Reworded the commit message
m25p80.c:
- Only implement and initialize the resume() pm_op
The V4 changes below implements power management support in m25p80 driver.
m25p80 pm resume() calls newly a implemented spi_nor_init() function
that sets up the spi-nor flash to its probed state. This is needed on
platfroms that turn off power to the spi-nor flash on pm suspend.
The code has been rebased to git://github.com/spi-nor/linux.git next
branch.
Kamal Dasu (2):
mtd: spi-nor: Added spi-nor init function
mtd: m25p80: Added pm ops support
drivers/mtd/devices/m25p80.c | 11 +++++++
drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
include/linux/mtd/spi-nor.h | 18 ++++++++++-
3 files changed, 73 insertions(+), 28 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <1487086368-4118-1-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-02-14 15:32 ` Kamal Dasu
[not found] ` <1487086368-4118-2-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-20 8:12 ` Rafał Miłecki
2017-02-14 15:32 ` [PATCH v4 2/2] mtd: m25p80: Added pm ops support Kamal Dasu
1 sibling, 2 replies; 12+ messages in thread
From: Kamal Dasu @ 2017-02-14 15:32 UTC (permalink / raw)
To: linux-spi-u79uwXL29TY76Z2rM5mHXA,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w, marex-ynQEQJNshbs,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Kamal Dasu
Refactored spi_nor_scan() code to add spi_nor_init() function that
programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor
spi_nor_scan() now calls spi_nor_init() to setup nor flash.
Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
include/linux/mtd/spi-nor.h | 18 ++++++++++-
2 files changed, 62 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 70e52ff..2bf7f4f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
return 0;
}
+int spi_nor_init(struct spi_nor *nor)
+{
+ int ret = 0;
+ const struct flash_info *info = nor->info;
+ struct device *dev = nor->dev;
+
+ /*
+ * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+ * with the software protection bits set
+ */
+
+ if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+ JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+ JEDEC_MFR(info) == SNOR_MFR_SST ||
+ info->flags & SPI_NOR_HAS_LOCK) {
+ write_enable(nor);
+ write_sr(nor, 0);
+ spi_nor_wait_till_ready(nor);
+ }
+
+ if (nor->flash_read == SPI_NOR_QUAD) {
+ ret = set_quad_mode(nor, info);
+ if (ret) {
+ dev_err(dev, "quad mode not supported\n");
+ return ret;
+ }
+ }
+
+ if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+ info->flags & SPI_NOR_4B_OPCODES)
+ spi_nor_set_4byte_opcodes(nor, info);
+ else
+ set_4byte(nor, info, 1);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
{
const struct flash_info *info = NULL;
@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
}
}
+ nor->info = info;
mutex_init(&nor->lock);
/*
@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
if (info->flags & SPI_S3AN)
nor->flags |= SNOR_F_READY_XSR_RDY;
- /*
- * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
- * with the software protection bits set
- */
-
- if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
- JEDEC_MFR(info) == SNOR_MFR_INTEL ||
- JEDEC_MFR(info) == SNOR_MFR_SST ||
- info->flags & SPI_NOR_HAS_LOCK) {
- write_enable(nor);
- write_sr(nor, 0);
- spi_nor_wait_till_ready(nor);
- }
-
if (!mtd->name)
mtd->name = dev_name(dev);
mtd->priv = nor;
@@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
/* Quad/Dual-read mode takes precedence over fast/normal */
if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
- ret = set_quad_mode(nor, info);
- if (ret) {
- dev_err(dev, "quad mode not supported\n");
- return ret;
- }
nor->flash_read = SPI_NOR_QUAD;
} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
nor->flash_read = SPI_NOR_DUAL;
@@ -1702,17 +1722,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
if (info->addr_width)
nor->addr_width = info->addr_width;
- else if (mtd->size > 0x1000000) {
+ else if (mtd->size > 0x1000000)
/* enable 4-byte addressing if the device exceeds 16MiB */
nor->addr_width = 4;
- if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
- info->flags & SPI_NOR_4B_OPCODES)
- spi_nor_set_4byte_opcodes(nor, info);
- else
- set_4byte(nor, info, 1);
- } else {
+ else
nor->addr_width = 3;
- }
if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
dev_err(dev, "address width is too large: %u\n",
@@ -1728,6 +1742,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
return ret;
}
+ ret = spi_nor_init(nor);
+ if (ret)
+ return ret;
+
dev_info(dev, "%s (%lld Kbytes)\n", info->name,
(long long)mtd->size >> 10);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..29a8283 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -143,6 +143,8 @@ enum spi_nor_option_flags {
SNOR_F_READY_XSR_RDY = BIT(4),
};
+struct flash_info;
+
/**
* struct spi_nor - Structure for defining a the SPI NOR layer
* @mtd: point to a mtd_info structure
@@ -174,6 +176,7 @@ enum spi_nor_option_flags {
* @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
* completely locked
* @priv: the private data
+ * @info: points to the flash_info structure
*/
struct spi_nor {
struct mtd_info mtd;
@@ -206,6 +209,7 @@ struct spi_nor {
int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
void *priv;
+ const struct flash_info *info;
};
static inline void spi_nor_set_flash_node(struct spi_nor *nor,
@@ -220,12 +224,24 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
}
/**
+ * spi_nor_init() - initialize SPI NOR
+ * @nor: the spi_nor structure
+ *
+ * The drivers uses this function to initialize the SPI NOR flash device to
+ * settings in spi_nor structure. The functions sets read mode, address width
+ * and removes protection on the flash device based on those settings.
+ *
+ * Return: 0 for success, others for failure.
+ */
+int spi_nor_init(struct spi_nor *nor);
+
+/**
* spi_nor_scan() - scan the SPI NOR
* @nor: the spi_nor structure
* @name: the chip type name
* @mode: the read mode supported by the driver
*
- * The drivers can use this fuction to scan the SPI NOR.
+ * The drivers can use this function to scan the SPI NOR.
* In the scanning, it will try to get all the necessary information to
* fill the mtd_info{} and the spi_nor{}.
*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/2] mtd: m25p80: Added pm ops support
[not found] ` <1487086368-4118-1-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-14 15:32 ` [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function Kamal Dasu
@ 2017-02-14 15:32 ` Kamal Dasu
[not found] ` <1487086368-4118-3-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 12+ messages in thread
From: Kamal Dasu @ 2017-02-14 15:32 UTC (permalink / raw)
To: linux-spi-u79uwXL29TY76Z2rM5mHXA,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w, marex-ynQEQJNshbs,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Kamal Dasu
Added power management ops for resume to be able to resan spi-nor
device and set it to right transfer modes in its probed state after
poweron. Some SoC implementations might power down the spi-nor flash
and loose its initial settings on suspend. A resume should retore the
part to its probed state.
Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/mtd/devices/m25p80.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c4df3b1..3ab30b2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -324,10 +324,21 @@ static int m25p_remove(struct spi_device *spi)
};
MODULE_DEVICE_TABLE(of, m25p_of_table);
+#ifdef CONFIG_PM_SLEEP
+static int m25p_resume(struct device *dev)
+{
+ struct m25p *flash = dev_get_drvdata(dev);
+
+ return spi_nor_init(&flash->spi_nor);
+}
+#endif
+static SIMPLE_DEV_PM_OPS(m25p_pm_ops, NULL, m25p_resume);
+
static struct spi_driver m25p80_driver = {
.driver = {
.name = "m25p80",
.of_match_table = m25p_of_table,
+ .pm = &m25p_pm_ops,
},
.id_table = m25p_ids,
.probe = m25p_probe,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <1487086368-4118-2-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-02-18 22:29 ` Marek Vasut
[not found] ` <b6079dbb-82fc-7579-32b2-b8c071ef02ef-ynQEQJNshbs@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2017-02-18 22:29 UTC (permalink / raw)
To: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> Refactored spi_nor_scan() code to add spi_nor_init() function that
> programs the spi-nor flash to:
> 1) remove flash protection if applicable
> 2) set read mode to quad mode if configured such
> 3) set the address width based on the flash size and vendor
>
> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>
> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Changelog missing ?
> ---
> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
> include/linux/mtd/spi-nor.h | 18 ++++++++++-
> 2 files changed, 62 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 70e52ff..2bf7f4f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
> return 0;
> }
>
> +int spi_nor_init(struct spi_nor *nor)
> +{
> + int ret = 0;
> + const struct flash_info *info = nor->info;
> + struct device *dev = nor->dev;
> +
> + /*
> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
> + * with the software protection bits set
> + */
> +
> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> + JEDEC_MFR(info) == SNOR_MFR_SST ||
> + info->flags & SPI_NOR_HAS_LOCK) {
> + write_enable(nor);
> + write_sr(nor, 0);
> + spi_nor_wait_till_ready(nor);
> + }
> +
> + if (nor->flash_read == SPI_NOR_QUAD) {
> + ret = set_quad_mode(nor, info);
> + if (ret) {
> + dev_err(dev, "quad mode not supported\n");
> + return ret;
> + }
> + }
> +
> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> + info->flags & SPI_NOR_4B_OPCODES)
> + spi_nor_set_4byte_opcodes(nor, info);
> + else
> + set_4byte(nor, info, 1);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(spi_nor_init);
> +
> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> {
> const struct flash_info *info = NULL;
> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> }
> }
>
> + nor->info = info;
> mutex_init(&nor->lock);
>
> /*
> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> if (info->flags & SPI_S3AN)
> nor->flags |= SNOR_F_READY_XSR_RDY;
>
> - /*
> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
> - * with the software protection bits set
> - */
> -
> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> - JEDEC_MFR(info) == SNOR_MFR_SST ||
> - info->flags & SPI_NOR_HAS_LOCK) {
> - write_enable(nor);
> - write_sr(nor, 0);
> - spi_nor_wait_till_ready(nor);
> - }
> -
> if (!mtd->name)
> mtd->name = dev_name(dev);
> mtd->priv = nor;
> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>
> /* Quad/Dual-read mode takes precedence over fast/normal */
> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
> - ret = set_quad_mode(nor, info);
> - if (ret) {
> - dev_err(dev, "quad mode not supported\n");
> - return ret;
> - }
> nor->flash_read = SPI_NOR_QUAD;
So from here on, any user will expect quad mode to be correctly set, but
it really isn't. Is that OK ?
> } else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
> nor->flash_read = SPI_NOR_DUAL;
> @@ -1702,17 +1722,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>
> if (info->addr_width)
> nor->addr_width = info->addr_width;
> - else if (mtd->size > 0x1000000) {
> + else if (mtd->size > 0x1000000)
> /* enable 4-byte addressing if the device exceeds 16MiB */
> nor->addr_width = 4;
> - if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> - info->flags & SPI_NOR_4B_OPCODES)
> - spi_nor_set_4byte_opcodes(nor, info);
> - else
> - set_4byte(nor, info, 1);
> - } else {
> + else
> nor->addr_width = 3;
> - }
>
> if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
> dev_err(dev, "address width is too large: %u\n",
> @@ -1728,6 +1742,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> return ret;
> }
>
> + ret = spi_nor_init(nor);
> + if (ret)
> + return ret;
> +
> dev_info(dev, "%s (%lld Kbytes)\n", info->name,
> (long long)mtd->size >> 10);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f2a7180..29a8283 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -143,6 +143,8 @@ enum spi_nor_option_flags {
> SNOR_F_READY_XSR_RDY = BIT(4),
> };
>
> +struct flash_info;
> +
> /**
> * struct spi_nor - Structure for defining a the SPI NOR layer
> * @mtd: point to a mtd_info structure
> @@ -174,6 +176,7 @@ enum spi_nor_option_flags {
> * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
> * completely locked
> * @priv: the private data
> + * @info: points to the flash_info structure
> */
> struct spi_nor {
> struct mtd_info mtd;
> @@ -206,6 +209,7 @@ struct spi_nor {
> int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>
> void *priv;
> + const struct flash_info *info;
> };
>
> static inline void spi_nor_set_flash_node(struct spi_nor *nor,
> @@ -220,12 +224,24 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
> }
>
> /**
> + * spi_nor_init() - initialize SPI NOR
> + * @nor: the spi_nor structure
> + *
> + * The drivers uses this function to initialize the SPI NOR flash device to
> + * settings in spi_nor structure. The functions sets read mode, address width
> + * and removes protection on the flash device based on those settings.
> + *
> + * Return: 0 for success, others for failure.
> + */
> +int spi_nor_init(struct spi_nor *nor);
> +
> +/**
> * spi_nor_scan() - scan the SPI NOR
> * @nor: the spi_nor structure
> * @name: the chip type name
> * @mode: the read mode supported by the driver
> *
> - * The drivers can use this fuction to scan the SPI NOR.
> + * The drivers can use this function to scan the SPI NOR.
> * In the scanning, it will try to get all the necessary information to
> * fill the mtd_info{} and the spi_nor{}.
> *
>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support
[not found] ` <1487086368-4118-3-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-02-18 22:31 ` Marek Vasut
[not found] ` <e5fb0bdf-0956-e364-14fb-c255471f371d-ynQEQJNshbs@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2017-02-18 22:31 UTC (permalink / raw)
To: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> Added power management ops for resume to be able to resan spi-nor
rescan ... but you're not really rescanning it, are you ? You're just
reconfiguring the pre-suspend parameters .
> device and set it to right transfer modes in its probed state after
> poweron. Some SoC implementations might power down the spi-nor flash
> and loose its initial settings on suspend. A resume should retore the
restore ...
> part to its probed state.
>
> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/mtd/devices/m25p80.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c4df3b1..3ab30b2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -324,10 +324,21 @@ static int m25p_remove(struct spi_device *spi)
> };
> MODULE_DEVICE_TABLE(of, m25p_of_table);
>
> +#ifdef CONFIG_PM_SLEEP
> +static int m25p_resume(struct device *dev)
> +{
> + struct m25p *flash = dev_get_drvdata(dev);
> +
> + return spi_nor_init(&flash->spi_nor);
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(m25p_pm_ops, NULL, m25p_resume);
> +
> static struct spi_driver m25p80_driver = {
> .driver = {
> .name = "m25p80",
> .of_match_table = m25p_of_table,
> + .pm = &m25p_pm_ops,
> },
> .id_table = m25p_ids,
> .probe = m25p_probe,
>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <b6079dbb-82fc-7579-32b2-b8c071ef02ef-ynQEQJNshbs@public.gmane.org>
@ 2017-02-19 9:47 ` Kamal Dasu
[not found] ` <CAKekbesKKS_s873D=xy2tQcVYei_26gYipn2ru1rVY4ihXbsXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Kamal Dasu @ 2017-02-19 9:47 UTC (permalink / raw)
To: Marek Vasut
Cc: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA, Cyrille Pitchen,
Mark Brown, MTD Maling List, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>> programs the spi-nor flash to:
>> 1) remove flash protection if applicable
>> 2) set read mode to quad mode if configured such
>> 3) set the address width based on the flash size and vendor
>>
>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>
>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Changelog missing ?
Yes will add it and resend.
>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>> include/linux/mtd/spi-nor.h | 18 ++++++++++-
>> 2 files changed, 62 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 70e52ff..2bf7f4f 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>> return 0;
>> }
>>
>> +int spi_nor_init(struct spi_nor *nor)
>> +{
>> + int ret = 0;
>> + const struct flash_info *info = nor->info;
>> + struct device *dev = nor->dev;
>> +
>> + /*
>> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>> + * with the software protection bits set
>> + */
>> +
>> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>> + JEDEC_MFR(info) == SNOR_MFR_SST ||
>> + info->flags & SPI_NOR_HAS_LOCK) {
>> + write_enable(nor);
>> + write_sr(nor, 0);
>> + spi_nor_wait_till_ready(nor);
>> + }
>> +
>> + if (nor->flash_read == SPI_NOR_QUAD) {
>> + ret = set_quad_mode(nor, info);
>> + if (ret) {
>> + dev_err(dev, "quad mode not supported\n");
>> + return ret;
>> + }
>> + }
quad mode is being set in the above block of code.
>> +
>> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>> + info->flags & SPI_NOR_4B_OPCODES)
>> + spi_nor_set_4byte_opcodes(nor, info);
>> + else
>> + set_4byte(nor, info, 1);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(spi_nor_init);
>> +
>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>> {
>> const struct flash_info *info = NULL;
>> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>> }
>> }
>>
>> + nor->info = info;
>> mutex_init(&nor->lock);
>>
>> /*
>> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>> if (info->flags & SPI_S3AN)
>> nor->flags |= SNOR_F_READY_XSR_RDY;
>>
>> - /*
>> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>> - * with the software protection bits set
>> - */
>> -
>> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>> - JEDEC_MFR(info) == SNOR_MFR_SST ||
>> - info->flags & SPI_NOR_HAS_LOCK) {
>> - write_enable(nor);
>> - write_sr(nor, 0);
>> - spi_nor_wait_till_ready(nor);
>> - }
>> -
>> if (!mtd->name)
>> mtd->name = dev_name(dev);
>> mtd->priv = nor;
>> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>
>> /* Quad/Dual-read mode takes precedence over fast/normal */
>> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>> - ret = set_quad_mode(nor, info);
>> - if (ret) {
>> - dev_err(dev, "quad mode not supported\n");
>> - return ret;
>> - }
>> nor->flash_read = SPI_NOR_QUAD;
>
> So from here on, any user will expect quad mode to be correctly set, but
> it really isn't. Is that OK ?
It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;
>
>> } else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
>> nor->flash_read = SPI_NOR_DUAL;
>> @@ -1702,17 +1722,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>
>> if (info->addr_width)
>> nor->addr_width = info->addr_width;
>> - else if (mtd->size > 0x1000000) {
>> + else if (mtd->size > 0x1000000)
>> /* enable 4-byte addressing if the device exceeds 16MiB */
>> nor->addr_width = 4;
>> - if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>> - info->flags & SPI_NOR_4B_OPCODES)
>> - spi_nor_set_4byte_opcodes(nor, info);
>> - else
>> - set_4byte(nor, info, 1);
>> - } else {
>> + else
>> nor->addr_width = 3;
>> - }
>>
>> if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
>> dev_err(dev, "address width is too large: %u\n",
>> @@ -1728,6 +1742,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>> return ret;
>> }
>>
>> + ret = spi_nor_init(nor);
>> + if (ret)
>> + return ret;
>> +
>> dev_info(dev, "%s (%lld Kbytes)\n", info->name,
>> (long long)mtd->size >> 10);
>>
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index f2a7180..29a8283 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -143,6 +143,8 @@ enum spi_nor_option_flags {
>> SNOR_F_READY_XSR_RDY = BIT(4),
>> };
>>
>> +struct flash_info;
>> +
>> /**
>> * struct spi_nor - Structure for defining a the SPI NOR layer
>> * @mtd: point to a mtd_info structure
>> @@ -174,6 +176,7 @@ enum spi_nor_option_flags {
>> * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
>> * completely locked
>> * @priv: the private data
>> + * @info: points to the flash_info structure
>> */
>> struct spi_nor {
>> struct mtd_info mtd;
>> @@ -206,6 +209,7 @@ struct spi_nor {
>> int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>
>> void *priv;
>> + const struct flash_info *info;
>> };
>>
>> static inline void spi_nor_set_flash_node(struct spi_nor *nor,
>> @@ -220,12 +224,24 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
>> }
>>
>> /**
>> + * spi_nor_init() - initialize SPI NOR
>> + * @nor: the spi_nor structure
>> + *
>> + * The drivers uses this function to initialize the SPI NOR flash device to
>> + * settings in spi_nor structure. The functions sets read mode, address width
>> + * and removes protection on the flash device based on those settings.
>> + *
>> + * Return: 0 for success, others for failure.
>> + */
>> +int spi_nor_init(struct spi_nor *nor);
>> +
>> +/**
>> * spi_nor_scan() - scan the SPI NOR
>> * @nor: the spi_nor structure
>> * @name: the chip type name
>> * @mode: the read mode supported by the driver
>> *
>> - * The drivers can use this fuction to scan the SPI NOR.
>> + * The drivers can use this function to scan the SPI NOR.
>> * In the scanning, it will try to get all the necessary information to
>> * fill the mtd_info{} and the spi_nor{}.
>> *
>>
>
>
> --
> Best regards,
> Marek Vasut
Thanks
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support
[not found] ` <e5fb0bdf-0956-e364-14fb-c255471f371d-ynQEQJNshbs@public.gmane.org>
@ 2017-02-19 9:48 ` Kamal Dasu
0 siblings, 0 replies; 12+ messages in thread
From: Kamal Dasu @ 2017-02-19 9:48 UTC (permalink / raw)
To: Marek Vasut
Cc: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA, Cyrille Pitchen,
Mark Brown, MTD Maling List, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On Sat, Feb 18, 2017 at 5:31 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> Added power management ops for resume to be able to resan spi-nor
>
> rescan ... but you're not really rescanning it, are you ? You're just
> reconfiguring the pre-suspend parameters .
>
>> device and set it to right transfer modes in its probed state after
>> poweron. Some SoC implementations might power down the spi-nor flash
>> and loose its initial settings on suspend. A resume should retore the
>
> restore ...
>
>> part to its probed state.
>>
Will fix typos.
>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/mtd/devices/m25p80.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c4df3b1..3ab30b2 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -324,10 +324,21 @@ static int m25p_remove(struct spi_device *spi)
>> };
>> MODULE_DEVICE_TABLE(of, m25p_of_table);
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int m25p_resume(struct device *dev)
>> +{
>> + struct m25p *flash = dev_get_drvdata(dev);
>> +
>> + return spi_nor_init(&flash->spi_nor);
>> +}
>> +#endif
>> +static SIMPLE_DEV_PM_OPS(m25p_pm_ops, NULL, m25p_resume);
>> +
>> static struct spi_driver m25p80_driver = {
>> .driver = {
>> .name = "m25p80",
>> .of_match_table = m25p_of_table,
>> + .pm = &m25p_pm_ops,
>> },
>> .id_table = m25p_ids,
>> .probe = m25p_probe,
>>
>
>
> --
> Best regards,
> Marek Vasut
Thanks
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <CAKekbesKKS_s873D=xy2tQcVYei_26gYipn2ru1rVY4ihXbsXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-02-19 10:37 ` Marek Vasut
[not found] ` <54393706-cd93-603a-193b-25fbf52b2d83-ynQEQJNshbs@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2017-02-19 10:37 UTC (permalink / raw)
To: Kamal Dasu
Cc: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA, Cyrille Pitchen,
Mark Brown, MTD Maling List, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On 02/19/2017 10:47 AM, Kamal Dasu wrote:
> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>>> programs the spi-nor flash to:
>>> 1) remove flash protection if applicable
>>> 2) set read mode to quad mode if configured such
>>> 3) set the address width based on the flash size and vendor
>>>
>>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>>
>>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Changelog missing ?
>
> Yes will add it and resend.
>
>>
>>> ---
>>> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>> include/linux/mtd/spi-nor.h | 18 ++++++++++-
>>> 2 files changed, 62 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 70e52ff..2bf7f4f 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>>> return 0;
>>> }
>>>
>>> +int spi_nor_init(struct spi_nor *nor)
>>> +{
>>> + int ret = 0;
>>> + const struct flash_info *info = nor->info;
>>> + struct device *dev = nor->dev;
>>> +
>>> + /*
>>> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>> + * with the software protection bits set
>>> + */
>>> +
>>> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>> + JEDEC_MFR(info) == SNOR_MFR_SST ||
>>> + info->flags & SPI_NOR_HAS_LOCK) {
>>> + write_enable(nor);
>>> + write_sr(nor, 0);
>>> + spi_nor_wait_till_ready(nor);
>>> + }
>>> +
>>> + if (nor->flash_read == SPI_NOR_QUAD) {
>>> + ret = set_quad_mode(nor, info);
>>> + if (ret) {
>>> + dev_err(dev, "quad mode not supported\n");
>>> + return ret;
>>> + }
>>> + }
>
> quad mode is being set in the above block of code.
>
>>> +
>>> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>> + info->flags & SPI_NOR_4B_OPCODES)
>>> + spi_nor_set_4byte_opcodes(nor, info);
>>> + else
>>> + set_4byte(nor, info, 1);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(spi_nor_init);
>>> +
>>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>> {
>>> const struct flash_info *info = NULL;
>>> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>> }
>>> }
>>>
>>> + nor->info = info;
>>> mutex_init(&nor->lock);
>>>
>>> /*
>>> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>> if (info->flags & SPI_S3AN)
>>> nor->flags |= SNOR_F_READY_XSR_RDY;
>>>
>>> - /*
>>> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>> - * with the software protection bits set
>>> - */
>>> -
>>> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>> - JEDEC_MFR(info) == SNOR_MFR_SST ||
>>> - info->flags & SPI_NOR_HAS_LOCK) {
>>> - write_enable(nor);
>>> - write_sr(nor, 0);
>>> - spi_nor_wait_till_ready(nor);
>>> - }
>>> -
>>> if (!mtd->name)
>>> mtd->name = dev_name(dev);
>>> mtd->priv = nor;
>>> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>
>>> /* Quad/Dual-read mode takes precedence over fast/normal */
>>> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>>> - ret = set_quad_mode(nor, info);
>>> - if (ret) {
>>> - dev_err(dev, "quad mode not supported\n");
>>> - return ret;
>>> - }
>>> nor->flash_read = SPI_NOR_QUAD;
>>
>> So from here on, any user will expect quad mode to be correctly set, but
>> it really isn't. Is that OK ?
>
> It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;
>
But that's invoked later, so whoever adds code between this place and
the invocation of spi_nor_init() will be misled into believing the SPI
NOR is set up in quad-mode, while it would not be, right ?
[...]
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <54393706-cd93-603a-193b-25fbf52b2d83-ynQEQJNshbs@public.gmane.org>
@ 2017-02-19 22:38 ` Kamal Dasu
[not found] ` <CAKekbetnpY=zRJM0=xn5t2enDcts5tQTV6ZtvPaW_geOd3=DQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Kamal Dasu @ 2017-02-19 22:38 UTC (permalink / raw)
To: Marek Vasut
Cc: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA, Cyrille Pitchen,
Mark Brown, MTD Maling List, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
Marek,
On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>>>> programs the spi-nor flash to:
>>>> 1) remove flash protection if applicable
>>>> 2) set read mode to quad mode if configured such
>>>> 3) set the address width based on the flash size and vendor
>>>>
>>>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>>>
>>>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> Changelog missing ?
>>
>> Yes will add it and resend.
>>
>>>
>>>> ---
>>>> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>> include/linux/mtd/spi-nor.h | 18 ++++++++++-
>>>> 2 files changed, 62 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 70e52ff..2bf7f4f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>>>> return 0;
>>>> }
>>>>
>>>> +int spi_nor_init(struct spi_nor *nor)
>>>> +{
>>>> + int ret = 0;
>>>> + const struct flash_info *info = nor->info;
>>>> + struct device *dev = nor->dev;
>>>> +
>>>> + /*
>>>> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>> + * with the software protection bits set
>>>> + */
>>>> +
>>>> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>> + JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>> + info->flags & SPI_NOR_HAS_LOCK) {
>>>> + write_enable(nor);
>>>> + write_sr(nor, 0);
>>>> + spi_nor_wait_till_ready(nor);
>>>> + }
>>>> +
>>>> + if (nor->flash_read == SPI_NOR_QUAD) {
>>>> + ret = set_quad_mode(nor, info);
>>>> + if (ret) {
>>>> + dev_err(dev, "quad mode not supported\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>
>> quad mode is being set in the above block of code.
>>
>>>> +
>>>> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>>> + info->flags & SPI_NOR_4B_OPCODES)
>>>> + spi_nor_set_4byte_opcodes(nor, info);
>>>> + else
>>>> + set_4byte(nor, info, 1);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(spi_nor_init);
>>>> +
>>>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> {
>>>> const struct flash_info *info = NULL;
>>>> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> }
>>>> }
>>>>
>>>> + nor->info = info;
>>>> mutex_init(&nor->lock);
>>>>
>>>> /*
>>>> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> if (info->flags & SPI_S3AN)
>>>> nor->flags |= SNOR_F_READY_XSR_RDY;
>>>>
>>>> - /*
>>>> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>> - * with the software protection bits set
>>>> - */
>>>> -
>>>> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>> - JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>> - info->flags & SPI_NOR_HAS_LOCK) {
>>>> - write_enable(nor);
>>>> - write_sr(nor, 0);
>>>> - spi_nor_wait_till_ready(nor);
>>>> - }
>>>> -
>>>> if (!mtd->name)
>>>> mtd->name = dev_name(dev);
>>>> mtd->priv = nor;
>>>> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>
>>>> /* Quad/Dual-read mode takes precedence over fast/normal */
>>>> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>>>> - ret = set_quad_mode(nor, info);
>>>> - if (ret) {
>>>> - dev_err(dev, "quad mode not supported\n");
>>>> - return ret;
>>>> - }
>>>> nor->flash_read = SPI_NOR_QUAD;
>>>
>>> So from here on, any user will expect quad mode to be correctly set, but
>>> it really isn't. Is that OK ?
>>
>> It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;
>>
>
> But that's invoked later, so whoever adds code between this place and
> the invocation of spi_nor_init() will be misled into believing the SPI
> NOR is set up in quad-mode, while it would not be, right ?
>
I made this change based on previous comments from Cyrlle. All
spi-nor based settings that need commands to the flash happens in one
function now. The code before calling spi_nor_init is setting the
configuration in the nor structure. I don't see how its very different
from before. I did separate each setting in functions without changing
the flow in spi_nor_scan() before and was to told to change it this
way to be more optimal. I don't see how anyone will have any
confusion. So please let me know how exactly you want it.
> [...]
>
> --
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
[not found] ` <CAKekbetnpY=zRJM0=xn5t2enDcts5tQTV6ZtvPaW_geOd3=DQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-02-19 22:51 ` Marek Vasut
0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2017-02-19 22:51 UTC (permalink / raw)
To: Kamal Dasu
Cc: Kamal Dasu, linux-spi-u79uwXL29TY76Z2rM5mHXA, Cyrille Pitchen,
Mark Brown, MTD Maling List, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
On 02/19/2017 11:38 PM, Kamal Dasu wrote:
> Marek,
>
> On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>>>>> programs the spi-nor flash to:
>>>>> 1) remove flash protection if applicable
>>>>> 2) set read mode to quad mode if configured such
>>>>> 3) set the address width based on the flash size and vendor
>>>>>
>>>>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>>>>
>>>>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> Changelog missing ?
>>>
>>> Yes will add it and resend.
>>>
>>>>
>>>>> ---
>>>>> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>>> include/linux/mtd/spi-nor.h | 18 ++++++++++-
>>>>> 2 files changed, 62 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>>> index 70e52ff..2bf7f4f 100644
>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +int spi_nor_init(struct spi_nor *nor)
>>>>> +{
>>>>> + int ret = 0;
>>>>> + const struct flash_info *info = nor->info;
>>>>> + struct device *dev = nor->dev;
>>>>> +
>>>>> + /*
>>>>> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>>> + * with the software protection bits set
>>>>> + */
>>>>> +
>>>>> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>>> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>>> + JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>>> + info->flags & SPI_NOR_HAS_LOCK) {
>>>>> + write_enable(nor);
>>>>> + write_sr(nor, 0);
>>>>> + spi_nor_wait_till_ready(nor);
>>>>> + }
>>>>> +
>>>>> + if (nor->flash_read == SPI_NOR_QUAD) {
>>>>> + ret = set_quad_mode(nor, info);
>>>>> + if (ret) {
>>>>> + dev_err(dev, "quad mode not supported\n");
>>>>> + return ret;
>>>>> + }
>>>>> + }
>>>
>>> quad mode is being set in the above block of code.
>>>
>>>>> +
>>>>> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>>>> + info->flags & SPI_NOR_4B_OPCODES)
>>>>> + spi_nor_set_4byte_opcodes(nor, info);
>>>>> + else
>>>>> + set_4byte(nor, info, 1);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(spi_nor_init);
>>>>> +
>>>>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>> {
>>>>> const struct flash_info *info = NULL;
>>>>> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>> }
>>>>> }
>>>>>
>>>>> + nor->info = info;
>>>>> mutex_init(&nor->lock);
>>>>>
>>>>> /*
>>>>> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>> if (info->flags & SPI_S3AN)
>>>>> nor->flags |= SNOR_F_READY_XSR_RDY;
>>>>>
>>>>> - /*
>>>>> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>>> - * with the software protection bits set
>>>>> - */
>>>>> -
>>>>> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>>> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>>> - JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>>> - info->flags & SPI_NOR_HAS_LOCK) {
>>>>> - write_enable(nor);
>>>>> - write_sr(nor, 0);
>>>>> - spi_nor_wait_till_ready(nor);
>>>>> - }
>>>>> -
>>>>> if (!mtd->name)
>>>>> mtd->name = dev_name(dev);
>>>>> mtd->priv = nor;
>>>>> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>>
>>>>> /* Quad/Dual-read mode takes precedence over fast/normal */
>>>>> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>>>>> - ret = set_quad_mode(nor, info);
>>>>> - if (ret) {
>>>>> - dev_err(dev, "quad mode not supported\n");
>>>>> - return ret;
>>>>> - }
>>>>> nor->flash_read = SPI_NOR_QUAD;
>>>>
>>>> So from here on, any user will expect quad mode to be correctly set, but
>>>> it really isn't. Is that OK ?
>>>
>>> It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;
>>>
>>
>> But that's invoked later, so whoever adds code between this place and
>> the invocation of spi_nor_init() will be misled into believing the SPI
>> NOR is set up in quad-mode, while it would not be, right ?
>>
>
> I made this change based on previous comments from Cyrlle. All
> spi-nor based settings that need commands to the flash happens in one
> function now. The code before calling spi_nor_init is setting the
> configuration in the nor structure. I don't see how its very different
> from before. I did separate each setting in functions without changing
> the flow in spi_nor_scan() before and was to told to change it this
> way to be more optimal. I don't see how anyone will have any
> confusion. So please let me know how exactly you want it.
Hmmmm, I wonder if nor->flash_read = SPI_NOR_QUAD; shouldn't be set in
set_quad_mode() ?
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
2017-02-14 15:32 ` [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function Kamal Dasu
[not found] ` <1487086368-4118-2-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-02-20 8:12 ` Rafał Miłecki
2017-02-20 8:15 ` Marek Vasut
1 sibling, 1 reply; 12+ messages in thread
From: Rafał Miłecki @ 2017-02-20 8:12 UTC (permalink / raw)
To: Kamal Dasu
Cc: Marek Vasut, Florian Fainelli, linux-spi, Mark Brown,
bcm-kernel-feedback-list, linux-mtd, Cyrille Pitchen
On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> Refactored spi_nor_scan() code to add spi_nor_init() function that
> programs the spi-nor flash to:
> 1) remove flash protection if applicable
> 2) set read mode to quad mode if configured such
> 3) set the address width based on the flash size and vendor
>
> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
Ideally a short reason info should go there.
"This allows reusing spi_nor_init during resume"
or whatever :)
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
2017-02-20 8:12 ` Rafał Miłecki
@ 2017-02-20 8:15 ` Marek Vasut
0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2017-02-20 8:15 UTC (permalink / raw)
To: Rafał Miłecki, Kamal Dasu
Cc: Florian Fainelli, linux-spi, Mark Brown,
bcm-kernel-feedback-list, linux-mtd, Cyrille Pitchen
On 02/20/2017 09:12 AM, Rafał Miłecki wrote:
> On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>> programs the spi-nor flash to:
>> 1) remove flash protection if applicable
>> 2) set read mode to quad mode if configured such
>> 3) set the address width based on the flash size and vendor
>>
>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>
>> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
>
> Ideally a short reason info should go there.
> "This allows reusing spi_nor_init during resume"
> or whatever :)
>
It reinits the SPI NOR (if I understand it correctly). Being able to use
it on some boards after resume is a side-effect, so it should be listed
as such.
--
Best regards,
Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-20 8:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-14 15:32 [PATCH v4 0/2] Added support for spi-nor device pm in m25p80 Kamal Dasu
[not found] ` <1487086368-4118-1-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-14 15:32 ` [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function Kamal Dasu
[not found] ` <1487086368-4118-2-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-18 22:29 ` Marek Vasut
[not found] ` <b6079dbb-82fc-7579-32b2-b8c071ef02ef-ynQEQJNshbs@public.gmane.org>
2017-02-19 9:47 ` Kamal Dasu
[not found] ` <CAKekbesKKS_s873D=xy2tQcVYei_26gYipn2ru1rVY4ihXbsXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-19 10:37 ` Marek Vasut
[not found] ` <54393706-cd93-603a-193b-25fbf52b2d83-ynQEQJNshbs@public.gmane.org>
2017-02-19 22:38 ` Kamal Dasu
[not found] ` <CAKekbetnpY=zRJM0=xn5t2enDcts5tQTV6ZtvPaW_geOd3=DQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-19 22:51 ` Marek Vasut
2017-02-20 8:12 ` Rafał Miłecki
2017-02-20 8:15 ` Marek Vasut
2017-02-14 15:32 ` [PATCH v4 2/2] mtd: m25p80: Added pm ops support Kamal Dasu
[not found] ` <1487086368-4118-3-git-send-email-kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-18 22:31 ` Marek Vasut
[not found] ` <e5fb0bdf-0956-e364-14fb-c255471f371d-ynQEQJNshbs@public.gmane.org>
2017-02-19 9:48 ` Kamal Dasu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).