diff mbox series

[01/23] mtd: spi-nor: Stop prefixing generic functions with a manufacturer name

Message ID 20200302180730.1886678-2-tudor.ambarus@microchip.com (mailing list archive)
State New, archived
Headers show
Series mtd: spi-nor: Move manufacturer/SFDP code out of the core | expand

Commit Message

Tudor Ambarus March 2, 2020, 6:07 p.m. UTC 
From: Boris Brezillon <bbrezillon@kernel.org>

Replace the manufacturer prefix by something describing more precisely
what those functions do.

Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
[tudor.ambarus@microchip.com: prepend spi_nor_ to all modified methods.]
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 88 ++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 43 deletions(-)

Comments

Vignesh Raghavendra March 13, 2020, 6:04 a.m. UTC | #1 
On 02/03/20 11:37 pm, Tudor.Ambarus@microchip.com wrote:
> From: Boris Brezillon <bbrezillon@kernel.org>
> 
> Replace the manufacturer prefix by something describing more precisely
> what those functions do.
> 
> Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> [tudor.ambarus@microchip.com: prepend spi_nor_ to all modified methods.]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 88 ++++++++++++++++++-----------------
>  1 file changed, 45 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index caf0c109cca0..b15e262765e1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -568,14 +568,15 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
>  }
>  
>  /**
> - * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
> + * spi_nor_en4_ex4_set_4byte() - Enter/Exit 4-byte mode for Macronix like
> + * flashes.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> +static int spi_nor_en4_ex4_set_4byte(struct spi_nor *nor, bool enable)


Sounds a bit weird, how about simplifying this to:

	spi_nor_set_4byte_addr_mode()

Or if you want to be specific:

	spi_nor_en_ex_4byte_addr_mode()

>  {
>  	int ret;
>  
> @@ -604,14 +605,15 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  }
>  
>  /**
> - * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
> + * spi_nor_en4_ex4_wen_set_4byte() - 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
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> +static int spi_nor_en4_ex4_wen_set_4byte(struct spi_nor *nor, bool enable)


Unrelated to this patch itself, but can we just have one set_4byte
variant that uses WREN and drop the other one?
I expect sending WREN should be harmless even for cmds that don't expect
one.

Rest looks good to me.

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>
Boris Brezillon March 13, 2020, 9:30 a.m. UTC | #2 
On Fri, 13 Mar 2020 11:34:55 +0530
Vignesh Raghavendra <vigneshr@ti.com> wrote:

> On 02/03/20 11:37 pm, Tudor.Ambarus@microchip.com wrote:
> > From: Boris Brezillon <bbrezillon@kernel.org>
> > 
> > Replace the manufacturer prefix by something describing more precisely
> > what those functions do.
> > 
> > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > [tudor.ambarus@microchip.com: prepend spi_nor_ to all modified methods.]
> > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 88 ++++++++++++++++++-----------------
> >  1 file changed, 45 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index caf0c109cca0..b15e262765e1 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -568,14 +568,15 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
> >  }
> >  
> >  /**
> > - * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
> > + * spi_nor_en4_ex4_set_4byte() - Enter/Exit 4-byte mode for Macronix like
> > + * flashes.
> >   * @nor:	pointer to 'struct spi_nor'.
> >   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
> >   *		address mode.
> >   *
> >   * Return: 0 on success, -errno otherwise.
> >   */
> > -static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> > +static int spi_nor_en4_ex4_set_4byte(struct spi_nor *nor, bool enable)  
> 
> 
> Sounds a bit weird, how about simplifying this to:
> 
> 	spi_nor_set_4byte_addr_mode()
> 
> Or if you want to be specific:
> 
> 	spi_nor_en_ex_4byte_addr_mode()

You're right. Maybe we can simplify things by having a single function
that does optional steps based on new flags

SPI_NOR_EN_EX_4B_NEEDS_WEN
SPI_NOR_CLEAR_EAR_ON_4B_EXIT

