* [PATCH v1 01/14] mtd: spi-nor: export more function to be used in vendor modules
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:13 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup() Michael Walle
` (14 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
We will move vendor specific code into the vendor modules and thus we
will have to export these functions so they can be called.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 10 +++++-----
drivers/mtd/spi-nor/core.h | 6 ++++++
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 04ea180118e3..f05ece6018dc 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -157,8 +157,8 @@ static int spi_nor_spimem_exec_op(struct spi_nor *nor, struct spi_mem_op *op)
return spi_mem_exec_op(nor->spimem, op);
}
-static int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
- u8 *buf, size_t len)
+int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
+ u8 *buf, size_t len)
{
if (spi_nor_protocol_is_dtr(nor->reg_proto))
return -EOPNOTSUPP;
@@ -166,8 +166,8 @@ static int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
return nor->controller_ops->read_reg(nor, opcode, buf, len);
}
-static int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
- const u8 *buf, size_t len)
+int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
+ const u8 *buf, size_t len)
{
if (spi_nor_protocol_is_dtr(nor->reg_proto))
return -EOPNOTSUPP;
@@ -683,7 +683,7 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
*
* Return: 1 if ready, 0 if not ready, -errno on errors.
*/
-static int spi_nor_sr_ready(struct spi_nor *nor)
+int spi_nor_sr_ready(struct spi_nor *nor)
{
int ret = spi_nor_read_sr(nor, nor->bouncebuf);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 2afb610853a9..c6578d3f598b 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -554,6 +554,7 @@ 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_read_sr(struct spi_nor *nor, u8 *sr);
+int spi_nor_sr_ready(struct spi_nor *nor);
int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
@@ -599,6 +600,11 @@ void spi_nor_try_unlock_all(struct spi_nor *nor);
void spi_nor_set_mtd_locking_ops(struct spi_nor *nor);
void spi_nor_set_mtd_otp_ops(struct spi_nor *nor);
+int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
+ u8 *buf, size_t len);
+int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
+ const u8 *buf, size_t len);
+
static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return container_of(mtd, struct spi_nor, mtd);
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 01/14] mtd: spi-nor: export more function to be used in vendor modules
2022-02-02 14:58 ` [PATCH v1 01/14] mtd: spi-nor: export more function to be used in " Michael Walle
@ 2022-02-10 3:13 ` Tudor.Ambarus
2022-02-15 18:30 ` Pratyush Yadav
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:13 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> We will move vendor specific code into the vendor modules and thus we
> will have to export these functions so they can be called.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
please move this patch closer to where the vendors actually use the methods.
With that:
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 10 +++++-----
> drivers/mtd/spi-nor/core.h | 6 ++++++
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 04ea180118e3..f05ece6018dc 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -157,8 +157,8 @@ static int spi_nor_spimem_exec_op(struct spi_nor *nor, struct spi_mem_op *op)
> return spi_mem_exec_op(nor->spimem, op);
> }
>
> -static int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
> - u8 *buf, size_t len)
> +int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, size_t len)
> {
> if (spi_nor_protocol_is_dtr(nor->reg_proto))
> return -EOPNOTSUPP;
> @@ -166,8 +166,8 @@ static int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
> return nor->controller_ops->read_reg(nor, opcode, buf, len);
> }
>
> -static int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
> - const u8 *buf, size_t len)
> +int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
> + const u8 *buf, size_t len)
> {
> if (spi_nor_protocol_is_dtr(nor->reg_proto))
> return -EOPNOTSUPP;
> @@ -683,7 +683,7 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
> *
> * Return: 1 if ready, 0 if not ready, -errno on errors.
> */
> -static int spi_nor_sr_ready(struct spi_nor *nor)
> +int spi_nor_sr_ready(struct spi_nor *nor)
> {
> int ret = spi_nor_read_sr(nor, nor->bouncebuf);
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 2afb610853a9..c6578d3f598b 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -554,6 +554,7 @@ 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_read_sr(struct spi_nor *nor, u8 *sr);
> +int spi_nor_sr_ready(struct spi_nor *nor);
> int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
> int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
> int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
> @@ -599,6 +600,11 @@ void spi_nor_try_unlock_all(struct spi_nor *nor);
> void spi_nor_set_mtd_locking_ops(struct spi_nor *nor);
> void spi_nor_set_mtd_otp_ops(struct spi_nor *nor);
>
> +int spi_nor_controller_ops_read_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, size_t len);
> +int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
> + const u8 *buf, size_t len);
> +
> static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
> {
> return container_of(mtd, struct spi_nor, mtd);
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 01/14] mtd: spi-nor: export more function to be used in vendor modules
2022-02-10 3:13 ` Tudor.Ambarus
@ 2022-02-15 18:30 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 18:30 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:13AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > We will move vendor specific code into the vendor modules and thus we
> > will have to export these functions so they can be called.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
>
> please move this patch closer to where the vendors actually use the methods.
> With that:
>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup()
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
2022-02-02 14:58 ` [PATCH v1 01/14] mtd: spi-nor: export more function to be used in " Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:00 ` Tudor.Ambarus
2022-02-15 18:32 ` Pratyush Yadav
2022-02-02 14:58 ` [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function Michael Walle
` (13 subsequent siblings)
15 siblings, 2 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Instead of always using a function pointer (and initializing it to our
default), just call the default function if the flash didn't set its own
one. That will make the call flow easier to follow.
Also mark the parameter as optional now.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 10 +++++-----
drivers/mtd/spi-nor/core.h | 8 ++++----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f05ece6018dc..c8cc906cf771 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2532,11 +2532,12 @@ static int spi_nor_setup(struct spi_nor *nor,
{
int ret;
- if (nor->params->setup) {
+ if (nor->params->setup)
ret = nor->params->setup(nor, hwcaps);
- if (ret)
- return ret;
- }
+ else
+ ret = spi_nor_default_setup(nor, hwcaps);
+ if (ret)
+ return ret;
return spi_nor_set_addr_width(nor);
}
@@ -2786,7 +2787,6 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
params->quad_enable = spi_nor_sr2_bit1_quad_enable;
params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
- params->setup = spi_nor_default_setup;
params->otp.org = &info->otp_org;
/* Default to 16-bit Write Status (01h) Command */
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index c6578d3f598b..10f478547dc2 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -257,10 +257,10 @@ struct spi_nor_otp {
* @convert_addr: converts an absolute address into something the flash
* will understand. Particularly useful when pagesize is
* not a power-of-2.
- * @setup: configures the SPI NOR memory. Useful for SPI NOR
- * flashes that have peculiarities to the SPI NOR standard
- * e.g. different opcodes, specific address calculation,
- * page size, etc.
+ * @setup: (optional) configures the SPI NOR memory. Useful for
+ * SPI NOR flashes that have peculiarities to the SPI NOR
+ * standard e.g. different opcodes, specific address
+ * calculation, page size, etc.
* @locking_ops: SPI NOR locking methods.
*/
struct spi_nor_flash_parameter {
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup()
2022-02-02 14:58 ` [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup() Michael Walle
@ 2022-02-10 3:00 ` Tudor.Ambarus
2022-02-10 8:01 ` Michael Walle
2022-02-15 18:32 ` Pratyush Yadav
1 sibling, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:00 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Instead of always using a function pointer (and initializing it to our
> default), just call the default function if the flash didn't set its own
> one. That will make the call flow easier to follow.
>
> Also mark the parameter as optional now.
what should be done in the future?
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 10 +++++-----
> drivers/mtd/spi-nor/core.h | 8 ++++----
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f05ece6018dc..c8cc906cf771 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2532,11 +2532,12 @@ static int spi_nor_setup(struct spi_nor *nor,
> {
> int ret;
>
> - if (nor->params->setup) {
> + if (nor->params->setup)
> ret = nor->params->setup(nor, hwcaps);
> - if (ret)
> - return ret;
> - }
> + else
> + ret = spi_nor_default_setup(nor, hwcaps);
> + if (ret)
> + return ret;
>
> return spi_nor_set_addr_width(nor);
> }
> @@ -2786,7 +2787,6 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
>
> params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
> - params->setup = spi_nor_default_setup;
> params->otp.org = &info->otp_org;
>
> /* Default to 16-bit Write Status (01h) Command */
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index c6578d3f598b..10f478547dc2 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -257,10 +257,10 @@ struct spi_nor_otp {
> * @convert_addr: converts an absolute address into something the flash
> * will understand. Particularly useful when pagesize is
> * not a power-of-2.
> - * @setup: configures the SPI NOR memory. Useful for SPI NOR
> - * flashes that have peculiarities to the SPI NOR standard
> - * e.g. different opcodes, specific address calculation,
> - * page size, etc.
> + * @setup: (optional) configures the SPI NOR memory. Useful for
> + * SPI NOR flashes that have peculiarities to the SPI NOR
> + * standard e.g. different opcodes, specific address
> + * calculation, page size, etc.
> * @locking_ops: SPI NOR locking methods.
> */
> struct spi_nor_flash_parameter {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup()
2022-02-10 3:00 ` Tudor.Ambarus
@ 2022-02-10 8:01 ` Michael Walle
2022-02-10 8:05 ` Tudor.Ambarus
0 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-10 8:01 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
Am 2022-02-10 04:00, schrieb Tudor.Ambarus@microchip.com:
> On 2/2/22 16:58, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Instead of always using a function pointer (and initializing it to our
>> default), just call the default function if the flash didn't set its
>> own
>> one. That will make the call flow easier to follow.
>>
>> Also mark the parameter as optional now.
>
> what should be done in the future?
What do you mean? IMHO the default path should be visible in the
function. Eg.
int spi_nor_bla(nor) {
if (nor->some_exceptional_cb)
return nor->some_exceptional_cb(nor);
return usual_cb(nor);
}
-michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup()
2022-02-10 8:01 ` Michael Walle
@ 2022-02-10 8:05 ` Tudor.Ambarus
0 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 8:05 UTC (permalink / raw)
To: michael
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
On 2/10/22 10:01, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am 2022-02-10 04:00, schrieb Tudor.Ambarus@microchip.com:
>> On 2/2/22 16:58, Michael Walle wrote:
cut
>>> Also mark the parameter as optional now.
Oh, I misread, in my head I read "for now". That's why I asked what should
be done next :).
>>
>> what should be done in the future?
>
> What do you mean? IMHO the default path should be visible in the
> function. Eg.
>
> int spi_nor_bla(nor) {
> if (nor->some_exceptional_cb)
> return nor->some_exceptional_cb(nor);
>
> return usual_cb(nor);
> }
>
> -michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup()
2022-02-02 14:58 ` [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup() Michael Walle
2022-02-10 3:00 ` Tudor.Ambarus
@ 2022-02-15 18:32 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 18:32 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> Instead of always using a function pointer (and initializing it to our
> default), just call the default function if the flash didn't set its own
> one. That will make the call flow easier to follow.
>
> Also mark the parameter as optional now.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
2022-02-02 14:58 ` [PATCH v1 01/14] mtd: spi-nor: export more function to be used in " Michael Walle
2022-02-02 14:58 ` [PATCH v1 02/14] mtd: spi-nor: slightly refactor the spi_nor_setup() Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:05 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c Michael Walle
` (12 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Xilinx and Micron flashes have their own implementation of the
spi_nor_ready() function. At the moment, the core will figure out
which one to call according to some flags. Lay the foundation to
make it possible that a flash can register its own ready()
function.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 4 ++++
drivers/mtd/spi-nor/core.h | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c8cc906cf771..c181f2680df2 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -794,6 +794,10 @@ static int spi_nor_ready(struct spi_nor *nor)
{
int sr, fsr;
+ /* flashes might override our standard routine */
+ if (nor->params->ready)
+ return nor->params->ready(nor);
+
if (nor->flags & SNOR_F_READY_XSR_RDY)
sr = spi_nor_xsr_ready(nor);
else
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 10f478547dc2..446218b0e017 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -261,6 +261,9 @@ struct spi_nor_otp {
* SPI NOR flashes that have peculiarities to the SPI NOR
* standard e.g. different opcodes, specific address
* calculation, page size, etc.
+ * @ready: (optional) flashes might use a different mechanism
+ * than reading the status register to indicate they
+ * are ready for a new command
* @locking_ops: SPI NOR locking methods.
*/
struct spi_nor_flash_parameter {
@@ -282,6 +285,7 @@ struct spi_nor_flash_parameter {
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);
+ int (*ready)(struct spi_nor *nor);
const struct spi_nor_locking_ops *locking_ops;
};
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function
2022-02-02 14:58 ` [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function Michael Walle
@ 2022-02-10 3:05 ` Tudor.Ambarus
2022-02-15 18:36 ` Pratyush Yadav
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:05 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Xilinx and Micron flashes have their own implementation of the
> spi_nor_ready() function. At the moment, the core will figure out
> which one to call according to some flags. Lay the foundation to
> make it possible that a flash can register its own ready()
> function.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 4 ++++
> drivers/mtd/spi-nor/core.h | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index c8cc906cf771..c181f2680df2 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -794,6 +794,10 @@ static int spi_nor_ready(struct spi_nor *nor)
> {
> int sr, fsr;
>
> + /* flashes might override our standard routine */
let's start comments with capital letter and put a dot at the end of
the sentence. s/our/the
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> + if (nor->params->ready)
> + return nor->params->ready(nor);
> +
> if (nor->flags & SNOR_F_READY_XSR_RDY)
> sr = spi_nor_xsr_ready(nor);
> else
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 10f478547dc2..446218b0e017 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -261,6 +261,9 @@ struct spi_nor_otp {
> * SPI NOR flashes that have peculiarities to the SPI NOR
> * standard e.g. different opcodes, specific address
> * calculation, page size, etc.
> + * @ready: (optional) flashes might use a different mechanism
> + * than reading the status register to indicate they
> + * are ready for a new command
> * @locking_ops: SPI NOR locking methods.
> */
> struct spi_nor_flash_parameter {
> @@ -282,6 +285,7 @@ struct spi_nor_flash_parameter {
> 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);
> + int (*ready)(struct spi_nor *nor);
>
> const struct spi_nor_locking_ops *locking_ops;
> };
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function
2022-02-10 3:05 ` Tudor.Ambarus
@ 2022-02-15 18:36 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 18:36 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:05AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Xilinx and Micron flashes have their own implementation of the
> > spi_nor_ready() function. At the moment, the core will figure out
> > which one to call according to some flags. Lay the foundation to
> > make it possible that a flash can register its own ready()
> > function.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > drivers/mtd/spi-nor/core.c | 4 ++++
> > drivers/mtd/spi-nor/core.h | 4 ++++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index c8cc906cf771..c181f2680df2 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -794,6 +794,10 @@ static int spi_nor_ready(struct spi_nor *nor)
> > {
> > int sr, fsr;
> >
> > + /* flashes might override our standard routine */
>
> let's start comments with capital letter and put a dot at the end of
> the sentence. s/our/the
+1
>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
With that fixed,
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> > + if (nor->params->ready)
> > + return nor->params->ready(nor);
> > +
> > if (nor->flags & SNOR_F_READY_XSR_RDY)
> > sr = spi_nor_xsr_ready(nor);
> > else
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index 10f478547dc2..446218b0e017 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -261,6 +261,9 @@ struct spi_nor_otp {
> > * SPI NOR flashes that have peculiarities to the SPI NOR
> > * standard e.g. different opcodes, specific address
> > * calculation, page size, etc.
> > + * @ready: (optional) flashes might use a different mechanism
> > + * than reading the status register to indicate they
> > + * are ready for a new command
> > * @locking_ops: SPI NOR locking methods.
> > */
> > struct spi_nor_flash_parameter {
> > @@ -282,6 +285,7 @@ struct spi_nor_flash_parameter {
> > 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);
> > + int (*ready)(struct spi_nor *nor);
> >
> > const struct spi_nor_locking_ops *locking_ops;
> > };
> > --
> > 2.30.2
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (2 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 03/14] mtd: spi-nor: allow a flash to define its own ready() function Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:04 ` Tudor.Ambarus
2022-02-15 18:57 ` Pratyush Yadav
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
` (11 subsequent siblings)
15 siblings, 2 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Mechanically move all the xilinx functions to its own module.
Then register the new flash specific ready() function.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 64 +------------------------------
drivers/mtd/spi-nor/core.h | 18 ---------
drivers/mtd/spi-nor/xilinx.c | 73 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 9 -----
4 files changed, 74 insertions(+), 90 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c181f2680df2..fdc8ef623254 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -598,57 +598,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
return ret;
}
-/**
- * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
- * @nor: pointer to 'struct spi_nor'.
- * @sr: pointer to a DMA-able buffer where the value of the
- * Status Register will be written.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(1, sr, 0));
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
- 1);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
-
- return ret;
-}
-
-/**
- * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
- * the flash is ready for new commands.
- * @nor: pointer to 'struct spi_nor'.
- *
- * Return: 1 if ready, 0 if not ready, -errno on errors.
- */
-static int spi_nor_xsr_ready(struct spi_nor *nor)
-{
- int ret;
-
- ret = spi_nor_xread_sr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- return !!(nor->bouncebuf[0] & XSR_RDY);
-}
-
/**
* spi_nor_clear_sr() - Clear the Status Register.
* @nor: pointer to 'struct spi_nor'.
@@ -798,10 +747,7 @@ static int spi_nor_ready(struct spi_nor *nor)
if (nor->params->ready)
return nor->params->ready(nor);
- if (nor->flags & SNOR_F_READY_XSR_RDY)
- sr = spi_nor_xsr_ready(nor);
- else
- sr = spi_nor_sr_ready(nor);
+ sr = spi_nor_sr_ready(nor);
if (sr < 0)
return sr;
fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
@@ -2677,14 +2623,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
if (flags & USE_FSR)
nor->flags |= SNOR_F_USE_FSR;
-
- /*
- * Make sure the XSR_RDY flag is set before calling
- * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
- * with Atmel SPI NOR.
- */
- if (flags & SPI_NOR_XSR_RDY)
- nor->flags |= SNOR_F_READY_XSR_RDY;
}
/**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 446218b0e017..fabc01ae9a81 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -15,7 +15,6 @@ enum spi_nor_option_flags {
SNOR_F_USE_FSR = BIT(0),
SNOR_F_HAS_SR_TB = BIT(1),
SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
- SNOR_F_READY_XSR_RDY = BIT(3),
SNOR_F_USE_CLSR = BIT(4),
SNOR_F_BROKEN_RESET = BIT(5),
SNOR_F_4B_OPCODES = BIT(6),
@@ -351,8 +350,6 @@ struct spi_nor_fixups {
* SPI_NOR_NO_FR: can't do fastread.
* USE_CLSR: use CLSR command.
* USE_FSR: use flag status register
- * SPI_NOR_XSR_RDY: S3AN flashes have specific opcode to read the
- * status register.
*
* @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
* Used when SFDP tables are not defined in the flash. These
@@ -405,7 +402,6 @@ struct flash_info {
#define SPI_NOR_NO_FR BIT(8)
#define USE_CLSR BIT(9)
#define USE_FSR BIT(10)
-#define SPI_NOR_XSR_RDY BIT(11)
u8 no_sfdp_flags;
#define SPI_NOR_SKIP_SFDP BIT(0)
@@ -462,19 +458,6 @@ struct flash_info {
.addr_width = (_addr_width), \
.flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR, \
-#define S3AN_INFO(_jedec_id, _n_sectors, _page_size) \
- .id = { \
- ((_jedec_id) >> 16) & 0xff, \
- ((_jedec_id) >> 8) & 0xff, \
- (_jedec_id) & 0xff \
- }, \
- .id_len = 3, \
- .sector_size = (8*_page_size), \
- .n_sectors = (_n_sectors), \
- .page_size = _page_size, \
- .addr_width = 3, \
- .flags = SPI_NOR_NO_FR | SPI_NOR_XSR_RDY,
-
#define OTP_INFO(_len, _n_regions, _base, _offset) \
.otp_org = { \
.len = (_len), \
@@ -564,7 +547,6 @@ int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
-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,
u8 *buf);
ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
index 580562bc1e45..a865dadc2e5d 100644
--- a/drivers/mtd/spi-nor/xilinx.c
+++ b/drivers/mtd/spi-nor/xilinx.c
@@ -8,6 +8,27 @@
#include "core.h"
+#define SPINOR_OP_XSE 0x50 /* Sector erase */
+#define SPINOR_OP_XPP 0x82 /* Page program */
+#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
+
+#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
+#define XSR_RDY BIT(7) /* Ready */
+
+#define S3AN_INFO(_jedec_id, _n_sectors, _page_size) \
+ .id = { \
+ ((_jedec_id) >> 16) & 0xff, \
+ ((_jedec_id) >> 8) & 0xff, \
+ (_jedec_id) & 0xff \
+ }, \
+ .id_len = 3, \
+ .sector_size = (8*_page_size), \
+ .n_sectors = (_n_sectors), \
+ .page_size = _page_size, \
+ .addr_width = 3, \
+ .flags = SPI_NOR_NO_FR
+
+/* Xilinx S3AN share MFR with Atmel SPI NOR */
static const struct flash_info xilinx_parts[] = {
/* Xilinx S3AN Internal Flash */
{ "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
@@ -38,6 +59,57 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
return page | offset;
}
+/**
+ * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
+ * @nor: pointer to 'struct spi_nor'.
+ * @sr: pointer to a DMA-able buffer where the value of the
+ * Status Register will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, sr, 0));
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
+ 1);
+ }
+
+ if (ret)
+ dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
+
+ return ret;
+}
+
+/**
+ * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
+ * the flash is ready for new commands.
+ * @nor: pointer to 'struct spi_nor'.
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+static int spi_nor_xsr_ready(struct spi_nor *nor)
+{
+ int ret;
+
+ ret = spi_nor_xread_sr(nor, nor->bouncebuf);
+ if (ret)
+ return ret;
+
+ return !!(nor->bouncebuf[0] & XSR_RDY);
+}
+
static int xilinx_nor_setup(struct spi_nor *nor,
const struct spi_nor_hwcaps *hwcaps)
{
@@ -83,6 +155,7 @@ static int xilinx_nor_setup(struct spi_nor *nor,
static void xilinx_late_init(struct spi_nor *nor)
{
nor->params->setup = xilinx_nor_setup;
+ nor->params->ready = spi_nor_xsr_ready;
}
static const struct spi_nor_fixups xilinx_fixups = {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fc90fce26e33..b44b05a6f934 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -86,15 +86,6 @@
#define SPINOR_OP_BP 0x02 /* Byte program */
#define SPINOR_OP_AAI_WP 0xad /* Auto address increment word program */
-/* Used for S3AN flashes only */
-#define SPINOR_OP_XSE 0x50 /* Sector erase */
-#define SPINOR_OP_XPP 0x82 /* Page program */
-#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
-
-#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
-#define XSR_RDY BIT(7) /* Ready */
-
-
/* Used for Macronix and Winbond flashes. */
#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c
2022-02-02 14:58 ` [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c Michael Walle
@ 2022-02-10 3:04 ` Tudor.Ambarus
2022-02-15 18:57 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:04 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Mechanically move all the xilinx functions to its own module.
>
> Then register the new flash specific ready() function.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
excellent!
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 64 +------------------------------
> drivers/mtd/spi-nor/core.h | 18 ---------
> drivers/mtd/spi-nor/xilinx.c | 73 ++++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 9 -----
> 4 files changed, 74 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index c181f2680df2..fdc8ef623254 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -598,57 +598,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
> return ret;
> }
>
> -/**
> - * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> - * @nor: pointer to 'struct spi_nor'.
> - * @sr: pointer to a DMA-able buffer where the value of the
> - * Status Register will be written.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op =
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
> - SPI_MEM_OP_NO_ADDR,
> - SPI_MEM_OP_NO_DUMMY,
> - SPI_MEM_OP_DATA_IN(1, sr, 0));
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
> - 1);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> -
> - return ret;
> -}
> -
> -/**
> - * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
> - * the flash is ready for new commands.
> - * @nor: pointer to 'struct spi_nor'.
> - *
> - * Return: 1 if ready, 0 if not ready, -errno on errors.
> - */
> -static int spi_nor_xsr_ready(struct spi_nor *nor)
> -{
> - int ret;
> -
> - ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - return !!(nor->bouncebuf[0] & XSR_RDY);
> -}
> -
> /**
> * spi_nor_clear_sr() - Clear the Status Register.
> * @nor: pointer to 'struct spi_nor'.
> @@ -798,10 +747,7 @@ static int spi_nor_ready(struct spi_nor *nor)
> if (nor->params->ready)
> return nor->params->ready(nor);
>
> - if (nor->flags & SNOR_F_READY_XSR_RDY)
> - sr = spi_nor_xsr_ready(nor);
> - else
> - sr = spi_nor_sr_ready(nor);
> + sr = spi_nor_sr_ready(nor);
> if (sr < 0)
> return sr;
> fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
> @@ -2677,14 +2623,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
>
> if (flags & USE_FSR)
> nor->flags |= SNOR_F_USE_FSR;
> -
> - /*
> - * Make sure the XSR_RDY flag is set before calling
> - * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
> - * with Atmel SPI NOR.
> - */
> - if (flags & SPI_NOR_XSR_RDY)
> - nor->flags |= SNOR_F_READY_XSR_RDY;
> }
>
> /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 446218b0e017..fabc01ae9a81 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -15,7 +15,6 @@ enum spi_nor_option_flags {
> SNOR_F_USE_FSR = BIT(0),
> SNOR_F_HAS_SR_TB = BIT(1),
> SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> - SNOR_F_READY_XSR_RDY = BIT(3),
> SNOR_F_USE_CLSR = BIT(4),
> SNOR_F_BROKEN_RESET = BIT(5),
> SNOR_F_4B_OPCODES = BIT(6),
> @@ -351,8 +350,6 @@ struct spi_nor_fixups {
> * SPI_NOR_NO_FR: can't do fastread.
> * USE_CLSR: use CLSR command.
> * USE_FSR: use flag status register
> - * SPI_NOR_XSR_RDY: S3AN flashes have specific opcode to read the
> - * status register.
> *
> * @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
> * Used when SFDP tables are not defined in the flash. These
> @@ -405,7 +402,6 @@ struct flash_info {
> #define SPI_NOR_NO_FR BIT(8)
> #define USE_CLSR BIT(9)
> #define USE_FSR BIT(10)
> -#define SPI_NOR_XSR_RDY BIT(11)
>
> u8 no_sfdp_flags;
> #define SPI_NOR_SKIP_SFDP BIT(0)
> @@ -462,19 +458,6 @@ struct flash_info {
> .addr_width = (_addr_width), \
> .flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR, \
>
> -#define S3AN_INFO(_jedec_id, _n_sectors, _page_size) \
> - .id = { \
> - ((_jedec_id) >> 16) & 0xff, \
> - ((_jedec_id) >> 8) & 0xff, \
> - (_jedec_id) & 0xff \
> - }, \
> - .id_len = 3, \
> - .sector_size = (8*_page_size), \
> - .n_sectors = (_n_sectors), \
> - .page_size = _page_size, \
> - .addr_width = 3, \
> - .flags = SPI_NOR_NO_FR | SPI_NOR_XSR_RDY,
> -
> #define OTP_INFO(_len, _n_regions, _base, _offset) \
> .otp_org = { \
> .len = (_len), \
> @@ -564,7 +547,6 @@ int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
> int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
> int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
>
> -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,
> u8 *buf);
> ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> index 580562bc1e45..a865dadc2e5d 100644
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ b/drivers/mtd/spi-nor/xilinx.c
> @@ -8,6 +8,27 @@
>
> #include "core.h"
>
> +#define SPINOR_OP_XSE 0x50 /* Sector erase */
> +#define SPINOR_OP_XPP 0x82 /* Page program */
> +#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
> +
> +#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> +#define XSR_RDY BIT(7) /* Ready */
> +
> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size) \
> + .id = { \
> + ((_jedec_id) >> 16) & 0xff, \
> + ((_jedec_id) >> 8) & 0xff, \
> + (_jedec_id) & 0xff \
> + }, \
> + .id_len = 3, \
> + .sector_size = (8*_page_size), \
> + .n_sectors = (_n_sectors), \
> + .page_size = _page_size, \
> + .addr_width = 3, \
> + .flags = SPI_NOR_NO_FR
> +
> +/* Xilinx S3AN share MFR with Atmel SPI NOR */
> static const struct flash_info xilinx_parts[] = {
> /* Xilinx S3AN Internal Flash */
> { "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
> @@ -38,6 +59,57 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
> return page | offset;
> }
>
> +/**
> + * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> + * @nor: pointer to 'struct spi_nor'.
> + * @sr: pointer to a DMA-able buffer where the value of the
> + * Status Register will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_IN(1, sr, 0));
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
> + 1);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> +
> + return ret;
> +}
> +
> +/**
> + * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
> + * the flash is ready for new commands.
> + * @nor: pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int spi_nor_xsr_ready(struct spi_nor *nor)
> +{
> + int ret;
> +
> + ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> + if (ret)
> + return ret;
> +
> + return !!(nor->bouncebuf[0] & XSR_RDY);
> +}
> +
> static int xilinx_nor_setup(struct spi_nor *nor,
> const struct spi_nor_hwcaps *hwcaps)
> {
> @@ -83,6 +155,7 @@ static int xilinx_nor_setup(struct spi_nor *nor,
> static void xilinx_late_init(struct spi_nor *nor)
> {
> nor->params->setup = xilinx_nor_setup;
> + nor->params->ready = spi_nor_xsr_ready;
> }
>
> static const struct spi_nor_fixups xilinx_fixups = {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index fc90fce26e33..b44b05a6f934 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -86,15 +86,6 @@
> #define SPINOR_OP_BP 0x02 /* Byte program */
> #define SPINOR_OP_AAI_WP 0xad /* Auto address increment word program */
>
> -/* Used for S3AN flashes only */
> -#define SPINOR_OP_XSE 0x50 /* Sector erase */
> -#define SPINOR_OP_XPP 0x82 /* Page program */
> -#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
> -
> -#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> -#define XSR_RDY BIT(7) /* Ready */
> -
> -
> /* Used for Macronix and Winbond flashes. */
> #define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
> #define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c
2022-02-02 14:58 ` [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c Michael Walle
2022-02-10 3:04 ` Tudor.Ambarus
@ 2022-02-15 18:57 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 18:57 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> Mechanically move all the xilinx functions to its own module.
If you do some code changes mechanically you should also paste the
semantic patch for future reference.
>
> Then register the new flash specific ready() function.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 64 +------------------------------
> drivers/mtd/spi-nor/core.h | 18 ---------
> drivers/mtd/spi-nor/xilinx.c | 73 ++++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 9 -----
> 4 files changed, 74 insertions(+), 90 deletions(-)
>
[...]
>
> +/**
> + * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> + * @nor: pointer to 'struct spi_nor'.
> + * @sr: pointer to a DMA-able buffer where the value of the
> + * Status Register will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
Nitpick: rename to xilinx_read_sr()?
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_IN(1, sr, 0));
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
> + 1);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> +
> + return ret;
> +}
> +
> +/**
> + * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
> + * the flash is ready for new commands.
> + * @nor: pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int spi_nor_xsr_ready(struct spi_nor *nor)
Nitpick: rename to xilinx_sr_ready()?
Either way,
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (3 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 04/14] mtd: spi-nor: move all xilinx specifics into xilinx.c Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-02 17:14 ` kernel test robot
` (3 more replies)
2022-02-02 14:58 ` [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message Michael Walle
` (10 subsequent siblings)
15 siblings, 4 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Drop the generic spi_nor prefix for all the xilinx functions.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/xilinx.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
index a865dadc2e5d..3e85530df1e4 100644
--- a/drivers/mtd/spi-nor/xilinx.c
+++ b/drivers/mtd/spi-nor/xilinx.c
@@ -8,9 +8,9 @@
#include "core.h"
-#define SPINOR_OP_XSE 0x50 /* Sector erase */
-#define SPINOR_OP_XPP 0x82 /* Page program */
-#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
+#define XILINX_OP_SE 0x50 /* Sector erase */
+#define XILINX_OP_PP 0x82 /* Page program */
+#define XILINX_OP_RDSR 0xd7 /* Read status register */
#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
#define XSR_RDY BIT(7) /* Ready */
@@ -60,20 +60,20 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
}
/**
- * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
+ * xilinx_read_sr() - Read the Status Register on S3AN flashes.
* @nor: pointer to 'struct spi_nor'.
* @sr: pointer to a DMA-able buffer where the value of the
* Status Register will be written.
*
* Return: 0 on success, -errno otherwise.
*/
-static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
+static int xilinx_read_sr(struct spi_nor *nor, u8 *sr)
{
int ret;
if (nor->spimem) {
struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
+ SPI_MEM_OP(SPI_MEM_OP_CMD(XILINX_OP_RDSR, 0),
SPI_MEM_OP_NO_ADDR,
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 0));
@@ -82,7 +82,7 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
+ ret = spi_nor_controller_ops_read_reg(nor, XILINX_OP_RDSR, sr,
1);
}
@@ -99,11 +99,11 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
*
* Return: 1 if ready, 0 if not ready, -errno on errors.
*/
-static int spi_nor_xsr_ready(struct spi_nor *nor)
+static int xilinx_sr_ready(struct spi_nor *nor)
{
int ret;
- ret = spi_nor_xread_sr(nor, nor->bouncebuf);
+ ret = xilinx_read_sr(nor, nor->bouncebuf);
if (ret)
return ret;
@@ -116,12 +116,12 @@ static int xilinx_nor_setup(struct spi_nor *nor,
u32 page_size;
int ret;
- ret = spi_nor_xread_sr(nor, nor->bouncebuf);
+ ret = xilinx_read_sr(nor, nor->bouncebuf);
if (ret)
return ret;
- nor->erase_opcode = SPINOR_OP_XSE;
- nor->program_opcode = SPINOR_OP_XPP;
+ nor->erase_opcode = XILINX_OP_SE;
+ nor->program_opcode = XILINX_OP_PP;
nor->read_opcode = SPINOR_OP_READ;
nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
@@ -155,7 +155,7 @@ static int xilinx_nor_setup(struct spi_nor *nor,
static void xilinx_late_init(struct spi_nor *nor)
{
nor->params->setup = xilinx_nor_setup;
- nor->params->ready = spi_nor_xsr_ready;
+ nor->params->ready = xilinx_sr_ready;
}
static const struct spi_nor_fixups xilinx_fixups = {
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
@ 2022-02-02 17:14 ` kernel test robot
2022-02-02 20:07 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 17:14 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
config: alpha-allmodconfig (https://download.01.org/0day-ci/archive/20220203/202202030130.meVKrEGw-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/6527d5f32d65d5695ddcc0bcf3ac31928e64a935
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
git checkout 6527d5f32d65d5695ddcc0bcf3ac31928e64a935
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/mtd/spi-nor/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mtd/spi-nor/xilinx.c:103: warning: expecting prototype for spi_nor_xsr_ready(). Prototype was for xilinx_sr_ready() instead
vim +103 drivers/mtd/spi-nor/xilinx.c
37ccf5a41e1528 Michael Walle 2022-02-02 94
37ccf5a41e1528 Michael Walle 2022-02-02 95 /**
37ccf5a41e1528 Michael Walle 2022-02-02 96 * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
37ccf5a41e1528 Michael Walle 2022-02-02 97 * the flash is ready for new commands.
37ccf5a41e1528 Michael Walle 2022-02-02 98 * @nor: pointer to 'struct spi_nor'.
37ccf5a41e1528 Michael Walle 2022-02-02 99 *
37ccf5a41e1528 Michael Walle 2022-02-02 100 * Return: 1 if ready, 0 if not ready, -errno on errors.
37ccf5a41e1528 Michael Walle 2022-02-02 101 */
6527d5f32d65d5 Michael Walle 2022-02-02 102 static int xilinx_sr_ready(struct spi_nor *nor)
37ccf5a41e1528 Michael Walle 2022-02-02 @103 {
37ccf5a41e1528 Michael Walle 2022-02-02 104 int ret;
37ccf5a41e1528 Michael Walle 2022-02-02 105
6527d5f32d65d5 Michael Walle 2022-02-02 106 ret = xilinx_read_sr(nor, nor->bouncebuf);
37ccf5a41e1528 Michael Walle 2022-02-02 107 if (ret)
37ccf5a41e1528 Michael Walle 2022-02-02 108 return ret;
37ccf5a41e1528 Michael Walle 2022-02-02 109
37ccf5a41e1528 Michael Walle 2022-02-02 110 return !!(nor->bouncebuf[0] & XSR_RDY);
37ccf5a41e1528 Michael Walle 2022-02-02 111 }
37ccf5a41e1528 Michael Walle 2022-02-02 112
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
2022-02-02 17:14 ` kernel test robot
@ 2022-02-02 20:07 ` kernel test robot
2022-02-10 3:08 ` Tudor.Ambarus
2022-02-15 19:04 ` Pratyush Yadav
3 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 20:07 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: llvm, kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
config: riscv-randconfig-r042-20220131 (https://download.01.org/0day-ci/archive/20220203/202202030313.BlFmGeWH-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/6527d5f32d65d5695ddcc0bcf3ac31928e64a935
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
git checkout 6527d5f32d65d5695ddcc0bcf3ac31928e64a935
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/mtd/spi-nor/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mtd/spi-nor/xilinx.c:103: warning: expecting prototype for spi_nor_xsr_ready(). Prototype was for xilinx_sr_ready() instead
vim +103 drivers/mtd/spi-nor/xilinx.c
37ccf5a41e1528e Michael Walle 2022-02-02 94
37ccf5a41e1528e Michael Walle 2022-02-02 95 /**
37ccf5a41e1528e Michael Walle 2022-02-02 96 * spi_nor_xsr_ready() - Query the Status Register of the S3AN flash to see if
37ccf5a41e1528e Michael Walle 2022-02-02 97 * the flash is ready for new commands.
37ccf5a41e1528e Michael Walle 2022-02-02 98 * @nor: pointer to 'struct spi_nor'.
37ccf5a41e1528e Michael Walle 2022-02-02 99 *
37ccf5a41e1528e Michael Walle 2022-02-02 100 * Return: 1 if ready, 0 if not ready, -errno on errors.
37ccf5a41e1528e Michael Walle 2022-02-02 101 */
6527d5f32d65d56 Michael Walle 2022-02-02 102 static int xilinx_sr_ready(struct spi_nor *nor)
37ccf5a41e1528e Michael Walle 2022-02-02 @103 {
37ccf5a41e1528e Michael Walle 2022-02-02 104 int ret;
37ccf5a41e1528e Michael Walle 2022-02-02 105
6527d5f32d65d56 Michael Walle 2022-02-02 106 ret = xilinx_read_sr(nor, nor->bouncebuf);
37ccf5a41e1528e Michael Walle 2022-02-02 107 if (ret)
37ccf5a41e1528e Michael Walle 2022-02-02 108 return ret;
37ccf5a41e1528e Michael Walle 2022-02-02 109
37ccf5a41e1528e Michael Walle 2022-02-02 110 return !!(nor->bouncebuf[0] & XSR_RDY);
37ccf5a41e1528e Michael Walle 2022-02-02 111 }
37ccf5a41e1528e Michael Walle 2022-02-02 112
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
2022-02-02 17:14 ` kernel test robot
2022-02-02 20:07 ` kernel test robot
@ 2022-02-10 3:08 ` Tudor.Ambarus
2022-02-10 8:04 ` Michael Walle
2022-02-15 19:04 ` Pratyush Yadav
3 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:08 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Drop the generic spi_nor prefix for all the xilinx functions.
mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is too
generic and can conflict with methods from other subsystems.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/xilinx.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> index a865dadc2e5d..3e85530df1e4 100644
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ b/drivers/mtd/spi-nor/xilinx.c
> @@ -8,9 +8,9 @@
>
> #include "core.h"
>
> -#define SPINOR_OP_XSE 0x50 /* Sector erase */
> -#define SPINOR_OP_XPP 0x82 /* Page program */
> -#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
> +#define XILINX_OP_SE 0x50 /* Sector erase */
> +#define XILINX_OP_PP 0x82 /* Page program */
> +#define XILINX_OP_RDSR 0xd7 /* Read status register */
>
> #define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> #define XSR_RDY BIT(7) /* Ready */
> @@ -60,20 +60,20 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
> }
>
> /**
> - * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> + * xilinx_read_sr() - Read the Status Register on S3AN flashes.
> * @nor: pointer to 'struct spi_nor'.
> * @sr: pointer to a DMA-able buffer where the value of the
> * Status Register will be written.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> +static int xilinx_read_sr(struct spi_nor *nor, u8 *sr)
> {
> int ret;
>
> if (nor->spimem) {
> struct spi_mem_op op =
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 0),
> + SPI_MEM_OP(SPI_MEM_OP_CMD(XILINX_OP_RDSR, 0),
> SPI_MEM_OP_NO_ADDR,
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_DATA_IN(1, sr, 0));
> @@ -82,7 +82,7 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
>
> ret = spi_mem_exec_op(nor->spimem, &op);
> } else {
> - ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_XRDSR, sr,
> + ret = spi_nor_controller_ops_read_reg(nor, XILINX_OP_RDSR, sr,
> 1);
> }
>
> @@ -99,11 +99,11 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> *
> * Return: 1 if ready, 0 if not ready, -errno on errors.
> */
> -static int spi_nor_xsr_ready(struct spi_nor *nor)
> +static int xilinx_sr_ready(struct spi_nor *nor)
> {
> int ret;
>
> - ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> + ret = xilinx_read_sr(nor, nor->bouncebuf);
> if (ret)
> return ret;
>
> @@ -116,12 +116,12 @@ static int xilinx_nor_setup(struct spi_nor *nor,
> u32 page_size;
> int ret;
>
> - ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> + ret = xilinx_read_sr(nor, nor->bouncebuf);
> if (ret)
> return ret;
>
> - nor->erase_opcode = SPINOR_OP_XSE;
> - nor->program_opcode = SPINOR_OP_XPP;
> + nor->erase_opcode = XILINX_OP_SE;
> + nor->program_opcode = XILINX_OP_PP;
> nor->read_opcode = SPINOR_OP_READ;
> nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
>
> @@ -155,7 +155,7 @@ static int xilinx_nor_setup(struct spi_nor *nor,
> static void xilinx_late_init(struct spi_nor *nor)
> {
> nor->params->setup = xilinx_nor_setup;
> - nor->params->ready = spi_nor_xsr_ready;
> + nor->params->ready = xilinx_sr_ready;
> }
>
> static const struct spi_nor_fixups xilinx_fixups = {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-10 3:08 ` Tudor.Ambarus
@ 2022-02-10 8:04 ` Michael Walle
2022-02-10 8:06 ` Tudor.Ambarus
0 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-10 8:04 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
> On 2/2/22 16:58, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Drop the generic spi_nor prefix for all the xilinx functions.
>
> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is too
> generic and can conflict with methods from other subsystems.
But all the other functions in this file start with xilinx_ ;)
I don't have a strong opinion here, other than it shouldn't
be called spi_nor_read_blaba() because that looks like a
standard spi nor function belonging in core.c
-michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-10 8:04 ` Michael Walle
@ 2022-02-10 8:06 ` Tudor.Ambarus
2022-02-15 8:25 ` Michael Walle
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 8:06 UTC (permalink / raw)
To: michael
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
On 2/10/22 10:04, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
>> On 2/2/22 16:58, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Drop the generic spi_nor prefix for all the xilinx functions.
>>
>> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is too
>> generic and can conflict with methods from other subsystems.
>
> But all the other functions in this file start with xilinx_ ;)
>
> I don't have a strong opinion here, other than it shouldn't
> be called spi_nor_read_blaba() because that looks like a
> standard spi nor function belonging in core.c
>
then let's prepend all with spi_nor_xilinx_*()?
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-10 8:06 ` Tudor.Ambarus
@ 2022-02-15 8:25 ` Michael Walle
2022-02-15 8:52 ` Tudor.Ambarus
0 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-15 8:25 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
Am 2022-02-10 09:06, schrieb Tudor.Ambarus@microchip.com:
> On 2/10/22 10:04, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
>>> On 2/2/22 16:58, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>> know
>>>> the content is safe
>>>>
>>>> Drop the generic spi_nor prefix for all the xilinx functions.
>>>
>>> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is
>>> too
>>> generic and can conflict with methods from other subsystems.
>>
>> But all the other functions in this file start with xilinx_ ;)
>>
>> I don't have a strong opinion here, other than it shouldn't
>> be called spi_nor_read_blaba() because that looks like a
>> standard spi nor function belonging in core.c
>>
>
> then let's prepend all with spi_nor_xilinx_*()?
I'm still not sure what to do here. Have a look at all the other
vendor modules in spi-nor. they are all prefixed with the vendor
name? E.g. there is a sst_write() which is far more likely to
cause a conflict. So should we rename all these functions? Or
do we just take our chance that it might have a conflict in
the future (with an easy fix to rename the function then). TBH
I doubt there will be a global symbol "xilinx_read_sr()".
But I care for consistency, so having some named xilinx_, sst_,
st_micron_ and some spi_nor_read_xsr sounds and looks awful.
-michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-15 8:25 ` Michael Walle
@ 2022-02-15 8:52 ` Tudor.Ambarus
2022-02-15 9:58 ` Michael Walle
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-15 8:52 UTC (permalink / raw)
To: michael, miquel.raynal
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
Miquel in To:
On 2/15/22 10:25, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am 2022-02-10 09:06, schrieb Tudor.Ambarus@microchip.com:
>> On 2/10/22 10:04, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
>>>> On 2/2/22 16:58, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Drop the generic spi_nor prefix for all the xilinx functions.
>>>>
>>>> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is
>>>> too
>>>> generic and can conflict with methods from other subsystems.
>>>
>>> But all the other functions in this file start with xilinx_ ;)
>>>
>>> I don't have a strong opinion here, other than it shouldn't
>>> be called spi_nor_read_blaba() because that looks like a
>>> standard spi nor function belonging in core.c
>>>
>>
>> then let's prepend all with spi_nor_xilinx_*()?
>
> I'm still not sure what to do here. Have a look at all the other
> vendor modules in spi-nor. they are all prefixed with the vendor
> name? E.g. there is a sst_write() which is far more likely to
> cause a conflict. So should we rename all these functions? Or
> do we just take our chance that it might have a conflict in
> the future (with an easy fix to rename the function then). TBH
> I doubt there will be a global symbol "xilinx_read_sr()".
I doubt it will not be a conflict.
>
> But I care for consistency, so having some named xilinx_, sst_,
> st_micron_ and some spi_nor_read_xsr sounds and looks awful.
yes, I agree. Take a look on what's happening in NAND. They prepend
the name with vendor_nand_*(). Or in SPI NAND they use flash family
names which should be unique. So how about aligning with NAND and
use vendor_nor_*()?
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-15 8:52 ` Tudor.Ambarus
@ 2022-02-15 9:58 ` Michael Walle
2022-02-15 10:12 ` Tudor.Ambarus
0 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-15 9:58 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: miquel.raynal, linux-mtd, linux-kernel, p.yadav, richard, vigneshr
Am 2022-02-15 09:52, schrieb Tudor.Ambarus@microchip.com:
> Miquel in To:
>
> On 2/15/22 10:25, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Am 2022-02-10 09:06, schrieb Tudor.Ambarus@microchip.com:
>>> On 2/10/22 10:04, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>> know
>>>> the content is safe
>>>>
>>>> Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
>>>>> On 2/2/22 16:58, Michael Walle wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>> know
>>>>>> the content is safe
>>>>>>
>>>>>> Drop the generic spi_nor prefix for all the xilinx functions.
>>>>>
>>>>> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is
>>>>> too
>>>>> generic and can conflict with methods from other subsystems.
>>>>
>>>> But all the other functions in this file start with xilinx_ ;)
>>>>
>>>> I don't have a strong opinion here, other than it shouldn't
>>>> be called spi_nor_read_blaba() because that looks like a
>>>> standard spi nor function belonging in core.c
>>>>
>>>
>>> then let's prepend all with spi_nor_xilinx_*()?
>>
>> I'm still not sure what to do here. Have a look at all the other
>> vendor modules in spi-nor. they are all prefixed with the vendor
>> name? E.g. there is a sst_write() which is far more likely to
>> cause a conflict. So should we rename all these functions? Or
>> do we just take our chance that it might have a conflict in
>> the future (with an easy fix to rename the function then). TBH
>> I doubt there will be a global symbol "xilinx_read_sr()".
>
> I doubt it will not be a conflict.
>
>>
>> But I care for consistency, so having some named xilinx_, sst_,
>> st_micron_ and some spi_nor_read_xsr sounds and looks awful.
>
> yes, I agree. Take a look on what's happening in NAND. They prepend
> the name with vendor_nand_*(). Or in SPI NAND they use flash family
> names which should be unique. So how about aligning with NAND and
> use vendor_nor_*()?
Sounds good. Regarding the flash family.. take a look at Winbond W25M
which can either be NAND or NOR depending on the size ;)
But the main question was rather whether we rename all the function
names at once or bit by bit. To proceed here with this series, I'd
use the vendor_nor_ prefix for the moved functions (but still keep
the micron_st_ st_micron_ rename patch).
-michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-15 9:58 ` Michael Walle
@ 2022-02-15 10:12 ` Tudor.Ambarus
0 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-15 10:12 UTC (permalink / raw)
To: michael
Cc: miquel.raynal, linux-mtd, linux-kernel, p.yadav, richard, vigneshr
On 2/15/22 11:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am 2022-02-15 09:52, schrieb Tudor.Ambarus@microchip.com:
>> Miquel in To:
>>
>> On 2/15/22 10:25, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2022-02-10 09:06, schrieb Tudor.Ambarus@microchip.com:
>>>> On 2/10/22 10:04, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Am 2022-02-10 04:08, schrieb Tudor.Ambarus@microchip.com:
>>>>>> On 2/2/22 16:58, Michael Walle wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>>> know
>>>>>>> the content is safe
>>>>>>>
>>>>>>> Drop the generic spi_nor prefix for all the xilinx functions.
>>>>>>
>>>>>> mm, no, I would keep the spi_nor prefix because xilinx_sr_ready is
>>>>>> too
>>>>>> generic and can conflict with methods from other subsystems.
>>>>>
>>>>> But all the other functions in this file start with xilinx_ ;)
>>>>>
>>>>> I don't have a strong opinion here, other than it shouldn't
>>>>> be called spi_nor_read_blaba() because that looks like a
>>>>> standard spi nor function belonging in core.c
>>>>>
>>>>
>>>> then let's prepend all with spi_nor_xilinx_*()?
>>>
>>> I'm still not sure what to do here. Have a look at all the other
>>> vendor modules in spi-nor. they are all prefixed with the vendor
>>> name? E.g. there is a sst_write() which is far more likely to
>>> cause a conflict. So should we rename all these functions? Or
>>> do we just take our chance that it might have a conflict in
>>> the future (with an easy fix to rename the function then). TBH
>>> I doubt there will be a global symbol "xilinx_read_sr()".
>>
>> I doubt it will not be a conflict.
>>
>>>
>>> But I care for consistency, so having some named xilinx_, sst_,
>>> st_micron_ and some spi_nor_read_xsr sounds and looks awful.
>>
>> yes, I agree. Take a look on what's happening in NAND. They prepend
>> the name with vendor_nand_*(). Or in SPI NAND they use flash family
>> names which should be unique. So how about aligning with NAND and
>> use vendor_nor_*()?
>
> Sounds good. Regarding the flash family.. take a look at Winbond W25M
> which can either be NAND or NOR depending on the size ;)
right, vendor_nor_*() should be just fine
>
> But the main question was rather whether we rename all the function
> names at once or bit by bit. To proceed here with this series, I'd
> use the vendor_nor_ prefix for the moved functions (but still keep
> the micron_st_ st_micron_ rename patch).
>
let's rename them all in a single patch before moving the vendor code
out of the core, and then you can fit with the moved bits in an
elegant way ;)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
` (2 preceding siblings ...)
2022-02-10 3:08 ` Tudor.Ambarus
@ 2022-02-15 19:04 ` Pratyush Yadav
3 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:04 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> Drop the generic spi_nor prefix for all the xilinx functions.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/xilinx.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> index a865dadc2e5d..3e85530df1e4 100644
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ b/drivers/mtd/spi-nor/xilinx.c
> @@ -8,9 +8,9 @@
>
> #include "core.h"
>
> -#define SPINOR_OP_XSE 0x50 /* Sector erase */
> -#define SPINOR_OP_XPP 0x82 /* Page program */
> -#define SPINOR_OP_XRDSR 0xd7 /* Read status register */
> +#define XILINX_OP_SE 0x50 /* Sector erase */
> +#define XILINX_OP_PP 0x82 /* Page program */
> +#define XILINX_OP_RDSR 0xd7 /* Read status register */
>
> #define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> #define XSR_RDY BIT(7) /* Ready */
> @@ -60,20 +60,20 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
> }
>
> /**
> - * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> + * xilinx_read_sr() - Read the Status Register on S3AN flashes.
> * @nor: pointer to 'struct spi_nor'.
> * @sr: pointer to a DMA-able buffer where the value of the
> * Status Register will be written.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> +static int xilinx_read_sr(struct spi_nor *nor, u8 *sr)
Great minds think alike huh ;-)
But I agree with Tudor. vendor_nor_* or spi_nor_vendor_* would be
better. I don't have much preference for any though. Please also
remember to update the name in the doc comment, as the kernel test robot
has already pointed out for this patch.
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (4 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 05/14] mtd: spi-nor: xilinx: rename vendor specific functions and defines Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:11 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c Michael Walle
` (9 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
We are reading the status register, there is no XRDSR register.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/xilinx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
index 3e85530df1e4..9e3ed9609250 100644
--- a/drivers/mtd/spi-nor/xilinx.c
+++ b/drivers/mtd/spi-nor/xilinx.c
@@ -87,7 +87,7 @@ static int xilinx_read_sr(struct spi_nor *nor, u8 *sr)
}
if (ret)
- dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
+ dev_dbg(nor->dev, "error %d reading SR\n", ret);
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message
2022-02-02 14:58 ` [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message Michael Walle
@ 2022-02-10 3:11 ` Tudor.Ambarus
2022-02-15 19:05 ` Pratyush Yadav
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:11 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> We are reading the status register, there is no XRDSR register.
don't use the commit message as a continuation of the subject. Tell in the
commit message what you're doing and why it is worth taking it.
with that fixed:
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/xilinx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> index 3e85530df1e4..9e3ed9609250 100644
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ b/drivers/mtd/spi-nor/xilinx.c
> @@ -87,7 +87,7 @@ static int xilinx_read_sr(struct spi_nor *nor, u8 *sr)
> }
>
> if (ret)
> - dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> + dev_dbg(nor->dev, "error %d reading SR\n", ret);
>
> return ret;
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message
2022-02-10 3:11 ` Tudor.Ambarus
@ 2022-02-15 19:05 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:05 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:11AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > We are reading the status register, there is no XRDSR register.
>
> don't use the commit message as a continuation of the subject. Tell in the
> commit message what you're doing and why it is worth taking it.
>
> with that fixed:
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (5 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 06/14] mtd: spi-nor: xilinx: correct the debug message Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:18 ` Tudor.Ambarus
2022-02-15 19:13 ` Pratyush Yadav
2022-02-02 14:58 ` [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag Michael Walle
` (8 subsequent siblings)
15 siblings, 2 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
The flag status register is only available on micron flashes. Move all
the functions around that into the micron module.
This is almost a mechanical move except for the spi_nor_fsr_ready()
which now also checks the normal status register. Previously, this was
done in spi_nor_ready().
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 123 +-----------------------------
drivers/mtd/spi-nor/micron-st.c | 129 ++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 8 --
3 files changed, 130 insertions(+), 130 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index fdc8ef623254..e9d9880149d2 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -412,50 +412,6 @@ int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
return ret;
}
-/**
- * spi_nor_read_fsr() - Read the Flag Status Register.
- * @nor: pointer to 'struct spi_nor'
- * @fsr: pointer to a DMA-able buffer where the value of the
- * Flag Status Register will be written. Should be at least 2
- * bytes.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDFSR, 0),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(1, fsr, 0));
-
- if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
- op.addr.nbytes = nor->params->rdsr_addr_nbytes;
- op.dummy.nbytes = nor->params->rdsr_dummy;
- /*
- * We don't want to read only one byte in DTR mode. So,
- * read 2 and then discard the second byte.
- */
- op.data.nbytes = 2;
- }
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDFSR, fsr,
- 1);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d reading FSR\n", ret);
-
- return ret;
-}
-
/**
* spi_nor_read_cr() - Read the Configuration Register using the
* SPINOR_OP_RDCR (35h) command.
@@ -664,75 +620,6 @@ int spi_nor_sr_ready(struct spi_nor *nor)
return !(nor->bouncebuf[0] & SR_WIP);
}
-/**
- * spi_nor_clear_fsr() - Clear the Flag Status Register.
- * @nor: pointer to 'struct spi_nor'.
- */
-static void spi_nor_clear_fsr(struct spi_nor *nor)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 0),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_NO_DATA);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLFSR,
- NULL, 0);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
-}
-
-/**
- * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
- * ready for new commands.
- * @nor: pointer to 'struct spi_nor'.
- *
- * Return: 1 if ready, 0 if not ready, -errno on errors.
- */
-static int spi_nor_fsr_ready(struct spi_nor *nor)
-{
- int ret = spi_nor_read_fsr(nor, nor->bouncebuf);
-
- if (ret)
- return ret;
-
- if (nor->bouncebuf[0] & (FSR_E_ERR | FSR_P_ERR)) {
- if (nor->bouncebuf[0] & FSR_E_ERR)
- dev_err(nor->dev, "Erase operation failed.\n");
- else
- dev_err(nor->dev, "Program operation failed.\n");
-
- if (nor->bouncebuf[0] & FSR_PT_ERR)
- dev_err(nor->dev,
- "Attempted to modify a protected sector.\n");
-
- spi_nor_clear_fsr(nor);
-
- /*
- * WEL bit remains set to one when an erase or page program
- * error occurs. Issue a Write Disable command to protect
- * against inadvertent writes that can possibly corrupt the
- * contents of the memory.
- */
- ret = spi_nor_write_disable(nor);
- if (ret)
- return ret;
-
- return -EIO;
- }
-
- return !!(nor->bouncebuf[0] & FSR_READY);
-}
-
/**
* spi_nor_ready() - Query the flash to see if it is ready for new commands.
* @nor: pointer to 'struct spi_nor'.
@@ -741,19 +628,11 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
*/
static int spi_nor_ready(struct spi_nor *nor)
{
- int sr, fsr;
-
/* flashes might override our standard routine */
if (nor->params->ready)
return nor->params->ready(nor);
- sr = spi_nor_sr_ready(nor);
- if (sr < 0)
- return sr;
- fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
- if (fsr < 0)
- return fsr;
- return sr && fsr;
+ return spi_nor_sr_ready(nor);
}
/**
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index bb95b1aabf74..c66580e8aa00 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -8,6 +8,8 @@
#include "core.h"
+#define SPINOR_OP_RDFSR 0x70 /* Read flag status register */
+#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
#define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
#define SPINOR_OP_MT_RD_ANY_REG 0x85 /* Read volatile register */
#define SPINOR_OP_MT_WR_ANY_REG 0x81 /* Write volatile register */
@@ -17,6 +19,12 @@
#define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
#define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
+/* Flag Status Register bits */
+#define FSR_READY BIT(7) /* Device status, 0 = Busy, 1 = Ready */
+#define FSR_E_ERR BIT(5) /* Erase operation status */
+#define FSR_P_ERR BIT(4) /* Program operation status */
+#define FSR_PT_ERR BIT(1) /* Protection error bit */
+
static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
{
struct spi_mem_op op;
@@ -273,12 +281,133 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
return spi_nor_write_disable(nor);
}
+/**
+ * spi_nor_read_fsr() - Read the Flag Status Register.
+ * @nor: pointer to 'struct spi_nor'
+ * @fsr: pointer to a DMA-able buffer where the value of the
+ * Flag Status Register will be written. Should be at least 2
+ * bytes.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDFSR, 0),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, fsr, 0));
+
+ if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
+ op.addr.nbytes = nor->params->rdsr_addr_nbytes;
+ op.dummy.nbytes = nor->params->rdsr_dummy;
+ /*
+ * We don't want to read only one byte in DTR mode. So,
+ * read 2 and then discard the second byte.
+ */
+ op.data.nbytes = 2;
+ }
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDFSR, fsr,
+ 1);
+ }
+
+ if (ret)
+ dev_dbg(nor->dev, "error %d reading FSR\n", ret);
+
+ return ret;
+}
+
+/**
+ * spi_nor_clear_fsr() - Clear the Flag Status Register.
+ * @nor: pointer to 'struct spi_nor'.
+ */
+static void spi_nor_clear_fsr(struct spi_nor *nor)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 0),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_DATA);
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLFSR,
+ NULL, 0);
+ }
+
+ if (ret)
+ dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
+}
+
+/**
+ * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
+ * ready for new commands.
+ * @nor: pointer to 'struct spi_nor'.
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+static int spi_nor_fsr_ready(struct spi_nor *nor)
+{
+ int sr_ready, ret;
+
+ sr_ready = spi_nor_sr_ready(nor);
+ if (sr_ready < 0)
+ return sr_ready;
+
+ ret = spi_nor_read_fsr(nor, nor->bouncebuf);
+ if (ret)
+ return ret;
+
+ if (nor->bouncebuf[0] & (FSR_E_ERR | FSR_P_ERR)) {
+ if (nor->bouncebuf[0] & FSR_E_ERR)
+ dev_err(nor->dev, "Erase operation failed.\n");
+ else
+ dev_err(nor->dev, "Program operation failed.\n");
+
+ if (nor->bouncebuf[0] & FSR_PT_ERR)
+ dev_err(nor->dev,
+ "Attempted to modify a protected sector.\n");
+
+ spi_nor_clear_fsr(nor);
+
+ /*
+ * WEL bit remains set to one when an erase or page program
+ * error occurs. Issue a Write Disable command to protect
+ * against inadvertent writes that can possibly corrupt the
+ * contents of the memory.
+ */
+ ret = spi_nor_write_disable(nor);
+ if (ret)
+ return ret;
+
+ return -EIO;
+ }
+
+ return sr_ready && !!(nor->bouncebuf[0] & FSR_READY);
+}
+
static void micron_st_default_init(struct spi_nor *nor)
{
nor->flags |= SNOR_F_HAS_LOCK;
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
nor->params->quad_enable = NULL;
nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
+
+ if (nor->flags & SNOR_F_USE_FSR)
+ nor->params->ready = spi_nor_fsr_ready;
}
static const struct spi_nor_fixups micron_st_fixups = {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index b44b05a6f934..4622251a79ff 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -47,8 +47,6 @@
#define SPINOR_OP_RDID 0x9f /* Read JEDEC ID */
#define SPINOR_OP_RDSFDP 0x5a /* Read SFDP */
#define SPINOR_OP_RDCR 0x35 /* Read configuration register */
-#define SPINOR_OP_RDFSR 0x70 /* Read flag status register */
-#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
#define SPINOR_OP_RDEAR 0xc8 /* Read Extended Address Register */
#define SPINOR_OP_WREAR 0xc5 /* Write Extended Address Register */
#define SPINOR_OP_SRSTEN 0x66 /* Software Reset Enable */
@@ -126,12 +124,6 @@
/* Enhanced Volatile Configuration Register bits */
#define EVCR_QUAD_EN_MICRON BIT(7) /* Micron Quad I/O */
-/* Flag Status Register bits */
-#define FSR_READY BIT(7) /* Device status, 0 = Busy, 1 = Ready */
-#define FSR_E_ERR BIT(5) /* Erase operation status */
-#define FSR_P_ERR BIT(4) /* Program operation status */
-#define FSR_PT_ERR BIT(1) /* Protection error bit */
-
/* Status Register 2 bits. */
#define SR2_QUAD_EN_BIT1 BIT(1)
#define SR2_LB1 BIT(3) /* Security Register Lock Bit 1 */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c
2022-02-02 14:58 ` [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c Michael Walle
@ 2022-02-10 3:18 ` Tudor.Ambarus
2022-02-15 19:13 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:18 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The flag status register is only available on micron flashes. Move all
> the functions around that into the micron module.
>
> This is almost a mechanical move except for the spi_nor_fsr_ready()
> which now also checks the normal status register. Previously, this was
> done in spi_nor_ready().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 123 +-----------------------------
> drivers/mtd/spi-nor/micron-st.c | 129 ++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 8 --
> 3 files changed, 130 insertions(+), 130 deletions(-)
>
cut
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index bb95b1aabf74..c66580e8aa00 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> static void micron_st_default_init(struct spi_nor *nor)
> {
> nor->flags |= SNOR_F_HAS_LOCK;
> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> nor->params->quad_enable = NULL;
> nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
> +
> + if (nor->flags & SNOR_F_USE_FSR)
> + nor->params->ready = spi_nor_fsr_ready;
I would like to get rid of the default_init hooks. Can't we use late_init for
setting the ready method?
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c
2022-02-02 14:58 ` [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c Michael Walle
2022-02-10 3:18 ` Tudor.Ambarus
@ 2022-02-15 19:13 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:13 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> The flag status register is only available on micron flashes. Move all
> the functions around that into the micron module.
>
> This is almost a mechanical move except for the spi_nor_fsr_ready()
> which now also checks the normal status register. Previously, this was
> done in spi_nor_ready().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 123 +-----------------------------
> drivers/mtd/spi-nor/micron-st.c | 129 ++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 8 --
> 3 files changed, 130 insertions(+), 130 deletions(-)
>
[...]
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index bb95b1aabf74..c66580e8aa00 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -8,6 +8,8 @@
>
> #include "core.h"
>
> +#define SPINOR_OP_RDFSR 0x70 /* Read flag status register */
> +#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
> #define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
> #define SPINOR_OP_MT_RD_ANY_REG 0x85 /* Read volatile register */
> #define SPINOR_OP_MT_WR_ANY_REG 0x81 /* Write volatile register */
> @@ -17,6 +19,12 @@
> #define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
> #define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
>
> +/* Flag Status Register bits */
> +#define FSR_READY BIT(7) /* Device status, 0 = Busy, 1 = Ready */
> +#define FSR_E_ERR BIT(5) /* Erase operation status */
> +#define FSR_P_ERR BIT(4) /* Program operation status */
> +#define FSR_PT_ERR BIT(1) /* Protection error bit */
> +
> static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
> {
> struct spi_mem_op op;
> @@ -273,12 +281,133 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> return spi_nor_write_disable(nor);
> }
>
> +/**
> + * spi_nor_read_fsr() - Read the Flag Status Register.
> + * @nor: pointer to 'struct spi_nor'
> + * @fsr: pointer to a DMA-able buffer where the value of the
> + * Flag Status Register will be written. Should be at least 2
> + * bytes.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDFSR, 0),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_IN(1, fsr, 0));
> +
> + if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
> + op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> + op.dummy.nbytes = nor->params->rdsr_dummy;
> + /*
> + * We don't want to read only one byte in DTR mode. So,
> + * read 2 and then discard the second byte.
> + */
> + op.data.nbytes = 2;
> + }
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDFSR, fsr,
> + 1);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d reading FSR\n", ret);
> +
> + return ret;
> +}
> +
> +/**
> + * spi_nor_clear_fsr() - Clear the Flag Status Register.
> + * @nor: pointer to 'struct spi_nor'.
> + */
> +static void spi_nor_clear_fsr(struct spi_nor *nor)
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 0),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_NO_DATA);
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLFSR,
> + NULL, 0);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
> +}
> +
> +/**
> + * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
> + * ready for new commands.
> + * @nor: pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int spi_nor_fsr_ready(struct spi_nor *nor)
Nitpick: At this point this function is not just spi_nor_fsr_ready(). I
think it should be renamed to something more accurate like
micron_st_ready() (with whatever prefix scheme that was decided upon).
Looks good otherwise.
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (6 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 07/14] mtd: spi-nor: move all micron-st specifics into micron-st.c Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:37 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix Michael Walle
` (7 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Now that all functions using that flag are local to the micron module,
we can convert the flag to a manufacturer one.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 3 --
drivers/mtd/spi-nor/core.h | 3 --
drivers/mtd/spi-nor/micron-st.c | 92 +++++++++++++++++++++------------
3 files changed, 59 insertions(+), 39 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index e9d9880149d2..be65aaa954ca 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2499,9 +2499,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
if (flags & USE_CLSR)
nor->flags |= SNOR_F_USE_CLSR;
-
- if (flags & USE_FSR)
- nor->flags |= SNOR_F_USE_FSR;
}
/**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index fabc01ae9a81..a02bf54289fb 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -12,7 +12,6 @@
#define SPI_NOR_MAX_ID_LEN 6
enum spi_nor_option_flags {
- SNOR_F_USE_FSR = BIT(0),
SNOR_F_HAS_SR_TB = BIT(1),
SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
SNOR_F_USE_CLSR = BIT(4),
@@ -349,7 +348,6 @@ struct spi_nor_fixups {
* NO_CHIP_ERASE: chip does not support chip erase.
* SPI_NOR_NO_FR: can't do fastread.
* USE_CLSR: use CLSR command.
- * USE_FSR: use flag status register
*
* @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
* Used when SFDP tables are not defined in the flash. These
@@ -401,7 +399,6 @@ struct flash_info {
#define NO_CHIP_ERASE BIT(7)
#define SPI_NOR_NO_FR BIT(8)
#define USE_CLSR BIT(9)
-#define USE_FSR BIT(10)
u8 no_sfdp_flags;
#define SPI_NOR_SKIP_SFDP BIT(0)
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index c66580e8aa00..33531c101ccb 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -8,6 +8,8 @@
#include "core.h"
+#define USE_FSR BIT(0)
+
#define SPINOR_OP_RDFSR 0x70 /* Read flag status register */
#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
#define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
@@ -140,15 +142,17 @@ static const struct spi_nor_fixups mt35xu512aba_fixups = {
static const struct flash_info micron_parts[] = {
{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE)
- .fixups = &mt35xu512aba_fixups},
+ MFR_FLAGS(USE_FSR)
+ .fixups = &mt35xu512aba_fixups
+ },
{ "mt35xu02g", INFO(0x2c5b1c, 0, 128 * 1024, 2048)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ)
- FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ MFR_FLAGS(USE_FSR)
+ },
};
static const struct flash_info st_parts[] = {
@@ -164,57 +168,79 @@ static const struct flash_info st_parts[] = {
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
{ "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
- SPI_NOR_BP3_SR_BIT6 | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ SPI_NOR_BP3_SR_BIT6)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
- SPI_NOR_BP3_SR_BIT6 | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ SPI_NOR_BP3_SR_BIT6)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25ql256a", INFO6(0x20ba19, 0x104400, 64 * 1024, 512)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
- FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
- SPI_NOR_QUAD_READ) },
+ SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25qu256a", INFO6(0x20bb19, 0x104400, 64 * 1024, 512)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
- FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q256ax1", INFO(0x20bb19, 0, 64 * 1024, 512)
- FLAGS(USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25ql512a", INFO6(0x20ba20, 0x104400, 64 * 1024, 1024)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
- FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q512ax3", INFO(0x20ba20, 0, 64 * 1024, 1024)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
- SPI_NOR_BP3_SR_BIT6 | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ SPI_NOR_BP3_SR_BIT6)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25qu512a", INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
- FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
- FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
- SPI_NOR_BP3_SR_BIT6 | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ SPI_NOR_BP3_SR_BIT6)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q00", INFO(0x20ba21, 0, 64 * 1024, 2048)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
- SPI_NOR_BP3_SR_BIT6 | NO_CHIP_ERASE | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ SPI_NOR_BP3_SR_BIT6 | NO_CHIP_ERASE)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "n25q00a", INFO(0x20bb21, 0, 64 * 1024, 2048)
- FLAGS(NO_CHIP_ERASE | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ FLAGS(NO_CHIP_ERASE)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096)
- FLAGS(NO_CHIP_ERASE | USE_FSR)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
+ FLAGS(NO_CHIP_ERASE)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096)
- FLAGS(NO_CHIP_ERASE | USE_FSR)
+ FLAGS(NO_CHIP_ERASE)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
- SPI_NOR_QUAD_READ) },
+ SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_FSR)
+ },
{ "m25p05", INFO(0x202010, 0, 32 * 1024, 2) },
{ "m25p10", INFO(0x202011, 0, 32 * 1024, 4) },
@@ -406,7 +432,7 @@ static void micron_st_default_init(struct spi_nor *nor)
nor->params->quad_enable = NULL;
nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
- if (nor->flags & SNOR_F_USE_FSR)
+ if (nor->info->mfr_flags & USE_FSR)
nor->params->ready = spi_nor_fsr_ready;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag
2022-02-02 14:58 ` [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag Michael Walle
@ 2022-02-10 3:37 ` Tudor.Ambarus
2022-02-15 19:16 ` Pratyush Yadav
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:37 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Now that all functions using that flag are local to the micron module,
> we can convert the flag to a manufacturer one.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 3 --
> drivers/mtd/spi-nor/core.h | 3 --
> drivers/mtd/spi-nor/micron-st.c | 92 +++++++++++++++++++++------------
> 3 files changed, 59 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index e9d9880149d2..be65aaa954ca 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2499,9 +2499,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
>
> if (flags & USE_CLSR)
> nor->flags |= SNOR_F_USE_CLSR;
> -
> - if (flags & USE_FSR)
> - nor->flags |= SNOR_F_USE_FSR;
> }
>
> /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index fabc01ae9a81..a02bf54289fb 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -12,7 +12,6 @@
> #define SPI_NOR_MAX_ID_LEN 6
>
> enum spi_nor_option_flags {
> - SNOR_F_USE_FSR = BIT(0),
> SNOR_F_HAS_SR_TB = BIT(1),
> SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> SNOR_F_USE_CLSR = BIT(4),
> @@ -349,7 +348,6 @@ struct spi_nor_fixups {
> * NO_CHIP_ERASE: chip does not support chip erase.
> * SPI_NOR_NO_FR: can't do fastread.
> * USE_CLSR: use CLSR command.
> - * USE_FSR: use flag status register
> *
> * @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
> * Used when SFDP tables are not defined in the flash. These
> @@ -401,7 +399,6 @@ struct flash_info {
> #define NO_CHIP_ERASE BIT(7)
> #define SPI_NOR_NO_FR BIT(8)
> #define USE_CLSR BIT(9)
> -#define USE_FSR BIT(10)
>
> u8 no_sfdp_flags;
> #define SPI_NOR_SKIP_SFDP BIT(0)
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index c66580e8aa00..33531c101ccb 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -8,6 +8,8 @@
>
> #include "core.h"
>
> +#define USE_FSR BIT(0)
please add a description and inform the reader that this is a manufacturer specific
flash_info flag.
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> +
> #define SPINOR_OP_RDFSR 0x70 /* Read flag status register */
> #define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
> #define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
> @@ -140,15 +142,17 @@ static const struct spi_nor_fixups mt35xu512aba_fixups = {
>
> static const struct flash_info micron_parts[] = {
> { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ |
> SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP)
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE)
> - .fixups = &mt35xu512aba_fixups},
> + MFR_FLAGS(USE_FSR)
> + .fixups = &mt35xu512aba_fixups
> + },
> { "mt35xu02g", INFO(0x2c5b1c, 0, 128 * 1024, 2048)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + MFR_FLAGS(USE_FSR)
> + },
> };
>
> static const struct flash_info st_parts[] = {
> @@ -164,57 +168,79 @@ static const struct flash_info st_parts[] = {
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> - SPI_NOR_BP3_SR_BIT6 | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + SPI_NOR_BP3_SR_BIT6)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> - SPI_NOR_BP3_SR_BIT6 | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + SPI_NOR_BP3_SR_BIT6)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25ql256a", INFO6(0x20ba19, 0x104400, 64 * 1024, 512)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> - SPI_NOR_QUAD_READ) },
> + SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25qu256a", INFO6(0x20bb19, 0x104400, 64 * 1024, 512)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q256ax1", INFO(0x20bb19, 0, 64 * 1024, 512)
> - FLAGS(USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25ql512a", INFO6(0x20ba20, 0x104400, 64 * 1024, 1024)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q512ax3", INFO(0x20ba20, 0, 64 * 1024, 1024)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> - SPI_NOR_BP3_SR_BIT6 | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + SPI_NOR_BP3_SR_BIT6)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25qu512a", INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
> - FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> - SPI_NOR_BP3_SR_BIT6 | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + SPI_NOR_BP3_SR_BIT6)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q00", INFO(0x20ba21, 0, 64 * 1024, 2048)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> - SPI_NOR_BP3_SR_BIT6 | NO_CHIP_ERASE | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + SPI_NOR_BP3_SR_BIT6 | NO_CHIP_ERASE)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "n25q00a", INFO(0x20bb21, 0, 64 * 1024, 2048)
> - FLAGS(NO_CHIP_ERASE | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + FLAGS(NO_CHIP_ERASE)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096)
> - FLAGS(NO_CHIP_ERASE | USE_FSR)
> - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> + FLAGS(NO_CHIP_ERASE)
> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
> { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096)
> - FLAGS(NO_CHIP_ERASE | USE_FSR)
> + FLAGS(NO_CHIP_ERASE)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> - SPI_NOR_QUAD_READ) },
> + SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_FSR)
> + },
>
> { "m25p05", INFO(0x202010, 0, 32 * 1024, 2) },
> { "m25p10", INFO(0x202011, 0, 32 * 1024, 4) },
> @@ -406,7 +432,7 @@ static void micron_st_default_init(struct spi_nor *nor)
> nor->params->quad_enable = NULL;
> nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
>
> - if (nor->flags & SNOR_F_USE_FSR)
> + if (nor->info->mfr_flags & USE_FSR)
> nor->params->ready = spi_nor_fsr_ready;
> }
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag
2022-02-10 3:37 ` Tudor.Ambarus
@ 2022-02-15 19:16 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:16 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:37AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Now that all functions using that flag are local to the micron module,
> > we can convert the flag to a manufacturer one.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > drivers/mtd/spi-nor/core.c | 3 --
> > drivers/mtd/spi-nor/core.h | 3 --
> > drivers/mtd/spi-nor/micron-st.c | 92 +++++++++++++++++++++------------
> > 3 files changed, 59 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index e9d9880149d2..be65aaa954ca 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2499,9 +2499,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
> >
> > if (flags & USE_CLSR)
> > nor->flags |= SNOR_F_USE_CLSR;
> > -
> > - if (flags & USE_FSR)
> > - nor->flags |= SNOR_F_USE_FSR;
> > }
> >
> > /**
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index fabc01ae9a81..a02bf54289fb 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -12,7 +12,6 @@
> > #define SPI_NOR_MAX_ID_LEN 6
> >
> > enum spi_nor_option_flags {
> > - SNOR_F_USE_FSR = BIT(0),
> > SNOR_F_HAS_SR_TB = BIT(1),
> > SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> > SNOR_F_USE_CLSR = BIT(4),
> > @@ -349,7 +348,6 @@ struct spi_nor_fixups {
> > * NO_CHIP_ERASE: chip does not support chip erase.
> > * SPI_NOR_NO_FR: can't do fastread.
> > * USE_CLSR: use CLSR command.
> > - * USE_FSR: use flag status register
> > *
> > * @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
> > * Used when SFDP tables are not defined in the flash. These
> > @@ -401,7 +399,6 @@ struct flash_info {
> > #define NO_CHIP_ERASE BIT(7)
> > #define SPI_NOR_NO_FR BIT(8)
> > #define USE_CLSR BIT(9)
> > -#define USE_FSR BIT(10)
> >
> > u8 no_sfdp_flags;
> > #define SPI_NOR_SKIP_SFDP BIT(0)
> > diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> > index c66580e8aa00..33531c101ccb 100644
> > --- a/drivers/mtd/spi-nor/micron-st.c
> > +++ b/drivers/mtd/spi-nor/micron-st.c
> > @@ -8,6 +8,8 @@
> >
> > #include "core.h"
> >
> > +#define USE_FSR BIT(0)
>
> please add a description and inform the reader that this is a manufacturer specific
> flash_info flag.
+1
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (7 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 08/14] mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:23 ` Tudor.Ambarus
2022-02-15 19:16 ` Pratyush Yadav
2022-02-02 14:58 ` [PATCH v1 10/14] mtd: spi-nor: micron-st: rename vendor specific functions and defines Michael Walle
` (6 subsequent siblings)
15 siblings, 2 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
The usual prefix is micron_st, so use that instead of st_micron.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/micron-st.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 33531c101ccb..ca368b48bcb0 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -284,7 +284,7 @@ static const struct flash_info st_parts[] = {
};
/**
- * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
+ * micron_st_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
* flashes.
* @nor: pointer to 'struct spi_nor'.
* @enable: true to enter the 4-byte address mode, false to exit the 4-byte
@@ -292,7 +292,7 @@ static const struct flash_info st_parts[] = {
*
* Return: 0 on success, -errno otherwise.
*/
-static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
+static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
{
int ret;
@@ -430,7 +430,7 @@ static void micron_st_default_init(struct spi_nor *nor)
nor->flags |= SNOR_F_HAS_LOCK;
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
nor->params->quad_enable = NULL;
- nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
+ nor->params->set_4byte_addr_mode = micron_st_set_4byte_addr_mode;
if (nor->info->mfr_flags & USE_FSR)
nor->params->ready = spi_nor_fsr_ready;
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix
2022-02-02 14:58 ` [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix Michael Walle
@ 2022-02-10 3:23 ` Tudor.Ambarus
2022-02-15 19:16 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:23 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The usual prefix is micron_st, so use that instead of st_micron.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/micron-st.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 33531c101ccb..ca368b48bcb0 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -284,7 +284,7 @@ static const struct flash_info st_parts[] = {
> };
>
> /**
> - * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
> + * micron_st_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
> * flashes.
> * @nor: pointer to 'struct spi_nor'.
> * @enable: true to enter the 4-byte address mode, false to exit the 4-byte
> @@ -292,7 +292,7 @@ static const struct flash_info st_parts[] = {
> *
> * Return: 0 on success, -errno otherwise.
> */
> -static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> +static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> {
> int ret;
>
> @@ -430,7 +430,7 @@ static void micron_st_default_init(struct spi_nor *nor)
> nor->flags |= SNOR_F_HAS_LOCK;
> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> nor->params->quad_enable = NULL;
> - nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
> + nor->params->set_4byte_addr_mode = micron_st_set_4byte_addr_mode;
>
> if (nor->info->mfr_flags & USE_FSR)
> nor->params->ready = spi_nor_fsr_ready;
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix
2022-02-02 14:58 ` [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix Michael Walle
2022-02-10 3:23 ` Tudor.Ambarus
@ 2022-02-15 19:16 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:16 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> The usual prefix is micron_st, so use that instead of st_micron.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 10/14] mtd: spi-nor: micron-st: rename vendor specific functions and defines
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (8 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 09/14] mtd: spi-nor: micron-st: fix micron_st prefix Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:23 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init() Michael Walle
` (5 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Drop the generic spi_nor prefix for all the micron-st functions.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/micron-st.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index ca368b48bcb0..988ecbffdc36 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -308,7 +308,7 @@ static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
}
/**
- * spi_nor_read_fsr() - Read the Flag Status Register.
+ * micron_st_read_fsr() - Read the Flag Status Register.
* @nor: pointer to 'struct spi_nor'
* @fsr: pointer to a DMA-able buffer where the value of the
* Flag Status Register will be written. Should be at least 2
@@ -316,7 +316,7 @@ static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
*
* Return: 0 on success, -errno otherwise.
*/
-static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
+static int micron_st_read_fsr(struct spi_nor *nor, u8 *fsr)
{
int ret;
@@ -352,10 +352,10 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
}
/**
- * spi_nor_clear_fsr() - Clear the Flag Status Register.
+ * micron_st_clear_fsr() - Clear the Flag Status Register.
* @nor: pointer to 'struct spi_nor'.
*/
-static void spi_nor_clear_fsr(struct spi_nor *nor)
+static void micron_st_clear_fsr(struct spi_nor *nor)
{
int ret;
@@ -379,13 +379,13 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
}
/**
- * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
+ * micron_st_fsr_ready() - Query the Flag Status Register to see if the flash is
* ready for new commands.
* @nor: pointer to 'struct spi_nor'.
*
* Return: 1 if ready, 0 if not ready, -errno on errors.
*/
-static int spi_nor_fsr_ready(struct spi_nor *nor)
+static int micron_st_fsr_ready(struct spi_nor *nor)
{
int sr_ready, ret;
@@ -393,7 +393,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
if (sr_ready < 0)
return sr_ready;
- ret = spi_nor_read_fsr(nor, nor->bouncebuf);
+ ret = micron_st_read_fsr(nor, nor->bouncebuf);
if (ret)
return ret;
@@ -407,7 +407,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
dev_err(nor->dev,
"Attempted to modify a protected sector.\n");
- spi_nor_clear_fsr(nor);
+ micron_st_clear_fsr(nor);
/*
* WEL bit remains set to one when an erase or page program
@@ -433,7 +433,7 @@ static void micron_st_default_init(struct spi_nor *nor)
nor->params->set_4byte_addr_mode = micron_st_set_4byte_addr_mode;
if (nor->info->mfr_flags & USE_FSR)
- nor->params->ready = spi_nor_fsr_ready;
+ nor->params->ready = micron_st_fsr_ready;
}
static const struct spi_nor_fixups micron_st_fixups = {
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 10/14] mtd: spi-nor: micron-st: rename vendor specific functions and defines
2022-02-02 14:58 ` [PATCH v1 10/14] mtd: spi-nor: micron-st: rename vendor specific functions and defines Michael Walle
@ 2022-02-10 3:23 ` Tudor.Ambarus
0 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:23 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Drop the generic spi_nor prefix for all the micron-st functions.
please, no.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/micron-st.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index ca368b48bcb0..988ecbffdc36 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -308,7 +308,7 @@ static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> }
>
> /**
> - * spi_nor_read_fsr() - Read the Flag Status Register.
> + * micron_st_read_fsr() - Read the Flag Status Register.
> * @nor: pointer to 'struct spi_nor'
> * @fsr: pointer to a DMA-able buffer where the value of the
> * Flag Status Register will be written. Should be at least 2
> @@ -316,7 +316,7 @@ static int micron_st_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> *
> * Return: 0 on success, -errno otherwise.
> */
> -static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
> +static int micron_st_read_fsr(struct spi_nor *nor, u8 *fsr)
> {
> int ret;
>
> @@ -352,10 +352,10 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
> }
>
> /**
> - * spi_nor_clear_fsr() - Clear the Flag Status Register.
> + * micron_st_clear_fsr() - Clear the Flag Status Register.
> * @nor: pointer to 'struct spi_nor'.
> */
> -static void spi_nor_clear_fsr(struct spi_nor *nor)
> +static void micron_st_clear_fsr(struct spi_nor *nor)
> {
> int ret;
>
> @@ -379,13 +379,13 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
> }
>
> /**
> - * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
> + * micron_st_fsr_ready() - Query the Flag Status Register to see if the flash is
> * ready for new commands.
> * @nor: pointer to 'struct spi_nor'.
> *
> * Return: 1 if ready, 0 if not ready, -errno on errors.
> */
> -static int spi_nor_fsr_ready(struct spi_nor *nor)
> +static int micron_st_fsr_ready(struct spi_nor *nor)
> {
> int sr_ready, ret;
>
> @@ -393,7 +393,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
> if (sr_ready < 0)
> return sr_ready;
>
> - ret = spi_nor_read_fsr(nor, nor->bouncebuf);
> + ret = micron_st_read_fsr(nor, nor->bouncebuf);
> if (ret)
> return ret;
>
> @@ -407,7 +407,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
> dev_err(nor->dev,
> "Attempted to modify a protected sector.\n");
>
> - spi_nor_clear_fsr(nor);
> + micron_st_clear_fsr(nor);
>
> /*
> * WEL bit remains set to one when an erase or page program
> @@ -433,7 +433,7 @@ static void micron_st_default_init(struct spi_nor *nor)
> nor->params->set_4byte_addr_mode = micron_st_set_4byte_addr_mode;
>
> if (nor->info->mfr_flags & USE_FSR)
> - nor->params->ready = spi_nor_fsr_ready;
> + nor->params->ready = micron_st_fsr_ready;
> }
>
> static const struct spi_nor_fixups micron_st_fixups = {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init()
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (9 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 10/14] mtd: spi-nor: micron-st: rename vendor specific functions and defines Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:26 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
` (4 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Increase readability of the code. Instead of returning early if the
flash size is smaller or equal than 16MiB and then do the fixups for
larger flashes, do it within the condition.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/spansion.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 534196b1d3e7..dedc2de90cb8 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -296,13 +296,12 @@ static const struct flash_info spansion_parts[] = {
static void spansion_late_init(struct spi_nor *nor)
{
- if (nor->params->size <= SZ_16M)
- return;
-
- nor->flags |= SNOR_F_4B_OPCODES;
- /* No small sector erase for 4-byte command set */
- nor->erase_opcode = SPINOR_OP_SE;
- nor->mtd.erasesize = nor->info->sector_size;
+ if (nor->params->size > SZ_16M) {
+ nor->flags |= SNOR_F_4B_OPCODES;
+ /* No small sector erase for 4-byte command set */
+ nor->erase_opcode = SPINOR_OP_SE;
+ nor->mtd.erasesize = nor->info->sector_size;
+ }
}
static const struct spi_nor_fixups spansion_fixups = {
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init()
2022-02-02 14:58 ` [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init() Michael Walle
@ 2022-02-10 3:26 ` Tudor.Ambarus
2022-02-10 8:16 ` Michael Walle
2022-02-14 18:59 ` Pratyush Yadav
0 siblings, 2 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:26 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Increase readability of the code. Instead of returning early if the
> flash size is smaller or equal than 16MiB and then do the fixups for
> larger flashes, do it within the condition.
>
mm, no, I'm not sure this improves readability, I see the two equivalent.
The original version has the benefit of no indentation. Pratyush?
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/spansion.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 534196b1d3e7..dedc2de90cb8 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -296,13 +296,12 @@ static const struct flash_info spansion_parts[] = {
>
> static void spansion_late_init(struct spi_nor *nor)
> {
> - if (nor->params->size <= SZ_16M)
> - return;
> -
> - nor->flags |= SNOR_F_4B_OPCODES;
> - /* No small sector erase for 4-byte command set */
> - nor->erase_opcode = SPINOR_OP_SE;
> - nor->mtd.erasesize = nor->info->sector_size;
> + if (nor->params->size > SZ_16M) {
> + nor->flags |= SNOR_F_4B_OPCODES;
> + /* No small sector erase for 4-byte command set */
> + nor->erase_opcode = SPINOR_OP_SE;
> + nor->mtd.erasesize = nor->info->sector_size;
> + }
> }
>
> static const struct spi_nor_fixups spansion_fixups = {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init()
2022-02-10 3:26 ` Tudor.Ambarus
@ 2022-02-10 8:16 ` Michael Walle
2022-02-10 8:42 ` Tudor.Ambarus
2022-02-14 18:59 ` Pratyush Yadav
1 sibling, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-10 8:16 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
Am 2022-02-10 04:26, schrieb Tudor.Ambarus@microchip.com:
> On 2/2/22 16:58, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Increase readability of the code. Instead of returning early if the
>> flash size is smaller or equal than 16MiB and then do the fixups for
>> larger flashes, do it within the condition.
>>
>
> mm, no, I'm not sure this improves readability, I see the two
> equivalent.
> The original version has the benefit of no indentation. Pratyush?
This is a preparation patch for 12/14, where the current version isn't
working anyway. If that is not enough reason why this is bad IMHO, I'll
give you two more.
I'd agree with you if that function was called
spansion_late_init_smaller_flashes() or something like that. But it is
a generic function valid for all flashes. And if you read it you might
get the impression there are only flashes smaller or equal than 16MiB.
You have to look twice to notice it was the intention that the
assignment afterwards are just for the smaller flashes (and you will
need to notice that there aren't any assignments for all spansion
flashes). There is no direct connection between the assignment and
the condition. Whereas with
if (condition) {
some_action();
}
It is clear that some_action() was intended to only execute if
condition is true.
Also - and that is worse IMHO - it might easily be missed as someone
just add stuff to the end of the function which might goes unnoticed
but it won't work for flashes >16MiB.
-michael
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init()
2022-02-10 8:16 ` Michael Walle
@ 2022-02-10 8:42 ` Tudor.Ambarus
0 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 8:42 UTC (permalink / raw)
To: michael
Cc: linux-mtd, linux-kernel, p.yadav, miquel.raynal, richard, vigneshr
On 2/10/22 10:16, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am 2022-02-10 04:26, schrieb Tudor.Ambarus@microchip.com:
>> On 2/2/22 16:58, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Increase readability of the code. Instead of returning early if the
>>> flash size is smaller or equal than 16MiB and then do the fixups for
>>> larger flashes, do it within the condition.
>>>
>>
>> mm, no, I'm not sure this improves readability, I see the two
>> equivalent.
>> The original version has the benefit of no indentation. Pratyush?
>
> This is a preparation patch for 12/14, where the current version isn't
> working anyway. If that is not enough reason why this is bad IMHO, I'll
> give you two more.
you can put the
+ if (nor->flags & SNOR_F_USE_CLSR)
+ nor->params->ready = spi_nor_sr_ready_and_clear;
above the size check and get rid of the prerequisite requirement, no?
But it will look ugly indeed.
If these two are so tightly related, how about squashing them?
>
> I'd agree with you if that function was called
> spansion_late_init_smaller_flashes() or something like that. But it is
> a generic function valid for all flashes. And if you read it you might
> get the impression there are only flashes smaller or equal than 16MiB.
> You have to look twice to notice it was the intention that the
> assignment afterwards are just for the smaller flashes (and you will
> need to notice that there aren't any assignments for all spansion
> flashes). There is no direct connection between the assignment and
> the condition. Whereas with
> if (condition) {
> some_action();
> }
> It is clear that some_action() was intended to only execute if
> condition is true.
>
> Also - and that is worse IMHO - it might easily be missed as someone
> just add stuff to the end of the function which might goes unnoticed
> but it won't work for flashes >16MiB.
>
You definitely care about it if you wrote such a long email :). I find
the first argument the strongest, these two are biased IMO. I'm waiting
for v2 with this change included! :)
Cheers,
ta
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init()
2022-02-10 3:26 ` Tudor.Ambarus
2022-02-10 8:16 ` Michael Walle
@ 2022-02-14 18:59 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-14 18:59 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:26AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Increase readability of the code. Instead of returning early if the
> > flash size is smaller or equal than 16MiB and then do the fixups for
> > larger flashes, do it within the condition.
> >
>
> mm, no, I'm not sure this improves readability, I see the two equivalent.
> The original version has the benefit of no indentation. Pratyush?
I am fine with both to be honest. But Michael's reasoning does make some
sense to me. So,
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (10 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 11/14] mtd: spi-nor: spansion: slightly rework control flow in late_init() Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-02 18:15 ` kernel test robot
` (4 more replies)
2022-02-02 14:58 ` [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag Michael Walle
` (3 subsequent siblings)
15 siblings, 5 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
The clear status register flags is only available on spansion flashes.
Move all the functions around that into the spanion module.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 52 +------------------------
drivers/mtd/spi-nor/spansion.c | 70 ++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 1 -
3 files changed, 72 insertions(+), 51 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index be65aaa954ca..5b00dfab77a6 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -554,33 +554,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
return ret;
}
-/**
- * spi_nor_clear_sr() - Clear the Status Register.
- * @nor: pointer to 'struct spi_nor'.
- */
-static void spi_nor_clear_sr(struct spi_nor *nor)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_NO_DATA);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
- NULL, 0);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d clearing SR\n", ret);
-}
-
/**
* spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
* for new commands.
@@ -590,33 +563,12 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
*/
int spi_nor_sr_ready(struct spi_nor *nor)
{
- int ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ int ret;
+ ret = spi_nor_read_sr(nor, nor->bouncebuf);
if (ret)
return ret;
- if (nor->flags & SNOR_F_USE_CLSR &&
- nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
- if (nor->bouncebuf[0] & SR_E_ERR)
- dev_err(nor->dev, "Erase Error occurred\n");
- else
- dev_err(nor->dev, "Programming Error occurred\n");
-
- spi_nor_clear_sr(nor);
-
- /*
- * WEL bit remains set to one when an erase or page program
- * error occurs. Issue a Write Disable command to protect
- * against inadvertent writes that can possibly corrupt the
- * contents of the memory.
- */
- ret = spi_nor_write_disable(nor);
- if (ret)
- return ret;
-
- return -EIO;
- }
-
return !(nor->bouncebuf[0] & SR_WIP);
}
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index dedc2de90cb8..4756fb88eab2 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -8,6 +8,7 @@
#include "core.h"
+#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
#define SPINOR_REG_CYPRESS_CFR2V 0x00800003
@@ -294,6 +295,72 @@ static const struct flash_info spansion_parts[] = {
},
};
+/**
+ * spi_nor_clear_sr() - Clear the Status Register.
+ * @nor: pointer to 'struct spi_nor'.
+ */
+static void spi_nor_clear_sr(struct spi_nor *nor)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_DATA);
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
+ NULL, 0);
+ }
+
+ if (ret)
+ dev_dbg(nor->dev, "error %d clearing SR\n", ret);
+}
+
+/**
+ * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
+ * is ready for new commands and clear it.
+ * @nor: pointer to 'struct spi_nor'.
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
+{
+ int ret;
+
+ ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ if (ret)
+ return ret;
+
+ if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
+ if (nor->bouncebuf[0] & SR_E_ERR)
+ dev_err(nor->dev, "Erase Error occurred\n");
+ else
+ dev_err(nor->dev, "Programming Error occurred\n");
+
+ spi_nor_clear_sr(nor);
+
+ /*
+ * WEL bit remains set to one when an erase or page program
+ * error occurs. Issue a Write Disable command to protect
+ * against inadvertent writes that can possibly corrupt the
+ * contents of the memory.
+ */
+ ret = spi_nor_write_disable(nor);
+ if (ret)
+ return ret;
+
+ return -EIO;
+ }
+
+ return !(nor->bouncebuf[0] & SR_WIP);
+}
+
static void spansion_late_init(struct spi_nor *nor)
{
if (nor->params->size > SZ_16M) {
@@ -302,6 +369,9 @@ static void spansion_late_init(struct spi_nor *nor)
nor->erase_opcode = SPINOR_OP_SE;
nor->mtd.erasesize = nor->info->sector_size;
}
+
+ if (nor->flags & SNOR_F_USE_CLSR)
+ nor->params->ready = spi_nor_sr_ready_and_clear;
}
static const struct spi_nor_fixups spansion_fixups = {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 4622251a79ff..5e25a7b75ae2 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -90,7 +90,6 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */
-#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
/* Used for Micron flashes only. */
#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
@ 2022-02-02 18:15 ` kernel test robot
2022-02-02 20:07 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 18:15 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
config: alpha-allmodconfig (https://download.01.org/0day-ci/archive/20220203/202202030228.8WUETAc8-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01f9808a20c96553c43f929e10f3fb448cd8bd93
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
git checkout 01f9808a20c96553c43f929e10f3fb448cd8bd93
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/mtd/spi-nor/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mtd/spi-nor/spansion.c:332:5: warning: no previous prototype for 'spi_nor_sr_ready_and_clear' [-Wmissing-prototypes]
332 | int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/spi_nor_sr_ready_and_clear +332 drivers/mtd/spi-nor/spansion.c
324
325 /**
326 * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
327 * is ready for new commands and clear it.
328 * @nor: pointer to 'struct spi_nor'.
329 *
330 * Return: 1 if ready, 0 if not ready, -errno on errors.
331 */
> 332 int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
333 {
334 int ret;
335
336 ret = spi_nor_read_sr(nor, nor->bouncebuf);
337 if (ret)
338 return ret;
339
340 if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
341 if (nor->bouncebuf[0] & SR_E_ERR)
342 dev_err(nor->dev, "Erase Error occurred\n");
343 else
344 dev_err(nor->dev, "Programming Error occurred\n");
345
346 spi_nor_clear_sr(nor);
347
348 /*
349 * WEL bit remains set to one when an erase or page program
350 * error occurs. Issue a Write Disable command to protect
351 * against inadvertent writes that can possibly corrupt the
352 * contents of the memory.
353 */
354 ret = spi_nor_write_disable(nor);
355 if (ret)
356 return ret;
357
358 return -EIO;
359 }
360
361 return !(nor->bouncebuf[0] & SR_WIP);
362 }
363
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
2022-02-02 18:15 ` kernel test robot
@ 2022-02-02 20:07 ` kernel test robot
2022-02-02 21:38 ` [RFC PATCH] mtd: spi-nor: spi_nor_sr_ready_and_clear() can be static kernel test robot
` (2 subsequent siblings)
4 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 20:07 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: llvm, kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20220203/202202030449.IQfj2Y5O-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01f9808a20c96553c43f929e10f3fb448cd8bd93
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
git checkout 01f9808a20c96553c43f929e10f3fb448cd8bd93
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mtd/spi-nor/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mtd/spi-nor/spansion.c:332:5: warning: no previous prototype for function 'spi_nor_sr_ready_and_clear' [-Wmissing-prototypes]
int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
^
drivers/mtd/spi-nor/spansion.c:332:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
^
static
1 warning generated.
vim +/spi_nor_sr_ready_and_clear +332 drivers/mtd/spi-nor/spansion.c
324
325 /**
326 * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
327 * is ready for new commands and clear it.
328 * @nor: pointer to 'struct spi_nor'.
329 *
330 * Return: 1 if ready, 0 if not ready, -errno on errors.
331 */
> 332 int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
333 {
334 int ret;
335
336 ret = spi_nor_read_sr(nor, nor->bouncebuf);
337 if (ret)
338 return ret;
339
340 if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
341 if (nor->bouncebuf[0] & SR_E_ERR)
342 dev_err(nor->dev, "Erase Error occurred\n");
343 else
344 dev_err(nor->dev, "Programming Error occurred\n");
345
346 spi_nor_clear_sr(nor);
347
348 /*
349 * WEL bit remains set to one when an erase or page program
350 * error occurs. Issue a Write Disable command to protect
351 * against inadvertent writes that can possibly corrupt the
352 * contents of the memory.
353 */
354 ret = spi_nor_write_disable(nor);
355 if (ret)
356 return ret;
357
358 return -EIO;
359 }
360
361 return !(nor->bouncebuf[0] & SR_WIP);
362 }
363
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 60+ messages in thread
* [RFC PATCH] mtd: spi-nor: spi_nor_sr_ready_and_clear() can be static
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
2022-02-02 18:15 ` kernel test robot
2022-02-02 20:07 ` kernel test robot
@ 2022-02-02 21:38 ` kernel test robot
2022-02-02 21:48 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c kernel test robot
2022-02-10 3:32 ` Tudor.Ambarus
4 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 21:38 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
drivers/mtd/spi-nor/spansion.c:332:5: warning: symbol 'spi_nor_sr_ready_and_clear' was not declared. Should it be static?
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
spansion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 4756fb88eab232..242d55e043f08d 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -329,7 +329,7 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
*
* Return: 1 if ready, 0 if not ready, -errno on errors.
*/
-int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
+static int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
{
int ret;
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
` (2 preceding siblings ...)
2022-02-02 21:38 ` [RFC PATCH] mtd: spi-nor: spi_nor_sr_ready_and_clear() can be static kernel test robot
@ 2022-02-02 21:48 ` kernel test robot
2022-02-10 3:32 ` Tudor.Ambarus
4 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-02-02 21:48 UTC (permalink / raw)
To: Michael Walle, linux-mtd, linux-kernel
Cc: kbuild-all, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Michael Walle
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
config: x86_64-randconfig-s021 (https://download.01.org/0day-ci/archive/20220203/202202030556.9u7o7D18-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/01f9808a20c96553c43f929e10f3fb448cd8bd93
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/mtd-spi-nor-move-vendor-specific-code-into-vendor-modules/20220202-230139
git checkout 01f9808a20c96553c43f929e10f3fb448cd8bd93
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mtd/spi-nor/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/mtd/spi-nor/spansion.c:332:5: sparse: sparse: symbol 'spi_nor_sr_ready_and_clear' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
` (3 preceding siblings ...)
2022-02-02 21:48 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c kernel test robot
@ 2022-02-10 3:32 ` Tudor.Ambarus
2022-02-15 19:23 ` Pratyush Yadav
4 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:32 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The clear status register flags is only available on spansion flashes.
> Move all the functions around that into the spanion module.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 52 +------------------------
> drivers/mtd/spi-nor/spansion.c | 70 ++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 1 -
> 3 files changed, 72 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index be65aaa954ca..5b00dfab77a6 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -554,33 +554,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
> return ret;
> }
>
> -/**
> - * spi_nor_clear_sr() - Clear the Status Register.
> - * @nor: pointer to 'struct spi_nor'.
> - */
> -static void spi_nor_clear_sr(struct spi_nor *nor)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op =
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> - SPI_MEM_OP_NO_ADDR,
> - SPI_MEM_OP_NO_DUMMY,
> - SPI_MEM_OP_NO_DATA);
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> - NULL, 0);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> -}
> -
> /**
> * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
> * for new commands.
> @@ -590,33 +563,12 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
> */
> int spi_nor_sr_ready(struct spi_nor *nor)
> {
> - int ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + int ret;
>
> + ret = spi_nor_read_sr(nor, nor->bouncebuf);
> if (ret)
> return ret;
:) don't change style for no reason. What's wrong with the previous version?
Anyway, with the reports fixed and no hidden style changes, it looks good to me.
>
> - if (nor->flags & SNOR_F_USE_CLSR &&
> - nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> - if (nor->bouncebuf[0] & SR_E_ERR)
> - dev_err(nor->dev, "Erase Error occurred\n");
> - else
> - dev_err(nor->dev, "Programming Error occurred\n");
> -
> - spi_nor_clear_sr(nor);
> -
> - /*
> - * WEL bit remains set to one when an erase or page program
> - * error occurs. Issue a Write Disable command to protect
> - * against inadvertent writes that can possibly corrupt the
> - * contents of the memory.
> - */
> - ret = spi_nor_write_disable(nor);
> - if (ret)
> - return ret;
> -
> - return -EIO;
> - }
> -
> return !(nor->bouncebuf[0] & SR_WIP);
> }
>
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index dedc2de90cb8..4756fb88eab2 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,7 @@
>
> #include "core.h"
>
> +#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
> #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> #define SPINOR_REG_CYPRESS_CFR2V 0x00800003
> @@ -294,6 +295,72 @@ static const struct flash_info spansion_parts[] = {
> },
> };
>
> +/**
> + * spi_nor_clear_sr() - Clear the Status Register.
> + * @nor: pointer to 'struct spi_nor'.
> + */
> +static void spi_nor_clear_sr(struct spi_nor *nor)
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_NO_DATA);
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> + NULL, 0);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> +}
> +
> +/**
> + * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
> + * is ready for new commands and clear it.
> + * @nor: pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
> +{
> + int ret;
> +
> + ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + if (ret)
> + return ret;
> +
> + if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> + if (nor->bouncebuf[0] & SR_E_ERR)
> + dev_err(nor->dev, "Erase Error occurred\n");
> + else
> + dev_err(nor->dev, "Programming Error occurred\n");
> +
> + spi_nor_clear_sr(nor);
> +
> + /*
> + * WEL bit remains set to one when an erase or page program
> + * error occurs. Issue a Write Disable command to protect
> + * against inadvertent writes that can possibly corrupt the
> + * contents of the memory.
> + */
> + ret = spi_nor_write_disable(nor);
> + if (ret)
> + return ret;
> +
> + return -EIO;
> + }
> +
> + return !(nor->bouncebuf[0] & SR_WIP);
> +}
> +
> static void spansion_late_init(struct spi_nor *nor)
> {
> if (nor->params->size > SZ_16M) {
> @@ -302,6 +369,9 @@ static void spansion_late_init(struct spi_nor *nor)
> nor->erase_opcode = SPINOR_OP_SE;
> nor->mtd.erasesize = nor->info->sector_size;
> }
> +
> + if (nor->flags & SNOR_F_USE_CLSR)
> + nor->params->ready = spi_nor_sr_ready_and_clear;
> }
>
> static const struct spi_nor_fixups spansion_fixups = {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4622251a79ff..5e25a7b75ae2 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -90,7 +90,6 @@
>
> /* Used for Spansion flashes only. */
> #define SPINOR_OP_BRWR 0x17 /* Bank register write */
> -#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
>
> /* Used for Micron flashes only. */
> #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c
2022-02-10 3:32 ` Tudor.Ambarus
@ 2022-02-15 19:23 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:23 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:32AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > The clear status register flags is only available on spansion flashes.
> > Move all the functions around that into the spanion module.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > drivers/mtd/spi-nor/core.c | 52 +------------------------
> > drivers/mtd/spi-nor/spansion.c | 70 ++++++++++++++++++++++++++++++++++
> > include/linux/mtd/spi-nor.h | 1 -
> > 3 files changed, 72 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index be65aaa954ca..5b00dfab77a6 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -554,33 +554,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
> > return ret;
> > }
> >
> > -/**
> > - * spi_nor_clear_sr() - Clear the Status Register.
> > - * @nor: pointer to 'struct spi_nor'.
> > - */
> > -static void spi_nor_clear_sr(struct spi_nor *nor)
> > -{
> > - int ret;
> > -
> > - if (nor->spimem) {
> > - struct spi_mem_op op =
> > - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> > - SPI_MEM_OP_NO_ADDR,
> > - SPI_MEM_OP_NO_DUMMY,
> > - SPI_MEM_OP_NO_DATA);
> > -
> > - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> > -
> > - ret = spi_mem_exec_op(nor->spimem, &op);
> > - } else {
> > - ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> > - NULL, 0);
> > - }
> > -
> > - if (ret)
> > - dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> > -}
> > -
> > /**
> > * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
> > * for new commands.
> > @@ -590,33 +563,12 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
> > */
> > int spi_nor_sr_ready(struct spi_nor *nor)
> > {
> > - int ret = spi_nor_read_sr(nor, nor->bouncebuf);
> > + int ret;
> >
> > + ret = spi_nor_read_sr(nor, nor->bouncebuf);
> > if (ret)
> > return ret;
>
> :) don't change style for no reason. What's wrong with the previous version?
FWIW, I like the newer style better. But that should come in a separate
patch in either case.
>
> Anyway, with the reports fixed and no hidden style changes, it looks good to me.
>
> >
> > - if (nor->flags & SNOR_F_USE_CLSR &&
> > - nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> > - if (nor->bouncebuf[0] & SR_E_ERR)
> > - dev_err(nor->dev, "Erase Error occurred\n");
> > - else
> > - dev_err(nor->dev, "Programming Error occurred\n");
> > -
> > - spi_nor_clear_sr(nor);
> > -
> > - /*
> > - * WEL bit remains set to one when an erase or page program
> > - * error occurs. Issue a Write Disable command to protect
> > - * against inadvertent writes that can possibly corrupt the
> > - * contents of the memory.
> > - */
> > - ret = spi_nor_write_disable(nor);
> > - if (ret)
> > - return ret;
> > -
> > - return -EIO;
> > - }
> > -
> > return !(nor->bouncebuf[0] & SR_WIP);
> > }
> >
> > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> > index dedc2de90cb8..4756fb88eab2 100644
> > --- a/drivers/mtd/spi-nor/spansion.c
> > +++ b/drivers/mtd/spi-nor/spansion.c
> > @@ -8,6 +8,7 @@
> >
> > #include "core.h"
> >
> > +#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
> > #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> > #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> > #define SPINOR_REG_CYPRESS_CFR2V 0x00800003
> > @@ -294,6 +295,72 @@ static const struct flash_info spansion_parts[] = {
> > },
> > };
> >
> > +/**
> > + * spi_nor_clear_sr() - Clear the Status Register.
> > + * @nor: pointer to 'struct spi_nor'.
> > + */
> > +static void spi_nor_clear_sr(struct spi_nor *nor)
> > +{
> > + int ret;
> > +
> > + if (nor->spimem) {
> > + struct spi_mem_op op =
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> > + SPI_MEM_OP_NO_ADDR,
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_NO_DATA);
> > +
> > + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> > +
> > + ret = spi_mem_exec_op(nor->spimem, &op);
> > + } else {
> > + ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> > + NULL, 0);
> > + }
> > +
> > + if (ret)
> > + dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> > +}
> > +
> > +/**
> > + * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
> > + * is ready for new commands and clear it.
> > + * @nor: pointer to 'struct spi_nor'.
> > + *
> > + * Return: 1 if ready, 0 if not ready, -errno on errors.
> > + */
> > +int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
Make it static.
[...]
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (11 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:34 ` Tudor.Ambarus
2022-02-02 14:58 ` [PATCH v1 14/14] mtd: spi-nor: renumber flags Michael Walle
` (2 subsequent siblings)
15 siblings, 1 reply; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Now that all functions using that flag are local to the spanion module,
we can convert the flag to a manufacturer one.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.c | 3 --
drivers/mtd/spi-nor/core.h | 3 --
drivers/mtd/spi-nor/spansion.c | 54 +++++++++++++++++++++-------------
3 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5b00dfab77a6..2d5517b3db96 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2448,9 +2448,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
if (flags & NO_CHIP_ERASE)
nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
-
- if (flags & USE_CLSR)
- nor->flags |= SNOR_F_USE_CLSR;
}
/**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index a02bf54289fb..2130a96e2044 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -14,7 +14,6 @@
enum spi_nor_option_flags {
SNOR_F_HAS_SR_TB = BIT(1),
SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
- SNOR_F_USE_CLSR = BIT(4),
SNOR_F_BROKEN_RESET = BIT(5),
SNOR_F_4B_OPCODES = BIT(6),
SNOR_F_HAS_4BAIT = BIT(7),
@@ -347,7 +346,6 @@ struct spi_nor_fixups {
* SPI_NOR_NO_ERASE: no erase command needed.
* NO_CHIP_ERASE: chip does not support chip erase.
* SPI_NOR_NO_FR: can't do fastread.
- * USE_CLSR: use CLSR command.
*
* @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
* Used when SFDP tables are not defined in the flash. These
@@ -398,7 +396,6 @@ struct flash_info {
#define SPI_NOR_NO_ERASE BIT(6)
#define NO_CHIP_ERASE BIT(7)
#define SPI_NOR_NO_FR BIT(8)
-#define USE_CLSR BIT(9)
u8 no_sfdp_flags;
#define SPI_NOR_SKIP_SFDP BIT(0)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 4756fb88eab2..c31ea11f71f2 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -8,6 +8,8 @@
#include "core.h"
+#define USE_CLSR BIT(0)
+
#define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
@@ -212,43 +214,53 @@ static const struct flash_info spansion_parts[] = {
{ "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128)
NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl128s0", INFO6(0x012018, 0x4d0080, 256 * 1024, 64)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fl256s0", INFO6(0x010219, 0x4d0080, 256 * 1024, 128)
- FLAGS(USE_CLSR)
NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SPI_NOR_DUAL_READ |
- SPI_NOR_QUAD_READ) },
+ SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fl256s1", INFO6(0x010219, 0x4d0180, 64 * 1024, 512)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256)
- FLAGS(SPI_NOR_HAS_LOCK | USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ FLAGS(SPI_NOR_HAS_LOCK)
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fs128s1", INFO6(0x012018, 0x4d0181, 64 * 1024, 256)
- FLAGS(USE_CLSR)
NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
.fixups = &s25fs_s_fixups, },
{ "s25fs256s0", INFO6(0x010219, 0x4d0081, 256 * 1024, 128)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fs512s", INFO6(0x010220, 0x4d0081, 256 * 1024, 256)
- FLAGS(USE_CLSR)
NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
.fixups = &s25fs_s_fixups, },
{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64) },
{ "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256) },
{ "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256)
- FLAGS(USE_CLSR)
- NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+ NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ MFR_FLAGS(USE_CLSR)
+ },
{ "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8) },
{ "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16) },
{ "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32) },
@@ -370,7 +382,7 @@ static void spansion_late_init(struct spi_nor *nor)
nor->mtd.erasesize = nor->info->sector_size;
}
- if (nor->flags & SNOR_F_USE_CLSR)
+ if (nor->info->mfr_flags & USE_CLSR)
nor->params->ready = spi_nor_sr_ready_and_clear;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag
2022-02-02 14:58 ` [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag Michael Walle
@ 2022-02-10 3:34 ` Tudor.Ambarus
2022-02-15 19:25 ` Pratyush Yadav
0 siblings, 1 reply; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:34 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Now that all functions using that flag are local to the spanion module,
> we can convert the flag to a manufacturer one.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/mtd/spi-nor/core.c | 3 --
> drivers/mtd/spi-nor/core.h | 3 --
> drivers/mtd/spi-nor/spansion.c | 54 +++++++++++++++++++++-------------
> 3 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5b00dfab77a6..2d5517b3db96 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2448,9 +2448,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
>
> if (flags & NO_CHIP_ERASE)
> nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
> -
> - if (flags & USE_CLSR)
> - nor->flags |= SNOR_F_USE_CLSR;
> }
>
> /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index a02bf54289fb..2130a96e2044 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -14,7 +14,6 @@
> enum spi_nor_option_flags {
> SNOR_F_HAS_SR_TB = BIT(1),
> SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> - SNOR_F_USE_CLSR = BIT(4),
> SNOR_F_BROKEN_RESET = BIT(5),
> SNOR_F_4B_OPCODES = BIT(6),
> SNOR_F_HAS_4BAIT = BIT(7),
> @@ -347,7 +346,6 @@ struct spi_nor_fixups {
> * SPI_NOR_NO_ERASE: no erase command needed.
> * NO_CHIP_ERASE: chip does not support chip erase.
> * SPI_NOR_NO_FR: can't do fastread.
> - * USE_CLSR: use CLSR command.
> *
> * @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
> * Used when SFDP tables are not defined in the flash. These
> @@ -398,7 +396,6 @@ struct flash_info {
> #define SPI_NOR_NO_ERASE BIT(6)
> #define NO_CHIP_ERASE BIT(7)
> #define SPI_NOR_NO_FR BIT(8)
> -#define USE_CLSR BIT(9)
>
> u8 no_sfdp_flags;
> #define SPI_NOR_SKIP_SFDP BIT(0)
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 4756fb88eab2..c31ea11f71f2 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,8 @@
>
> #include "core.h"
>
> +#define USE_CLSR BIT(0)
add a description, tell the reader this is a manufacturer specific flag.
excellent work:
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> +
> #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
> #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> @@ -212,43 +214,53 @@ static const struct flash_info spansion_parts[] = {
> { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128)
> NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> { "s25fl128s0", INFO6(0x012018, 0x4d0080, 256 * 1024, 64)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fl256s0", INFO6(0x010219, 0x4d0080, 256 * 1024, 128)
> - FLAGS(USE_CLSR)
> NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SPI_NOR_DUAL_READ |
> - SPI_NOR_QUAD_READ) },
> + SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fl256s1", INFO6(0x010219, 0x4d0180, 64 * 1024, 512)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256)
> - FLAGS(SPI_NOR_HAS_LOCK | USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + FLAGS(SPI_NOR_HAS_LOCK)
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fs128s1", INFO6(0x012018, 0x4d0181, 64 * 1024, 256)
> - FLAGS(USE_CLSR)
> NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> .fixups = &s25fs_s_fixups, },
> { "s25fs256s0", INFO6(0x010219, 0x4d0081, 256 * 1024, 128)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fs512s", INFO6(0x010220, 0x4d0081, 256 * 1024, 256)
> - FLAGS(USE_CLSR)
> NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> .fixups = &s25fs_s_fixups, },
> { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64) },
> { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256) },
> { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256)
> - FLAGS(USE_CLSR)
> - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> + MFR_FLAGS(USE_CLSR)
> + },
> { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8) },
> { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16) },
> { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32) },
> @@ -370,7 +382,7 @@ static void spansion_late_init(struct spi_nor *nor)
> nor->mtd.erasesize = nor->info->sector_size;
> }
>
> - if (nor->flags & SNOR_F_USE_CLSR)
> + if (nor->info->mfr_flags & USE_CLSR)
> nor->params->ready = spi_nor_sr_ready_and_clear;
> }
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag
2022-02-10 3:34 ` Tudor.Ambarus
@ 2022-02-15 19:25 ` Pratyush Yadav
0 siblings, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:25 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: michael, linux-mtd, linux-kernel, miquel.raynal, richard, vigneshr
On 10/02/22 03:34AM, Tudor.Ambarus@microchip.com wrote:
> On 2/2/22 16:58, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Now that all functions using that flag are local to the spanion module,
s/spanion/spansion/
> > we can convert the flag to a manufacturer one.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > drivers/mtd/spi-nor/core.c | 3 --
> > drivers/mtd/spi-nor/core.h | 3 --
> > drivers/mtd/spi-nor/spansion.c | 54 +++++++++++++++++++++-------------
> > 3 files changed, 33 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 5b00dfab77a6..2d5517b3db96 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2448,9 +2448,6 @@ static void spi_nor_init_flags(struct spi_nor *nor)
> >
> > if (flags & NO_CHIP_ERASE)
> > nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
> > -
> > - if (flags & USE_CLSR)
> > - nor->flags |= SNOR_F_USE_CLSR;
> > }
> >
> > /**
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index a02bf54289fb..2130a96e2044 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -14,7 +14,6 @@
> > enum spi_nor_option_flags {
> > SNOR_F_HAS_SR_TB = BIT(1),
> > SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> > - SNOR_F_USE_CLSR = BIT(4),
> > SNOR_F_BROKEN_RESET = BIT(5),
> > SNOR_F_4B_OPCODES = BIT(6),
> > SNOR_F_HAS_4BAIT = BIT(7),
> > @@ -347,7 +346,6 @@ struct spi_nor_fixups {
> > * SPI_NOR_NO_ERASE: no erase command needed.
> > * NO_CHIP_ERASE: chip does not support chip erase.
> > * SPI_NOR_NO_FR: can't do fastread.
> > - * USE_CLSR: use CLSR command.
> > *
> > * @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
> > * Used when SFDP tables are not defined in the flash. These
> > @@ -398,7 +396,6 @@ struct flash_info {
> > #define SPI_NOR_NO_ERASE BIT(6)
> > #define NO_CHIP_ERASE BIT(7)
> > #define SPI_NOR_NO_FR BIT(8)
> > -#define USE_CLSR BIT(9)
> >
> > u8 no_sfdp_flags;
> > #define SPI_NOR_SKIP_SFDP BIT(0)
> > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> > index 4756fb88eab2..c31ea11f71f2 100644
> > --- a/drivers/mtd/spi-nor/spansion.c
> > +++ b/drivers/mtd/spi-nor/spansion.c
> > @@ -8,6 +8,8 @@
> >
> > #include "core.h"
> >
> > +#define USE_CLSR BIT(0)
>
> add a description, tell the reader this is a manufacturer specific flag.
+1
> excellent work:
+1
>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> > +
> > #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
> > #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> > #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> > @@ -212,43 +214,53 @@ static const struct flash_info spansion_parts[] = {
> > { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128)
> > NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > { "s25fl128s0", INFO6(0x012018, 0x4d0080, 256 * 1024, 64)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fl256s0", INFO6(0x010219, 0x4d0080, 256 * 1024, 128)
> > - FLAGS(USE_CLSR)
> > NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SPI_NOR_DUAL_READ |
> > - SPI_NOR_QUAD_READ) },
> > + SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fl256s1", INFO6(0x010219, 0x4d0180, 64 * 1024, 512)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256)
> > - FLAGS(SPI_NOR_HAS_LOCK | USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + FLAGS(SPI_NOR_HAS_LOCK)
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fs128s1", INFO6(0x012018, 0x4d0181, 64 * 1024, 256)
> > - FLAGS(USE_CLSR)
> > NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > .fixups = &s25fs_s_fixups, },
> > { "s25fs256s0", INFO6(0x010219, 0x4d0081, 256 * 1024, 128)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fs512s", INFO6(0x010220, 0x4d0081, 256 * 1024, 256)
> > - FLAGS(USE_CLSR)
> > NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > .fixups = &s25fs_s_fixups, },
> > { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64) },
> > { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256) },
> > { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256)
> > - FLAGS(USE_CLSR)
> > - NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > + NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > + MFR_FLAGS(USE_CLSR)
> > + },
> > { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8) },
> > { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16) },
> > { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32) },
> > @@ -370,7 +382,7 @@ static void spansion_late_init(struct spi_nor *nor)
> > nor->mtd.erasesize = nor->info->sector_size;
> > }
> >
> > - if (nor->flags & SNOR_F_USE_CLSR)
> > + if (nor->info->mfr_flags & USE_CLSR)
> > nor->params->ready = spi_nor_sr_ready_and_clear;
> > }
> >
> > --
> > 2.30.2
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v1 14/14] mtd: spi-nor: renumber flags
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (12 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 13/14] mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag Michael Walle
@ 2022-02-02 14:58 ` Michael Walle
2022-02-10 3:38 ` Tudor.Ambarus
2022-02-15 19:27 ` Pratyush Yadav
2022-02-10 3:42 ` [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Tudor.Ambarus
2022-02-17 7:31 ` Tudor.Ambarus
15 siblings, 2 replies; 60+ messages in thread
From: Michael Walle @ 2022-02-02 14:58 UTC (permalink / raw)
To: linux-mtd, linux-kernel
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
As we have deleted some flag, lets renumber them so there are no holes.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/mtd/spi-nor/core.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 2130a96e2044..b7fd760e3b47 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -12,20 +12,20 @@
#define SPI_NOR_MAX_ID_LEN 6
enum spi_nor_option_flags {
- SNOR_F_HAS_SR_TB = BIT(1),
- SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
- SNOR_F_BROKEN_RESET = BIT(5),
- SNOR_F_4B_OPCODES = BIT(6),
- SNOR_F_HAS_4BAIT = BIT(7),
- SNOR_F_HAS_LOCK = BIT(8),
- SNOR_F_HAS_16BIT_SR = BIT(9),
- SNOR_F_NO_READ_CR = BIT(10),
- SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
- SNOR_F_HAS_4BIT_BP = BIT(12),
- SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
- SNOR_F_IO_MODE_EN_VOLATILE = BIT(14),
- SNOR_F_SOFT_RESET = BIT(15),
- SNOR_F_SWP_IS_VOLATILE = BIT(16),
+ SNOR_F_HAS_SR_TB = BIT(0),
+ SNOR_F_NO_OP_CHIP_ERASE = BIT(1),
+ SNOR_F_BROKEN_RESET = BIT(2),
+ SNOR_F_4B_OPCODES = BIT(3),
+ SNOR_F_HAS_4BAIT = BIT(4),
+ SNOR_F_HAS_LOCK = BIT(5),
+ SNOR_F_HAS_16BIT_SR = BIT(6),
+ SNOR_F_NO_READ_CR = BIT(7),
+ SNOR_F_HAS_SR_TB_BIT6 = BIT(8),
+ SNOR_F_HAS_4BIT_BP = BIT(9),
+ SNOR_F_HAS_SR_BP3_BIT6 = BIT(10),
+ SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
+ SNOR_F_SOFT_RESET = BIT(12),
+ SNOR_F_SWP_IS_VOLATILE = BIT(13),
};
struct spi_nor_read_command {
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v1 14/14] mtd: spi-nor: renumber flags
2022-02-02 14:58 ` [PATCH v1 14/14] mtd: spi-nor: renumber flags Michael Walle
@ 2022-02-10 3:38 ` Tudor.Ambarus
2022-02-15 19:27 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:38 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> As we have deleted some flag, lets renumber them so there are no holes.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.h | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 2130a96e2044..b7fd760e3b47 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -12,20 +12,20 @@
> #define SPI_NOR_MAX_ID_LEN 6
>
> enum spi_nor_option_flags {
> - SNOR_F_HAS_SR_TB = BIT(1),
> - SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
> - SNOR_F_BROKEN_RESET = BIT(5),
> - SNOR_F_4B_OPCODES = BIT(6),
> - SNOR_F_HAS_4BAIT = BIT(7),
> - SNOR_F_HAS_LOCK = BIT(8),
> - SNOR_F_HAS_16BIT_SR = BIT(9),
> - SNOR_F_NO_READ_CR = BIT(10),
> - SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
> - SNOR_F_HAS_4BIT_BP = BIT(12),
> - SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
> - SNOR_F_IO_MODE_EN_VOLATILE = BIT(14),
> - SNOR_F_SOFT_RESET = BIT(15),
> - SNOR_F_SWP_IS_VOLATILE = BIT(16),
> + SNOR_F_HAS_SR_TB = BIT(0),
> + SNOR_F_NO_OP_CHIP_ERASE = BIT(1),
> + SNOR_F_BROKEN_RESET = BIT(2),
> + SNOR_F_4B_OPCODES = BIT(3),
> + SNOR_F_HAS_4BAIT = BIT(4),
> + SNOR_F_HAS_LOCK = BIT(5),
> + SNOR_F_HAS_16BIT_SR = BIT(6),
> + SNOR_F_NO_READ_CR = BIT(7),
> + SNOR_F_HAS_SR_TB_BIT6 = BIT(8),
> + SNOR_F_HAS_4BIT_BP = BIT(9),
> + SNOR_F_HAS_SR_BP3_BIT6 = BIT(10),
> + SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
> + SNOR_F_SOFT_RESET = BIT(12),
> + SNOR_F_SWP_IS_VOLATILE = BIT(13),
> };
>
> struct spi_nor_read_command {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 14/14] mtd: spi-nor: renumber flags
2022-02-02 14:58 ` [PATCH v1 14/14] mtd: spi-nor: renumber flags Michael Walle
2022-02-10 3:38 ` Tudor.Ambarus
@ 2022-02-15 19:27 ` Pratyush Yadav
1 sibling, 0 replies; 60+ messages in thread
From: Pratyush Yadav @ 2022-02-15 19:27 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, linux-kernel, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra
On 02/02/22 03:58PM, Michael Walle wrote:
> As we have deleted some flag, lets renumber them so there are no holes.
I was expecting you to forget about this and was about to comment this
in a previous patch. Good thing I looked ahead to double check :-)
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> Signed-off-by: Michael Walle <michael@walle.cc>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (13 preceding siblings ...)
2022-02-02 14:58 ` [PATCH v1 14/14] mtd: spi-nor: renumber flags Michael Walle
@ 2022-02-10 3:42 ` Tudor.Ambarus
2022-02-17 7:31 ` Tudor.Ambarus
15 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-10 3:42 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel
Cc: p.yadav, miquel.raynal, richard, vigneshr
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> It turns out that most of the special status register handling is
> specific for a particular vendor. I.e. Xilinx has some different
> opcodes for the status register read, Micron has an additional FSR
> register and Spansion has these flags integrated into the SR.
>
> Create a callback to ready() where a flash chip can register its
> own function. This will let us move all the vendor specific stuff
> out of the core into the vendor modules.
>
> Please note that this is only compile-time tested.
>
> For sake of consistency and better readability of the code flow,
> I also converted the setup() callback to be optional.
>
> Michael Walle (14):
> mtd: spi-nor: export more function to be used in vendor modules
> mtd: spi-nor: slightly refactor the spi_nor_setup()
> mtd: spi-nor: allow a flash to define its own ready() function
> mtd: spi-nor: move all xilinx specifics into xilinx.c
> mtd: spi-nor: xilinx: rename vendor specific functions and defines
> mtd: spi-nor: xilinx: correct the debug message
> mtd: spi-nor: move all micron-st specifics into micron-st.c
> mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag
> mtd: spi-nor: micron-st: fix micron_st prefix
> mtd: spi-nor: micron-st: rename vendor specific functions and defines
> mtd: spi-nor: spansion: slightly rework control flow in late_init()
> mtd: spi-nor: move all spansion specifics into spansion.c
> mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag
> mtd: spi-nor: renumber flags
>
> drivers/mtd/spi-nor/core.c | 265 ++------------------------------
> drivers/mtd/spi-nor/core.h | 70 ++++-----
> drivers/mtd/spi-nor/micron-st.c | 225 ++++++++++++++++++++++-----
> drivers/mtd/spi-nor/spansion.c | 133 ++++++++++++----
> drivers/mtd/spi-nor/xilinx.c | 79 +++++++++-
> include/linux/mtd/spi-nor.h | 18 ---
> 6 files changed, 417 insertions(+), 373 deletions(-)
>
Good job, Michael! I added R-b to some patches to inform you that they
look fine to me. You don't need to add the tags on the next version, as
I'll be the one that will apply them. I think I have a micron and a spansion
flash somewhere, will try to test your series once v2 is out.
Cheers,
ta
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules
2022-02-02 14:58 [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Michael Walle
` (14 preceding siblings ...)
2022-02-10 3:42 ` [PATCH v1 00/14] mtd: spi-nor: move vendor specific code into vendor modules Tudor.Ambarus
@ 2022-02-17 7:31 ` Tudor.Ambarus
15 siblings, 0 replies; 60+ messages in thread
From: Tudor.Ambarus @ 2022-02-17 7:31 UTC (permalink / raw)
To: michael, linux-mtd, linux-kernel, yaliang.wang
Cc: p.yadav, miquel.raynal, richard, vigneshr
+ Yaliang,
might be interested in reviewing/testing this, as there were some similar
patches submitted a while ago:
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=237043
Cheers,
ta
On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> It turns out that most of the special status register handling is
> specific for a particular vendor. I.e. Xilinx has some different
> opcodes for the status register read, Micron has an additional FSR
> register and Spansion has these flags integrated into the SR.
>
> Create a callback to ready() where a flash chip can register its
> own function. This will let us move all the vendor specific stuff
> out of the core into the vendor modules.
>
> Please note that this is only compile-time tested.
>
> For sake of consistency and better readability of the code flow,
> I also converted the setup() callback to be optional.
>
> Michael Walle (14):
> mtd: spi-nor: export more function to be used in vendor modules
> mtd: spi-nor: slightly refactor the spi_nor_setup()
> mtd: spi-nor: allow a flash to define its own ready() function
> mtd: spi-nor: move all xilinx specifics into xilinx.c
> mtd: spi-nor: xilinx: rename vendor specific functions and defines
> mtd: spi-nor: xilinx: correct the debug message
> mtd: spi-nor: move all micron-st specifics into micron-st.c
> mtd: spi-nor: micron-st: convert USE_FSR to a manufacturer flag
> mtd: spi-nor: micron-st: fix micron_st prefix
> mtd: spi-nor: micron-st: rename vendor specific functions and defines
> mtd: spi-nor: spansion: slightly rework control flow in late_init()
> mtd: spi-nor: move all spansion specifics into spansion.c
> mtd: spi-nor: spansion: convert USE_CLSR to a manufacturer flag
> mtd: spi-nor: renumber flags
>
> drivers/mtd/spi-nor/core.c | 265 ++------------------------------
> drivers/mtd/spi-nor/core.h | 70 ++++-----
> drivers/mtd/spi-nor/micron-st.c | 225 ++++++++++++++++++++++-----
> drivers/mtd/spi-nor/spansion.c | 133 ++++++++++++----
> drivers/mtd/spi-nor/xilinx.c | 79 +++++++++-
> include/linux/mtd/spi-nor.h | 18 ---
> 6 files changed, 417 insertions(+), 373 deletions(-)
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 60+ messages in thread