* [RFC PATCH v2 0/2] Add support to Disable the flash quad mode
@ 2020-05-12 11:26 Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable " Yicong Yang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yicong Yang @ 2020-05-12 11:26 UTC (permalink / raw)
To: tudor.ambarus, linux-mtd
Cc: vigneshr, sergei.shtylyov, richard, me, john.garry, linuxarm,
yangyicong, alexander.sverdlin, miquel.raynal
Previously we didn't disable the flash's quad mode when it's removed
Then comes the problem that if we next time load the flash
in SPI/Dual mode, it will not be correctly enabled as the quad enable
bits has not been cleared. I validated the condition on Cypress s25fs128s1.
This series adds the capability to disable the flash's quad mode. And
restore the flash when it's removed in spi_nor_restore().
RFC:
- I cleared quad mode bit in spi_nor_restore() when flash removed or
shutdown, follow what reset 4byte address does. But I don't know if
there is a situation that the flash is in quad mode by default. If so,
we should disable it in probe stage.
- The issue occurs when the user switch the spi mode of the flash.
Not sure whether a situation exists.
Changes since v1:
- modify the comments and fix the condition suggested by Pratyush's.
Link: https://lore.kernel.org/linux-mtd/1587720044-49172-1-git-send-email-yangyicong@hisilicon.com/
Yicong Yang (2):
mtd: spi-nor: Add capability to disable flash quad mode
mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
drivers/mtd/spi-nor/core.c | 56 +++++++++++++++++++++++++++++++++-------------
drivers/mtd/spi-nor/core.h | 8 +++----
2 files changed, 44 insertions(+), 20 deletions(-)
--
2.8.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable flash quad mode
2020-05-12 11:26 [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
@ 2020-05-12 11:26 ` Yicong Yang
2020-06-15 10:53 ` Pratyush Yadav
2020-05-12 11:26 ` [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore() Yicong Yang
2020-06-15 9:19 ` [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
2 siblings, 1 reply; 7+ messages in thread
From: Yicong Yang @ 2020-05-12 11:26 UTC (permalink / raw)
To: tudor.ambarus, linux-mtd
Cc: vigneshr, sergei.shtylyov, richard, me, john.garry, linuxarm,
yangyicong, alexander.sverdlin, miquel.raynal
Previous we didn't provide a way to disable the flash's quad mode.
Which means we cannot do some cleanup works when to remove or
poweroff the flash, like what set 4-byte address mode does in
spi_nor_restore().
Add the capability to disable the flash quad mode, by introducing
an enable flag in the flash parameters quad_enable() hooks and
related functions.
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
drivers/mtd/spi-nor/core.c | 53 ++++++++++++++++++++++++++++++++--------------
drivers/mtd/spi-nor/core.h | 8 +++----
2 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cc68ea8..72e8d8b 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1907,15 +1907,17 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
}
/**
- * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
+ * spi_nor_sr1_bit6_quad_enable() - Set/Unset the Quad Enable BIT(6) in the
+ * Status
* Register 1.
* @nor: pointer to a 'struct spi_nor'
+ * @enable: true to enter quad mode. false to leave quad mode.
*
* Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
*
* Return: 0 on success, -errno otherwise.
*/
-int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
+int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor, bool enable)
{
int ret;
@@ -1923,45 +1925,59 @@ int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
if (ret)
return ret;
- if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
+ if ((enable && (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)) ||
+ !(enable || (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)))
return 0;
- nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
+ if (enable)
+ nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
+ else
+ nor->bouncebuf[0] &= ~SR1_QUAD_EN_BIT6;
return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
}
/**
- * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
+ * spi_nor_sr2_bit1_quad_enable() - set/unset the Quad Enable BIT(1) in the
+ * Status
* Register 2.
* @nor: pointer to a 'struct spi_nor'.
+ * @enable: true to enter quad mode. false to leave quad mode.
*
* Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
*
* Return: 0 on success, -errno otherwise.
*/
-int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
+int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor, bool enable)
{
int ret;
if (nor->flags & SNOR_F_NO_READ_CR)
- return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
+ return spi_nor_write_16bit_cr_and_check(nor,
+ enable ? SR2_QUAD_EN_BIT1 : 0);
ret = spi_nor_read_cr(nor, nor->bouncebuf);
if (ret)
return ret;
- if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
+ if ((enable && (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)) ||
+ !(enable || (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)))
return 0;
nor->bouncebuf[0] |= SR2_QUAD_EN_BIT1;
+ if (enable)
+ nor->bouncebuf[0] |= SR2_QUAD_EN_BIT1;
+ else
+ nor->bouncebuf[0] &= ~SR2_QUAD_EN_BIT1;
+
return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
}
/**
- * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
+ * spi_nor_sr2_bit7_quad_enable() - set/unset QE bit in Status Register 2.
* @nor: pointer to a 'struct spi_nor'
+ * @enable: true to enter quad mode. false to leave quad mode.
*
* Set the Quad Enable (QE) bit in the Status Register 2.
*
@@ -1971,7 +1987,7 @@ int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
*
* Return: 0 on success, -errno otherwise.
*/
-int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
+int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor, bool enable)
{
u8 *sr2 = nor->bouncebuf;
int ret;
@@ -1981,11 +1997,15 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
ret = spi_nor_read_sr2(nor, sr2);
if (ret)
return ret;
- if (*sr2 & SR2_QUAD_EN_BIT7)
+ if ((enable && (*sr2 & SR2_QUAD_EN_BIT7)) ||
+ !(enable || (*sr2 & SR2_QUAD_EN_BIT7)))
return 0;
/* Update the Quad Enable bit. */
- *sr2 |= SR2_QUAD_EN_BIT7;
+ if (enable)
+ *sr2 |= SR2_QUAD_EN_BIT7;
+ else
+ *sr2 &= ~SR2_QUAD_EN_BIT7;
ret = spi_nor_write_sr2(nor, sr2);
if (ret)
@@ -2898,12 +2918,13 @@ static int spi_nor_init_params(struct spi_nor *nor)
}
/**
- * spi_nor_quad_enable() - enable Quad I/O if needed.
+ * spi_nor_quad_enable() - enable/disable Quad I/O if needed.
* @nor: pointer to a 'struct spi_nor'
+ * @enable: true to enable quad mode. false to disable.
*
* Return: 0 on success, -errno otherwise.
*/
-static int spi_nor_quad_enable(struct spi_nor *nor)
+static int spi_nor_quad_enable(struct spi_nor *nor, bool enable)
{
if (!nor->params->quad_enable)
return 0;
@@ -2912,7 +2933,7 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
spi_nor_get_protocol_width(nor->write_proto) == 4))
return 0;
- return nor->params->quad_enable(nor);
+ return nor->params->quad_enable(nor, enable);
}
/**
@@ -2936,7 +2957,7 @@ static int spi_nor_init(struct spi_nor *nor)
{
int err;
- err = spi_nor_quad_enable(nor);
+ err = spi_nor_quad_enable(nor, true);
if (err) {
dev_dbg(nor->dev, "quad mode not supported\n");
return err;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 6f2f6b2..719a31d 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -219,7 +219,7 @@ struct spi_nor_flash_parameter {
struct spi_nor_erase_map erase_map;
- int (*quad_enable)(struct spi_nor *nor);
+ int (*quad_enable)(struct spi_nor *nor, bool enable);
int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
@@ -406,9 +406,9 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
int spi_nor_wait_till_ready(struct spi_nor *nor);
int spi_nor_lock_and_prep(struct spi_nor *nor);
void spi_nor_unlock_and_unprep(struct spi_nor *nor);
-int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
-int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
-int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
+int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor, bool enable);
+int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor, bool enable);
+int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor, bool enable);
int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr);
ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
--
2.8.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
2020-05-12 11:26 [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable " Yicong Yang
@ 2020-05-12 11:26 ` Yicong Yang
2020-06-15 10:56 ` Pratyush Yadav
2020-06-15 9:19 ` [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
2 siblings, 1 reply; 7+ messages in thread
From: Yicong Yang @ 2020-05-12 11:26 UTC (permalink / raw)
To: tudor.ambarus, linux-mtd
Cc: vigneshr, sergei.shtylyov, richard, me, john.garry, linuxarm,
yangyicong, alexander.sverdlin, miquel.raynal
If the flash's quad mode is enabled, it'll remain in the quad mode when
it's removed. If we drive the flash next time in SPI/Dual mode, then
problem occurs as the flash's quad enable bit is not cleared.
Disable the quad mode in spi_nor_restore(), the flash will leave
quad mode when remove. This will make sure the flash always enter the
correct mode when loaded.
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
drivers/mtd/spi-nor/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 72e8d8b..564de02 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3004,6 +3004,9 @@ void spi_nor_restore(struct spi_nor *nor)
if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
nor->flags & SNOR_F_BROKEN_RESET)
nor->params->set_4byte_addr_mode(nor, false);
+
+ /* disable quad mode */
+ spi_nor_quad_enable(nor, false);
}
EXPORT_SYMBOL_GPL(spi_nor_restore);
--
2.8.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 0/2] Add support to Disable the flash quad mode
2020-05-12 11:26 [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable " Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore() Yicong Yang
@ 2020-06-15 9:19 ` Yicong Yang
2 siblings, 0 replies; 7+ messages in thread
From: Yicong Yang @ 2020-06-15 9:19 UTC (permalink / raw)
To: tudor.ambarus, linux-mtd
Cc: vigneshr, sergei.shtylyov, richard, me, john.garry, linuxarm,
alexander.sverdlin, miquel.raynal
a friendly ping...
any comments on this?
Thanks.
On 2020/5/12 19:26, Yicong Yang wrote:
> Previously we didn't disable the flash's quad mode when it's removed
> Then comes the problem that if we next time load the flash
> in SPI/Dual mode, it will not be correctly enabled as the quad enable
> bits has not been cleared. I validated the condition on Cypress s25fs128s1.
>
> This series adds the capability to disable the flash's quad mode. And
> restore the flash when it's removed in spi_nor_restore().
>
> RFC:
> - I cleared quad mode bit in spi_nor_restore() when flash removed or
> shutdown, follow what reset 4byte address does. But I don't know if
> there is a situation that the flash is in quad mode by default. If so,
> we should disable it in probe stage.
> - The issue occurs when the user switch the spi mode of the flash.
> Not sure whether a situation exists.
>
> Changes since v1:
> - modify the comments and fix the condition suggested by Pratyush's.
> Link: https://lore.kernel.org/linux-mtd/1587720044-49172-1-git-send-email-yangyicong@hisilicon.com/
>
> Yicong Yang (2):
> mtd: spi-nor: Add capability to disable flash quad mode
> mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
>
> drivers/mtd/spi-nor/core.c | 56 +++++++++++++++++++++++++++++++++-------------
> drivers/mtd/spi-nor/core.h | 8 +++----
> 2 files changed, 44 insertions(+), 20 deletions(-)
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable flash quad mode
2020-05-12 11:26 ` [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable " Yicong Yang
@ 2020-06-15 10:53 ` Pratyush Yadav
2020-06-16 7:10 ` Yicong Yang
0 siblings, 1 reply; 7+ messages in thread
From: Pratyush Yadav @ 2020-06-15 10:53 UTC (permalink / raw)
To: Yicong Yang
Cc: vigneshr, sergei.shtylyov, tudor.ambarus, richard, me,
john.garry, linuxarm, linux-mtd, miquel.raynal,
alexander.sverdlin
Hi Yicong,
You generally shouldn't mark a series as "RFC" if you intend it to get
merged in.
On 12/05/20 07:26PM, Yicong Yang wrote:
> Previous we didn't provide a way to disable the flash's quad mode.
> Which means we cannot do some cleanup works when to remove or
> poweroff the flash, like what set 4-byte address mode does in
> spi_nor_restore().
>
> Add the capability to disable the flash quad mode, by introducing
> an enable flag in the flash parameters quad_enable() hooks and
> related functions.
>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> drivers/mtd/spi-nor/core.c | 53 ++++++++++++++++++++++++++++++++--------------
> drivers/mtd/spi-nor/core.h | 8 +++----
> 2 files changed, 41 insertions(+), 20 deletions(-)
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
Nits below.
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cc68ea8..72e8d8b 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1907,15 +1907,17 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> }
>
> /**
> - * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
> + * spi_nor_sr1_bit6_quad_enable() - Set/Unset the Quad Enable BIT(6) in the
> + * Status
> * Register 1.
The "Register 1" should be on the same line as the "Status above".
> * @nor: pointer to a 'struct spi_nor'
> + * @enable: true to enter quad mode. false to leave quad mode.
> *
> * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
> +int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor, bool enable)
> {
> int ret;
>
> @@ -1923,45 +1925,59 @@ int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
> if (ret)
> return ret;
>
> - if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
> + if ((enable && (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)) ||
> + !(enable || (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)))
I still think writing it as:
(!enable && !(nor->bouncebuf[0] & SR1_QUAD_EN_BIT6))
is slightly more readable. But maybe it's just me so this is OK I guess.
> return 0;
>
> - nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
> + if (enable)
> + nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
> + else
> + nor->bouncebuf[0] &= ~SR1_QUAD_EN_BIT6;
>
> return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
> }
>
> /**
> - * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
> + * spi_nor_sr2_bit1_quad_enable() - set/unset the Quad Enable BIT(1) in the
> + * Status
> * Register 2.
The "Register 2" should be on the same line as the "Status above".
> * @nor: pointer to a 'struct spi_nor'.
> + * @enable: true to enter quad mode. false to leave quad mode.
> *
> * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
> +int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor, bool enable)
> {
> int ret;
[...]
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 6f2f6b2..719a31d 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -219,7 +219,7 @@ struct spi_nor_flash_parameter {
>
> struct spi_nor_erase_map erase_map;
>
> - int (*quad_enable)(struct spi_nor *nor);
> + int (*quad_enable)(struct spi_nor *nor, bool enable);
Update the comment above reflecting that @quad_enable "enables/disables
SPI NOR quad mode".
> int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
> u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
> int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
2020-05-12 11:26 ` [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore() Yicong Yang
@ 2020-06-15 10:56 ` Pratyush Yadav
0 siblings, 0 replies; 7+ messages in thread
From: Pratyush Yadav @ 2020-06-15 10:56 UTC (permalink / raw)
To: Yicong Yang
Cc: vigneshr, sergei.shtylyov, tudor.ambarus, richard, me,
john.garry, linuxarm, linux-mtd, miquel.raynal,
alexander.sverdlin
Hi Yicong,
On 12/05/20 07:26PM, Yicong Yang wrote:
> If the flash's quad mode is enabled, it'll remain in the quad mode when
> it's removed. If we drive the flash next time in SPI/Dual mode, then
> problem occurs as the flash's quad enable bit is not cleared.
>
> Disable the quad mode in spi_nor_restore(), the flash will leave
> quad mode when remove. This will make sure the flash always enter the
> correct mode when loaded.
>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> drivers/mtd/spi-nor/core.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
Small nitpick below.
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 72e8d8b..564de02 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3004,6 +3004,9 @@ void spi_nor_restore(struct spi_nor *nor)
> if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
> nor->flags & SNOR_F_BROKEN_RESET)
> nor->params->set_4byte_addr_mode(nor, false);
> +
> + /* disable quad mode */
I don't think this comment is needed. IMO the line below makes it pretty
clear that we want to disable quad mode.
> + spi_nor_quad_enable(nor, false);
> }
> EXPORT_SYMBOL_GPL(spi_nor_restore);
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable flash quad mode
2020-06-15 10:53 ` Pratyush Yadav
@ 2020-06-16 7:10 ` Yicong Yang
0 siblings, 0 replies; 7+ messages in thread
From: Yicong Yang @ 2020-06-16 7:10 UTC (permalink / raw)
To: Pratyush Yadav
Cc: vigneshr, sergei.shtylyov, tudor.ambarus, richard, me,
john.garry, linuxarm, linux-mtd, miquel.raynal,
alexander.sverdlin
Hi Pratyush,
Thanks for your review. I'll modify Patch 1 and 2 and
send a formal serie.
Regards,
Yicong
On 2020/6/15 18:53, Pratyush Yadav wrote:
> Hi Yicong,
>
> You generally shouldn't mark a series as "RFC" if you intend it to get
> merged in.
>
> On 12/05/20 07:26PM, Yicong Yang wrote:
>> Previous we didn't provide a way to disable the flash's quad mode.
>> Which means we cannot do some cleanup works when to remove or
>> poweroff the flash, like what set 4-byte address mode does in
>> spi_nor_restore().
>>
>> Add the capability to disable the flash quad mode, by introducing
>> an enable flag in the flash parameters quad_enable() hooks and
>> related functions.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>> drivers/mtd/spi-nor/core.c | 53 ++++++++++++++++++++++++++++++++--------------
>> drivers/mtd/spi-nor/core.h | 8 +++----
>> 2 files changed, 41 insertions(+), 20 deletions(-)
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> Nits below.
>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index cc68ea8..72e8d8b 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1907,15 +1907,17 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>> }
>>
>> /**
>> - * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
>> + * spi_nor_sr1_bit6_quad_enable() - Set/Unset the Quad Enable BIT(6) in the
>> + * Status
>> * Register 1.
> The "Register 1" should be on the same line as the "Status above".
>
>> * @nor: pointer to a 'struct spi_nor'
>> + * @enable: true to enter quad mode. false to leave quad mode.
>> *
>> * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
>> *
>> * Return: 0 on success, -errno otherwise.
>> */
>> -int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
>> +int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor, bool enable)
>> {
>> int ret;
>>
>> @@ -1923,45 +1925,59 @@ int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
>> if (ret)
>> return ret;
>>
>> - if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
>> + if ((enable && (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)) ||
>> + !(enable || (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)))
> I still think writing it as:
>
> (!enable && !(nor->bouncebuf[0] & SR1_QUAD_EN_BIT6))
>
> is slightly more readable. But maybe it's just me so this is OK I guess.
>
>> return 0;
>>
>> - nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
>> + if (enable)
>> + nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
>> + else
>> + nor->bouncebuf[0] &= ~SR1_QUAD_EN_BIT6;
>>
>> return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
>> }
>>
>> /**
>> - * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
>> + * spi_nor_sr2_bit1_quad_enable() - set/unset the Quad Enable BIT(1) in the
>> + * Status
>> * Register 2.
> The "Register 2" should be on the same line as the "Status above".
>
>> * @nor: pointer to a 'struct spi_nor'.
>> + * @enable: true to enter quad mode. false to leave quad mode.
>> *
>> * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
>> *
>> * Return: 0 on success, -errno otherwise.
>> */
>> -int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
>> +int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor, bool enable)
>> {
>> int ret;
> [...]
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 6f2f6b2..719a31d 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -219,7 +219,7 @@ struct spi_nor_flash_parameter {
>>
>> struct spi_nor_erase_map erase_map;
>>
>> - int (*quad_enable)(struct spi_nor *nor);
>> + int (*quad_enable)(struct spi_nor *nor, bool enable);
> Update the comment above reflecting that @quad_enable "enables/disables
> SPI NOR quad mode".
>
>> int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
>> u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
>> int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-16 7:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12 11:26 [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 1/2] mtd: spi-nor: Add capability to disable " Yicong Yang
2020-06-15 10:53 ` Pratyush Yadav
2020-06-16 7:10 ` Yicong Yang
2020-05-12 11:26 ` [RFC PATCH v2 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore() Yicong Yang
2020-06-15 10:56 ` Pratyush Yadav
2020-06-15 9:19 ` [RFC PATCH v2 0/2] Add support to Disable the flash quad mode Yicong Yang
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).