This should probably be done in a separate patch though, so ack on the
spi_nor_en_ex_4byte_addr_mode() rename, assuming we also change the
bool argument name to enter.

> 
> >  {
> >  	int ret;
> >  
> > @@ -604,14 +605,15 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> >  }
> >  
> >  /**
> > - * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
> > + * spi_nor_en4_ex4_wen_set_4byte() - 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
> >   *		address mode.
> >   *
> >   * Return: 0 on success, -errno otherwise.
> >   */
> > -static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> > +static int spi_nor_en4_ex4_wen_set_4byte(struct spi_nor *nor, bool enable)  
> 
> 
> Unrelated to this patch itself, but can we just have one set_4byte
> variant that uses WREN and drop the other one?

Hm, not sure that's a good idea to insert WEN instructions for
everyone, sounds like a recipe for regressions.

> I expect sending WREN should be harmless even for cmds that don't expect
> one.

In theory yes, but you know flash chips are capricious, so let's not
take the risk of breaking things :-).

> 
> Rest looks good to me.
> 
> Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>
>
Tudor Ambarus March 13, 2020, 2:30 p.m. UTC | #3 
On Friday, March 13, 2020 11:30:07 AM EET Boris Brezillon wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Fri, 13 Mar 2020 11:34:55 +0530
> 
> Vignesh Raghavendra <vigneshr@ti.com> wrote:
> > On 02/03/20 11:37 pm, Tudor.Ambarus@microchip.com wrote:
> > > From: Boris Brezillon <bbrezillon@kernel.org>
> > > 
> > > Replace the manufacturer prefix by something describing more precisely
> > > what those functions do.
> > > 
> > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > [tudor.ambarus@microchip.com: prepend spi_nor_ to all modified methods.]
> > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> > > ---
> > > 
> > >  drivers/mtd/spi-nor/spi-nor.c | 88 ++++++++++++++++++-----------------
> > >  1 file changed, 45 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > b/drivers/mtd/spi-nor/spi-nor.c
> > > index caf0c109cca0..b15e262765e1 100644
> > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > @@ -568,14 +568,15 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8
> > > *cr)> > 
> > >  }
> > >  
> > >  /**
> > > 
> > > - * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
> > > + * spi_nor_en4_ex4_set_4byte() - Enter/Exit 4-byte mode for Macronix
> > > like
> > > + * flashes.
> > > 
> > >   * @nor:   pointer to 'struct spi_nor'.
> > >   * @enable:        true to enter the 4-byte address mode, false to exit
> > >   the 4-byte *         address mode.
> > >   *
> > >   * Return: 0 on success, -errno otherwise.
> > >   */
> > > 
> > > -static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> > > +static int spi_nor_en4_ex4_set_4byte(struct spi_nor *nor, bool enable)
> > 
> > Sounds a bit weird, how about simplifying this to:
> >       spi_nor_set_4byte_addr_mode()
> > 
> > Or if you want to be specific:
> >       spi_nor_en_ex_4byte_addr_mode()
> 
> You're right. Maybe we can simplify things by having a single function
> that does optional steps based on new flags
> 
> SPI_NOR_EN_EX_4B_NEEDS_WEN
> SPI_NOR_CLEAR_EAR_ON_4B_EXIT
> 
> This should probably be done in a separate patch though, so ack on the
> spi_nor_en_ex_4byte_addr_mode() rename, assuming we also change the
> bool argument name to enter.

A single big function will be ugly to handle because of the 
spansion_set_4byte() -> it uses a different opcode.

I like the nor->params>set_4byte hook.

I think that spi_nor_en4_ex4_set_4byte() can be renamed to spi_nor_set_4byte() 
and be the only set_4byte() method exposed to manufacturers. 
spansion_set_4byte() will be static in core.c and the rest will be private in 
the manufacturer drivers.

