public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] mtd: spi-nor: Make octal_dtr_enable() dedicate for enabling Octal DTR
@ 2023-06-16  5:06 tkuw584924
  2023-07-13  6:43 ` Tudor Ambarus
  2023-07-14 15:07 ` [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR Tudor Ambarus
  0 siblings, 2 replies; 5+ messages in thread
From: tkuw584924 @ 2023-06-16  5:06 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, d-gole, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The function should be responsible only for enabling Octal DTR. Remove
'enable' parameter from octal_dtr_enable() and add octal_dtr_disable()
that takes care for disabling. This can remove 'enable' flag checks in
core and manufacturer drivers and improve readability.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c      | 42 ++++++++++++++++++++++++++-------
 drivers/mtd/spi-nor/core.h      |  4 +++-
 drivers/mtd/spi-nor/micron-st.c | 11 +++------
 drivers/mtd/spi-nor/spansion.c  | 36 ++++++++++++++--------------
 4 files changed, 57 insertions(+), 36 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 143ca3c9b477..d2571f309f41 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3086,11 +3086,10 @@ static int spi_nor_init_params(struct spi_nor *nor)
 
 /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
  * @nor:                 pointer to a 'struct spi_nor'
- * @enable:              whether to enable or disable Octal DTR
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+static int spi_nor_octal_dtr_enable(struct spi_nor *nor)
 {
 	int ret;
 
@@ -3104,14 +3103,39 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
 	if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
 		return 0;
 
-	ret = nor->params->octal_dtr_enable(nor, enable);
+	ret = nor->params->octal_dtr_enable(nor);
 	if (ret)
 		return ret;
 
-	if (enable)
-		nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
-	else
-		nor->reg_proto = SNOR_PROTO_1_1_1;
+	nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
+
+	return 0;
+}
+
+/** spi_nor_octal_dtr_disable() - disable Octal DTR I/O if it is enabled
+ * @nor:                 pointer to a 'struct spi_nor'
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_octal_dtr_disable(struct spi_nor *nor)
+{
+	int ret;
+
+	if (!nor->params->octal_dtr_disable)
+		return 0;
+
+	if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
+	      nor->write_proto == SNOR_PROTO_8_8_8_DTR))
+		return 0;
+
+	if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
+		return 0;
+
+	ret = nor->params->octal_dtr_disable(nor);
+	if (ret)
+		return ret;
+
+	nor->reg_proto = SNOR_PROTO_1_1_1;
 
 	return 0;
 }
@@ -3165,7 +3189,7 @@ static int spi_nor_init(struct spi_nor *nor)
 {
 	int err;
 
-	err = spi_nor_octal_dtr_enable(nor, true);
+	err = spi_nor_octal_dtr_enable(nor);
 	if (err) {
 		dev_dbg(nor->dev, "octal mode not supported\n");
 		return err;
@@ -3267,7 +3291,7 @@ static int spi_nor_suspend(struct mtd_info *mtd)
 	int ret;
 
 	/* Disable octal DTR mode if we enabled it. */
-	ret = spi_nor_octal_dtr_enable(nor, false);
+	ret = spi_nor_octal_dtr_disable(nor);
 	if (ret)
 		dev_err(nor->dev, "suspend() failed\n");
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index fd61c4793a10..9c33d7c8395b 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -364,6 +364,7 @@ struct spi_nor_otp {
  *                      Table.
  * @otp:		SPI NOR OTP info.
  * @octal_dtr_enable:	enables SPI NOR octal DTR mode.
+ * @octal_dtr_disable:	disables SPI NOR octal DTR mode.
  * @quad_enable:	enables SPI NOR quad mode.
  * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
  * @convert_addr:	converts an absolute address into something the flash
@@ -397,7 +398,8 @@ struct spi_nor_flash_parameter {
 	struct spi_nor_erase_map        erase_map;
 	struct spi_nor_otp		otp;
 
-	int (*octal_dtr_enable)(struct spi_nor *nor, bool enable);
+	int (*octal_dtr_enable)(struct spi_nor *nor);
+	int (*octal_dtr_disable)(struct spi_nor *nor);
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
 	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 4b919756a205..15c70085085b 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -47,7 +47,7 @@
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_NO_DATA)
 
-static int micron_st_nor_octal_dtr_en(struct spi_nor *nor)
+static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
 	u8 *buf = nor->bouncebuf;
@@ -84,7 +84,7 @@ static int micron_st_nor_octal_dtr_en(struct spi_nor *nor)
 	return 0;
 }
 
-static int micron_st_nor_octal_dtr_dis(struct spi_nor *nor)
+static int micron_st_nor_octal_dtr_disable(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
 	u8 *buf = nor->bouncebuf;
@@ -120,15 +120,10 @@ static int micron_st_nor_octal_dtr_dis(struct spi_nor *nor)
 	return 0;
 }
 
-static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
-{
-	return enable ? micron_st_nor_octal_dtr_en(nor) :
-			micron_st_nor_octal_dtr_dis(nor);
-}
-
 static void mt35xu512aba_default_init(struct spi_nor *nor)
 {
 	nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
+	nor->params->octal_dtr_disable = micron_st_nor_octal_dtr_disable;
 }
 
 static int mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 15f9a80c10b9..a0231075e18a 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -141,7 +141,16 @@ static int cypress_nor_sr_ready_and_clear(struct spi_nor *nor)
 	return 1;
 }
 