Here's how the manufacturers enter and exit the 4 byte mode:
-> eon, gidadevice, issi, macronix, xmc use EN4B/EX4B
-> micron-st needs WEN -> private method as they are the only ones that 
require this
-> newer spansion have a 4BAM opcode (new, public command), older spansion 
flashes use BRWR (legacy in core.c, spansion_set_4byte())
-> winbond set_4byte method is hackish and may be reason for just a flash 
fixup hook -> private method

Let me know if you agree with this, so I can respin later today or tomorrow.

cut

> 
> > I expect sending WREN should be harmless even for cmds that don't expect
> > one.

The commands may be ok, but the flash can be corrupted. The WEL bit is NOT 
reset after the completion of the EN4B command on macronix flashes, so the 
gates are opened for some inadvertent commands that may corrupt the memory.

Cheers,
ta
Tudor Ambarus March 13, 2020, 3:50 p.m. UTC | #4 
On Friday, March 13, 2020 4:30:48 PM EET Tudor.Ambarus@microchip.com wrote:
> > > > - * macronix_set_4byte() - Set 4-byte address mode for Macronix
> > > > flashes.
> > > > + * spi_nor_en4_ex4_set_4byte() - Enter/Exit 4-byte mode for Macronix
> > > > like
> > > > + * flashes.
> > > > 
> > > > * @nor:   pointer to 'struct spi_nor'.
> > > > * @enable:        true to enter the 4-byte address mode, false to exit
> > > > the 4-byte *         address mode.
> > > > *
> > > > * Return: 0 on success, -errno otherwise.
> > > > */
> > > > 
> > > > -static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> > > > +static int spi_nor_en4_ex4_set_4byte(struct spi_nor *nor, bool
> > > > enable)
> > > 
> > > Sounds a bit weird, how about simplifying this to:
> > > spi_nor_set_4byte_addr_mode()
> > > 
> > > Or if you want to be specific:
> > > spi_nor_en_ex_4byte_addr_mode()
> > 
> > You're right. Maybe we can simplify things by having a single function
> > that does optional steps based on new flags
> > 
> > SPI_NOR_EN_EX_4B_NEEDS_WEN
> > SPI_NOR_CLEAR_EAR_ON_4B_EXIT
> > 
> > This should probably be done in a separate patch though, so ack on the
> > spi_nor_en_ex_4byte_addr_mode() rename, assuming we also change the
> > bool argument name to enter.
> 
> A single big function will be ugly to handle because of the
> spansion_set_4byte() -> it uses a different opcode.
> 
> I like the nor->params>set_4byte hook.
> 
> I think that spi_nor_en4_ex4_set_4byte() can be renamed to
> spi_nor_set_4byte() and be the only set_4byte() method exposed to
> manufacturers.
> spansion_set_4byte() will be static in core.c and the rest will be private
> in the manufacturer drivers.
> 
> Here's how the manufacturers enter and exit the 4 byte mode:
> -> eon, gidadevice, issi, macronix, xmc use EN4B/EX4B
> -> micron-st needs WEN -> private method as they are the only ones that
> require this
> -> newer spansion have a 4BAM opcode (new, public command), older spansion
> flashes use BRWR (legacy in core.c, spansion_set_4byte())
> -> winbond set_4byte method is hackish and may be reason for just a flash
> fixup hook -> private method

I'll drop the set_4byte changes from this patch. I'm adding a dedicated patch 
immediately after this, to implement what I said above.

Cheers,
ta
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index caf0c109cca0..b15e262765e1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -568,14 +568,15 @@  static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
 }
 
 /**
- * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
+ * spi_nor_en4_ex4_set_4byte() - Enter/Exit 4-byte mode for Macronix like
+ * flashes.
  * @nor:	pointer to 'struct spi_nor'.
  * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
  *		address mode.
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int macronix_set_4byte(struct spi_nor *nor, bool enable)
+static int spi_nor_en4_ex4_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
 
@@ -604,14 +605,15 @@  static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 }
 
 /**
- * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
+ * spi_nor_en4_ex4_wen_set_4byte() - 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
  *		address mode.
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
+static int spi_nor_en4_ex4_wen_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
 
@@ -619,7 +621,7 @@  static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
 	if (ret)
 		return ret;
 
-	ret = macronix_set_4byte(nor, enable);
+	ret = spi_nor_en4_ex4_set_4byte(nor, enable);
 	if (ret)
 		return ret;
 
@@ -703,7 +705,7 @@  static int winbond_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
 
-	ret = macronix_set_4byte(nor, enable);
+	ret = spi_nor_en4_ex4_set_4byte(nor, enable);
 	if (ret || enable)
 		return ret;
 
@@ -755,13 +757,13 @@  static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
 }
 
 /**
- * s3an_sr_ready() - Query the Status Register of the S3AN flash to see if the
- * flash is ready for new commands.
+ * 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: 0 on success, -errno otherwise.
  */
-static int s3an_sr_ready(struct spi_nor *nor)
+static int spi_nor_xsr_ready(struct spi_nor *nor)
 {
 	int ret;
 
@@ -892,7 +894,7 @@  static int spi_nor_ready(struct spi_nor *nor)
 	int sr, fsr;
 
 	if (nor->flags & SNOR_F_READY_XSR_RDY)
-		sr = s3an_sr_ready(nor);
+		sr = spi_nor_xsr_ready(nor);
 	else
 		sr = spi_nor_sr_ready(nor);
 	if (sr < 0)
@@ -1784,8 +1786,8 @@  static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 	return ret;
 }
 
-static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
-				 uint64_t *len)
+static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
+					uint64_t *len)
 {
 	struct mtd_info *mtd = &nor->mtd;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
@@ -1813,8 +1815,8 @@  static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
  * Return 1 if the entire region is locked (if @locked is true) or unlocked (if
  * @locked is false); 0 otherwise
  */
-static int stm_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
-				    u8 sr, bool locked)
+static int spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
+					uint64_t len, u8 sr, bool locked)
 {
 	loff_t lock_offs;
 	uint64_t lock_len;
@@ -1822,7 +1824,7 @@  static int stm_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, uint64_t le
 	if (!len)
 		return 1;
 
-	stm_get_locked_range(nor, sr, &lock_offs, &lock_len);
+	spi_nor_get_locked_range_sr(nor, sr, &lock_offs, &lock_len);
 
 	if (locked)
 		/* Requested range is a sub-range of locked range */
@@ -1832,16 +1834,16 @@  static int stm_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, uint64_t le
 		return (ofs >= lock_offs + lock_len) || (ofs + len <= lock_offs);
 }
 
-static int stm_is_locked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
-			    u8 sr)
+static int spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
+				u8 sr)
 {
-	return stm_check_lock_status_sr(nor, ofs, len, sr, true);
+	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true);
 }
 
-static int stm_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
-			      u8 sr)
+static int spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
+				  u8 sr)
 {
-	return stm_check_lock_status_sr(nor, ofs, len, sr, false);
+	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
 }
 
 /*
@@ -1876,7 +1878,7 @@  static int stm_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
  *
  * Returns negative on errors, 0 on success.
  */