-static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
+/**
+ * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
+ * @nor:		pointer to a 'struct spi_nor'
+ *
+ * This also sets the memory access latency cycles to 24 to allow the flash to
+ * run at up to 200MHz.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int cypress_nor_octal_dtr_enable(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
 	u8 *buf = nor->bouncebuf;
@@ -184,7 +193,13 @@ static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
 	return 0;
 }
 
-static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
+/**
+ * cypress_nor_octal_dtr_disable() - Disable octal DTR on Cypress flashes.
+ * @nor:		pointer to a 'struct spi_nor'
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int cypress_nor_octal_dtr_disable(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
 	u8 *buf = nor->bouncebuf;
@@ -606,22 +621,6 @@ static struct spi_nor_fixups s25hx_t_fixups = {
 	.late_init = s25hx_t_late_init,
 };
 
-/**
- * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
- * @nor:		pointer to a 'struct spi_nor'
- * @enable:              whether to enable or disable Octal DTR
- *
- * This also sets the memory access latency cycles to 24 to allow the flash to
- * run at up to 200MHz.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
-{
-	return enable ? cypress_nor_octal_dtr_en(nor) :
-			cypress_nor_octal_dtr_dis(nor);
-}
-
 static int s28hx_t_post_sfdp_fixup(struct spi_nor *nor)
 {
 	/*
@@ -668,6 +667,7 @@ static int s28hx_t_post_bfpt_fixup(struct spi_nor *nor,
 static void s28hx_t_late_init(struct spi_nor *nor)
 {
 	nor->params->octal_dtr_enable = cypress_nor_octal_dtr_enable;
+	nor->params->octal_dtr_disable = cypress_nor_octal_dtr_disable;
 	cypress_nor_ecc_init(nor);
 }
 
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spi-nor: Make octal_dtr_enable() dedicate for enabling Octal DTR
  2023-06-16  5:06 [PATCH] mtd: spi-nor: Make octal_dtr_enable() dedicate for enabling Octal DTR tkuw584924
@ 2023-07-13  6:43 ` Tudor Ambarus
  2023-07-14 15:07 ` [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR Tudor Ambarus
  1 sibling, 0 replies; 5+ messages in thread
From: Tudor Ambarus @ 2023-07-13  6:43 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr, d-gole,
	Bacem.Daassi, Takahiro Kuwano

Hi, Takahiro,

On 16.06.2023 08:06, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> The function should be responsible only for enabling Octal DTR. Remove
> 'enable' parameter from octal_dtr_enable() and add octal_dtr_disable()
> that takes care for disabling. This can remove 'enable' flag checks in
> core and manufacturer drivers and improve readability.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
>  drivers/mtd/spi-nor/core.c      | 42 ++++++++++++++++++++++++++-------
>  drivers/mtd/spi-nor/core.h      |  4 +++-
>  drivers/mtd/spi-nor/micron-st.c | 11 +++------
>  drivers/mtd/spi-nor/spansion.c  | 36 ++++++++++++++--------------
>  4 files changed, 57 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 143ca3c9b477..d2571f309f41 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3086,11 +3086,10 @@ static int spi_nor_init_params(struct spi_nor *nor)
>  
>  /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
>   * @nor:                 pointer to a 'struct spi_nor'
> - * @enable:              whether to enable or disable Octal DTR
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +static int spi_nor_octal_dtr_enable(struct spi_nor *nor)
>  {
>  	int ret;
>  
> @@ -3104,14 +3103,39 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
>  	if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
>  		return 0;
>  
> -	ret = nor->params->octal_dtr_enable(nor, enable);
> +	ret = nor->params->octal_dtr_enable(nor);
>  	if (ret)
>  		return ret;
>  
> -	if (enable)
> -		nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> -	else
> -		nor->reg_proto = SNOR_PROTO_1_1_1;
> +	nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> +
> +	return 0;
> +}
> +
> +/** spi_nor_octal_dtr_disable() - disable Octal DTR I/O if it is enabled
> + * @nor:                 pointer to a 'struct spi_nor'
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_octal_dtr_disable(struct spi_nor *nor)
> +{
> +	int ret;
> +
> +	if (!nor->params->octal_dtr_disable)
> +		return 0;
> +
> +	if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
> +	      nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> +		return 0;
> +
> +	if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
> +		return 0;
> +
> +	ret = nor->params->octal_dtr_disable(nor);
> +	if (ret)
> +		return ret;
> +
> +	nor->reg_proto = SNOR_PROTO_1_1_1;
>  

I gave a little thought on this. Indeed having an
*_enable(..., bool enable) method bends the logic and we have to act on
the name, but from the core's point of view I think we should stick to a
single method, otherwise the duplication of code is obvious. On the
manufacturers drivers side however, we saw that the enable/disable
methods quite differ (see [1]), so I would recommend the have dedicated
methods for the manufacturers. Thus I would do something like this:

in the core:
int (*set_octal_dtr)(struct spi_nor *nor, bool enable);

in the manufacturer drivers:
static int manufacturer_snor_set_octal_dtr(struct spi_nor *nor, bool enable)
{
	return enable ? manufacturer_snor_octal_dtr_enable() :
			manufacturer_snor_octal_dtr_disable();
}

I'll scratch a patch later today.
Cheers,
ta

[1]
https://lore.kernel.org/linux-mtd/cover.1686557139.git.Takahiro.Kuwano@infineon.com/

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR
  2023-06-16  5:06 [PATCH] mtd: spi-nor: Make octal_dtr_enable() dedicate for enabling Octal DTR tkuw584924
  2023-07-13  6:43 ` Tudor Ambarus
@ 2023-07-14 15:07 ` Tudor Ambarus
  2023-07-18  9:28   ` Michael Walle
  2023-07-18 17:43   ` Tudor Ambarus
  1 sibling, 2 replies; 5+ messages in thread
From: Tudor Ambarus @ 2023-07-14 15:07 UTC (permalink / raw)
  To: tkuw584924, takahiro.kuwano, michael, pratyush
  Cc: linux-mtd, linux-kernel, bacem.daassi, miquel.raynal, richard,
	Tudor Ambarus

Having an *_enable(..., bool enable) definition was misleading
as the method is used both to enable and to disable the octal DTR
mode. Splitting the method in the core in two, one to enable and
another to disable the octal DTR mode does not make sense as the
method is straight forward and we'd introduce code duplication.

Update the core to use:
int (*set_octal_dtr)(struct spi_nor *nor, bool enable);

Manufacturer drivers use different sequences of commands to enable
and disable the octal DTR mode, thus for clarity they shall
implement it as:
static int manufacturer_snor_set_octal_dtr(struct spi_nor *nor, bool enable)
{
	return enable ? manufacturer_snor_octal_dtr_enable() :
			manufacturer_snor_octal_dtr_disable();
}

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/core.c      | 12 ++++++------
 drivers/mtd/spi-nor/core.h      |  4 ++--
 drivers/mtd/spi-nor/micron-st.c |  4 ++--
 drivers/mtd/spi-nor/spansion.c  |  6 +++---
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 434c545c0ce4..273258f7e77f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3090,17 +3090,17 @@ static int spi_nor_init_params(struct spi_nor *nor)
 	return 0;
 }
 
-/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
+/** spi_nor_set_octal_dtr() - enable or disable Octal DTR I/O.
  * @nor:                 pointer to a 'struct spi_nor'
  * @enable:              whether to enable or disable Octal DTR
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+static int spi_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
 {
 	int ret;
 
-	if (!nor->params->octal_dtr_enable)
+	if (!nor->params->set_octal_dtr)
 		return 0;
 
 	if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
@@ -3110,7 +3110,7 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
 	if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
 		return 0;
 
-	ret = nor->params->octal_dtr_enable(nor, enable);
+	ret = nor->params->set_octal_dtr(nor, enable);
 	if (ret)
 		return ret;
 
@@ -3171,7 +3171,7 @@ static int spi_nor_init(struct spi_nor *nor)
 {
 	int err;
 
-	err = spi_nor_octal_dtr_enable(nor, true);
+	err = spi_nor_set_octal_dtr(nor, true);
 	if (err) {
 		dev_dbg(nor->dev, "octal mode not supported\n");
 		return err;
@@ -3273,7 +3273,7 @@ static int spi_nor_suspend(struct mtd_info *mtd)
 	int ret;
 
 	/* Disable octal DTR mode if we enabled it. */