-static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 {
 	struct mtd_info *mtd = &nor->mtd;
 	int ret, status_old, status_new;
@@ -1894,16 +1896,16 @@  static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	status_old = nor->bouncebuf[0];
 
 	/* If nothing in our range is unlocked, we don't need to do anything */
-	if (stm_is_locked_sr(nor, ofs, len, status_old))
+	if (spi_nor_is_locked_sr(nor, ofs, len, status_old))
 		return 0;
 
 	/* If anything below us is unlocked, we can't use 'bottom' protection */
-	if (!stm_is_locked_sr(nor, 0, ofs, status_old))
+	if (!spi_nor_is_locked_sr(nor, 0, ofs, status_old))
 		can_be_bottom = false;
 
 	/* If anything above us is unlocked, we can't use 'top' protection */
-	if (!stm_is_locked_sr(nor, ofs + len, mtd->size - (ofs + len),
-				status_old))
+	if (!spi_nor_is_locked_sr(nor, ofs + len, mtd->size - (ofs + len),
+				  status_old))
 		can_be_top = false;
 
 	if (!can_be_bottom && !can_be_top)
@@ -1958,11 +1960,11 @@  static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 }
 
 /*
- * Unlock a region of the flash. See stm_lock() for more info
+ * Unlock a region of the flash. See spi_nor_sr_lock() for more info
  *
  * Returns negative on errors, 0 on success.
  */
-static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 {
 	struct mtd_info *mtd = &nor->mtd;
 	int ret, status_old, status_new;
@@ -1980,16 +1982,16 @@  static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	status_old = nor->bouncebuf[0];
 
 	/* If nothing in our range is locked, we don't need to do anything */
-	if (stm_is_unlocked_sr(nor, ofs, len, status_old))
+	if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old))
 		return 0;
 
 	/* If anything below us is locked, we can't use 'top' protection */
-	if (!stm_is_unlocked_sr(nor, 0, ofs, status_old))
+	if (!spi_nor_is_unlocked_sr(nor, 0, ofs, status_old))
 		can_be_top = false;
 
 	/* If anything above us is locked, we can't use 'bottom' protection */
-	if (!stm_is_unlocked_sr(nor, ofs + len, mtd->size - (ofs + len),
-				status_old))
+	if (!spi_nor_is_unlocked_sr(nor, ofs + len, mtd->size - (ofs + len),
+				    status_old))
 		can_be_bottom = false;
 
 	if (!can_be_bottom && !can_be_top)
@@ -2046,13 +2048,13 @@  static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 }
 
 /*
- * Check if a region of the flash is (completely) locked. See stm_lock() for
- * more info.
+ * Check if a region of the flash is (completely) locked. See spi_nor_sr_lock()
+ * for more info.
  *
  * Returns 1 if entire region is locked, 0 if any portion is unlocked, and
  * negative on errors.
  */
-static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
+static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
 {
 	int ret;
 
@@ -2060,13 +2062,13 @@  static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	if (ret)
 		return ret;
 
-	return stm_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
+	return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
 }
 
-static const struct spi_nor_locking_ops stm_locking_ops = {
-	.lock = stm_lock,
-	.unlock = stm_unlock,
-	.is_locked = stm_is_locked,
+static const struct spi_nor_locking_ops spi_nor_sr_locking_ops = {
+	.lock = spi_nor_sr_lock,
+	.unlock = spi_nor_sr_unlock,
+	.is_locked = spi_nor_sr_is_locked,
 };
 
 static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
@@ -4655,7 +4657,7 @@  static void issi_set_default_init(struct spi_nor *nor)
 static void macronix_set_default_init(struct spi_nor *nor)
 {
 	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
-	nor->params.set_4byte = macronix_set_4byte;
+	nor->params.set_4byte = spi_nor_en4_ex4_set_4byte;
 }
 
 static void sst_set_default_init(struct spi_nor *nor)
@@ -4668,7 +4670,7 @@  static void st_micron_set_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 = st_micron_set_4byte;
+	nor->params.set_4byte = spi_nor_en4_ex4_wen_set_4byte;
 }
 
 static void winbond_set_default_init(struct spi_nor *nor)
@@ -4895,7 +4897,7 @@  static void spi_nor_late_init_params(struct spi_nor *nor)
 	 * the default ones.
 	 */
 	if (nor->flags & SNOR_F_HAS_LOCK && !nor->params.locking_ops)
-		nor->params.locking_ops = &stm_locking_ops;
+		nor->params.locking_ops = &spi_nor_sr_locking_ops;
 }
 
 /**