-	ret = spi_nor_octal_dtr_enable(nor, false);
+	ret = spi_nor_set_octal_dtr(nor, false);
 	if (ret)
 		dev_err(nor->dev, "suspend() failed\n");
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 55b5e7abce6e..f2fc2cf78e55 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -364,7 +364,7 @@ struct spi_nor_otp {
  * @erase_map:		the erase map parsed from the SFDP Sector Map Parameter
  *                      Table.
  * @otp:		SPI NOR OTP info.
- * @octal_dtr_enable:	enables SPI NOR octal DTR mode.
+ * @set_octal_dtr:	enables or disables SPI NOR octal DTR mode.
  * @quad_enable:	enables SPI NOR quad mode.
  * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
  * @convert_addr:	converts an absolute address into something the flash
@@ -398,7 +398,7 @@ struct spi_nor_flash_parameter {
 	struct spi_nor_erase_map        erase_map;
 	struct spi_nor_otp		otp;
 
-	int (*octal_dtr_enable)(struct spi_nor *nor, bool enable);
+	int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
 	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 4b919756a205..f79e71d99124 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -120,7 +120,7 @@ static int micron_st_nor_octal_dtr_dis(struct spi_nor *nor)
 	return 0;
 }
 
-static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+static int micron_st_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
 {
 	return enable ? micron_st_nor_octal_dtr_en(nor) :
 			micron_st_nor_octal_dtr_dis(nor);
@@ -128,7 +128,7 @@ static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
 
 static void mt35xu512aba_default_init(struct spi_nor *nor)
 {
-	nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
+	nor->params->set_octal_dtr = micron_st_nor_set_octal_dtr;
 }
 
 static int mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 36876aa849ed..6d6466a3436e 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -607,7 +607,7 @@ static struct spi_nor_fixups s25hx_t_fixups = {
 };
 
 /**
- * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
+ * cypress_nor_set_octal_dtr() - Enable or disable octal DTR on Cypress flashes.
  * @nor:		pointer to a 'struct spi_nor'
  * @enable:              whether to enable or disable Octal DTR
  *
@@ -616,7 +616,7 @@ static struct spi_nor_fixups s25hx_t_fixups = {
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+static int cypress_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
 {
 	return enable ? cypress_nor_octal_dtr_en(nor) :
 			cypress_nor_octal_dtr_dis(nor);
@@ -667,7 +667,7 @@ static int s28hx_t_post_bfpt_fixup(struct spi_nor *nor,
 
 static void s28hx_t_late_init(struct spi_nor *nor)
 {
-	nor->params->octal_dtr_enable = cypress_nor_octal_dtr_enable;
+	nor->params->set_octal_dtr = cypress_nor_set_octal_dtr;
 	cypress_nor_ecc_init(nor);
 }
 
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR
  2023-07-14 15:07 ` [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR Tudor Ambarus
@ 2023-07-18  9:28   ` Michael Walle
  2023-07-18 17:43   ` Tudor Ambarus
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Walle @ 2023-07-18  9:28 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: tkuw584924, takahiro.kuwano, pratyush, linux-mtd, linux-kernel,
	bacem.daassi, miquel.raynal, richard, Tudor Ambarus

Btw. this was threaded within another thread. At least on the
netdev (and spi) ML this is discouraged.

Am 2023-07-14 17:07, schrieb Tudor Ambarus:
> Having an *_enable(..., bool enable) definition was misleading
> as the method is used both to enable and to disable the octal DTR
> mode. Splitting the method in the core in two, one to enable and
> another to disable the octal DTR mode does not make sense as the
> method is straight forward and we'd introduce code duplication.
> 
> Update the core to use:
> int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
> 
> Manufacturer drivers use different sequences of commands to enable
> and disable the octal DTR mode, thus for clarity they shall
> implement it as:
> static int manufacturer_snor_set_octal_dtr(struct spi_nor *nor, bool 
> enable)
> {
> 	return enable ? manufacturer_snor_octal_dtr_enable() :
> 			manufacturer_snor_octal_dtr_disable();
> }
> 

I don't care much for this naming. I've also seen _enable() functions
which take a bool and then actually disable something in the kernel.

So I'm fine either way:

Reviewed-by: Michael Walle <mwalle@kernel.org>

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR
  2023-07-14 15:07 ` [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR Tudor Ambarus
  2023-07-18  9:28   ` Michael Walle
@ 2023-07-18 17:43   ` Tudor Ambarus
  1 sibling, 0 replies; 5+ messages in thread
From: Tudor Ambarus @ 2023-07-18 17:43 UTC (permalink / raw)
  To: tkuw584924, takahiro.kuwano, michael, pratyush, Tudor Ambarus
  Cc: linux-mtd, linux-kernel, bacem.daassi, miquel.raynal, richard

On Fri, 14 Jul 2023 18:07:57 +0300, Tudor Ambarus wrote:
> Having an *_enable(..., bool enable) definition was misleading
> as the method is used both to enable and to disable the octal DTR
> mode. Splitting the method in the core in two, one to enable and
> another to disable the octal DTR mode does not make sense as the
> method is straight forward and we'd introduce code duplication.
> 
> Update the core to use:
> int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
> 
> [...]

Applied to git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git,
spi-nor/next branch. Thanks!

[1/1] mtd: spi-nor: rename method for enabling or disabling octal DTR
      https://git.kernel.org/mtd/c/d4996700abc1

Cheers,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-19  6:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-16  5:06 [PATCH] mtd: spi-nor: Make octal_dtr_enable() dedicate for enabling Octal DTR tkuw584924
2023-07-13  6:43 ` Tudor Ambarus
2023-07-14 15:07 ` [PATCH] mtd: spi-nor: rename method for enabling or disabling octal DTR Tudor Ambarus
2023-07-18  9:28   ` Michael Walle
2023-07-18 17:43   ` Tudor Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox