[v4,13/20] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
diff mbox series

Message ID 20191102112316.20715-14-tudor.ambarus@microchip.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series
  • mtd: spi-nor: Quad Enable and (un)lock methods
Related show

Commit Message

Ambarus Tudor Nov. 2, 2019, 11:23 a.m. UTC
From: Tudor Ambarus <tudor.ambarus@microchip.com>

Make sure that when doing a lock() or an unlock() operation we don't clear
the QE bit from Status Register 2.

JESD216 revB or later offers information about the *default* Status
Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
standard, Status Register 1 refers to the first data byte transferred on a
Read Status (05h) or Write Status (01h) command. Status register 2 refers
to the byte read using instruction 35h. Status register 2 is the second
byte transferred in a Write Status (01h) command.

Industry naming and definitions of these Status Registers may differ.
The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
There are cases in which writing only one byte to the Status Register 1
has the side-effect of clearing Status Register 2 and implicitly the Quad
Enable bit. This side-effect is hit just by the
BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.

Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 120 ++++++++++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |   3 ++
 2 files changed, 118 insertions(+), 5 deletions(-)

Comments

Vignesh Raghavendra Nov. 5, 2019, 5:07 p.m. UTC | #1
On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Make sure that when doing a lock() or an unlock() operation we don't clear
> the QE bit from Status Register 2.
> 
> JESD216 revB or later offers information about the *default* Status
> Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
> standard, Status Register 1 refers to the first data byte transferred on a
> Read Status (05h) or Write Status (01h) command. Status register 2 refers
> to the byte read using instruction 35h. Status register 2 is the second
> byte transferred in a Write Status (01h) command.
> 
> Industry naming and definitions of these Status Registers may differ.
> The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
> There are cases in which writing only one byte to the Status Register 1
> has the side-effect of clearing Status Register 2 and implicitly the Quad
> Enable bit. This side-effect is hit just by the
> BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
>

But I see that 1 byte SR1 write still happens as part of
spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?
Should this patch be rearranged to appear along with 20/20?


> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 120 ++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   |   3 ++
>  2 files changed, 118 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 725dab241271..f96bc80c0ed1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -959,12 +959,19 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
>  	return spi_nor_wait_till_ready(nor);
>  }
>  
[...]
> +/**
>   * spi_nor_write_sr2() - Write the Status Register 2 using the
>   * SPINOR_OP_WRSR2 (3eh) command.
>   * @nor:	pointer to 'struct spi_nor'.
> @@ -3634,19 +3723,38 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
> +		/*
> +		 * Writing only one byte to the Status Register has the
> +		 * side-effect of clearing Status Register 2.
> +		 */
>  	case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
> +		/*
> +		 * Read Configuration Register (35h) instruction is not
> +		 * supported.
> +		 */
> +		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;

Since SNOR_F_HAS_16BIT_SR is set by default in
spi_nor_info_init_params(), no need to set the flag here again

>  		params->quad_enable = spansion_no_read_cr_quad_enable;
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR1_BIT6:
> +		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>  		params->quad_enable = macronix_quad_enable;
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR2_BIT7:
> +		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>  		params->quad_enable = sr2_bit7_quad_enable;
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR2_BIT1:
> +		/*
> +		 * JESD216 rev B or later does not specify if writing only one
> +		 * byte to the Status Register clears or not the Status
> +		 * Register 2, so let's be cautious and keep the default
> +		 * assumption of a 16-bit Write Status (01h) command.
> +		 */
> +		nor->flags |= SNOR_F_HAS_16BIT_SR;
> +

Same here...

>  		params->quad_enable = spansion_read_cr_quad_enable;
>  		break;
>  
> @@ -4613,6 +4721,8 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  	params->quad_enable = spansion_read_cr_quad_enable;
>  	params->set_4byte = spansion_set_4byte;
>  	params->setup = spi_nor_default_setup;
> +	/* Default to 16-bit Write Status (01h) Command */
> +	nor->flags |= SNOR_F_HAS_16BIT_SR;
>  
>  	/* Set SPI NOR sizes. */
>  	params->size = (u64)info->sector_size * info->n_sectors;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index d1d736d3c8ab..d6ec55cc6d97 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -243,6 +243,9 @@ enum spi_nor_option_flags {
>  	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),
> +
>  };
>  
>  /**
>
Ambarus Tudor Nov. 6, 2019, 8:33 a.m. UTC | #2
On 11/05/2019 07:07 PM, Vignesh Raghavendra wrote:
> On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Make sure that when doing a lock() or an unlock() operation we don't clear
>> the QE bit from Status Register 2.
>>
>> JESD216 revB or later offers information about the *default* Status
>> Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
>> standard, Status Register 1 refers to the first data byte transferred on a
>> Read Status (05h) or Write Status (01h) command. Status register 2 refers
>> to the byte read using instruction 35h. Status register 2 is the second
>> byte transferred in a Write Status (01h) command.
>>
>> Industry naming and definitions of these Status Registers may differ.
>> The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
>> There are cases in which writing only one byte to the Status Register 1
>> has the side-effect of clearing Status Register 2 and implicitly the Quad
>> Enable bit. This side-effect is hit just by the
>> BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
>>
> But I see that 1 byte SR1 write still happens as part of
> spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?

Fixing spi_nor_clear_sr_bp() would mean to add dead code that will be removed
anyway with patch 20/20. This patch fixes the clearing of the QE bit, while in
20/20 the QE bit is already zero when the one byte SR1 write is used, so the
quad mode is not affected. 20/20 fixes indirectly the clearing of all the bits
from SR2 but QE bit, because it's already zero. I would say it's not a partial
fix, but a different bug.

There are different angles to look at this, I chose the modifying of the quad
mode angle. Given the two arguments from above (avoid adding dead code and
changing of quad mode approach), I would prefer to keep things as they are. But
I get your approach too, so if you still want yours, I can do it. Please let me
know.

> Should this patch be rearranged to appear along with 20/20?

Not necessarily (different bugs) but I can bring 20/20 immediately after this
one if you want.

> 
> 
>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 120 ++++++++++++++++++++++++++++++++++++++++--
>>  include/linux/mtd/spi-nor.h   |   3 ++
>>  2 files changed, 118 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 725dab241271..f96bc80c0ed1 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -959,12 +959,19 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
>>  	return spi_nor_wait_till_ready(nor);
>>  }
>>  
> [...]
>> +/**
>>   * spi_nor_write_sr2() - Write the Status Register 2 using the
>>   * SPINOR_OP_WRSR2 (3eh) command.
>>   * @nor:	pointer to 'struct spi_nor'.
>> @@ -3634,19 +3723,38 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>  		break;
>>  
>>  	case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
>> +		/*
>> +		 * Writing only one byte to the Status Register has the
>> +		 * side-effect of clearing Status Register 2.
>> +		 */
>>  	case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
>> +		/*
>> +		 * Read Configuration Register (35h) instruction is not
>> +		 * supported.
>> +		 */
>> +		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
> Since SNOR_F_HAS_16BIT_SR is set by default in
> spi_nor_info_init_params(), no need to set the flag here again
> 

I did this on purpose. I set SNOR_F_HAS_16BIT_SR here based on SFDP standard, I
want to indicate where the standard requires the 16 bit SR write .
spi_nor_info_init_params() initializes data based on info, but that data can be
overwritten (even with the same data) when parsing SFDP.

Thanks,
ta
Vignesh Raghavendra Nov. 6, 2019, 4:26 p.m. UTC | #3
Hi,

On 06/11/19 2:03 PM, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 11/05/2019 07:07 PM, Vignesh Raghavendra wrote:
>> On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>> Make sure that when doing a lock() or an unlock() operation we don't clear
>>> the QE bit from Status Register 2.
>>>
>>> JESD216 revB or later offers information about the *default* Status
>>> Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
>>> standard, Status Register 1 refers to the first data byte transferred on a
>>> Read Status (05h) or Write Status (01h) command. Status register 2 refers
>>> to the byte read using instruction 35h. Status register 2 is the second
>>> byte transferred in a Write Status (01h) command.
>>>
>>> Industry naming and definitions of these Status Registers may differ.
>>> The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
>>> There are cases in which writing only one byte to the Status Register 1
>>> has the side-effect of clearing Status Register 2 and implicitly the Quad
>>> Enable bit. This side-effect is hit just by the
>>> BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
>>>
>> But I see that 1 byte SR1 write still happens as part of
>> spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?
> 
> Fixing spi_nor_clear_sr_bp() would mean to add dead code that will be removed
> anyway with patch 20/20. This patch fixes the clearing of the QE bit, while in
> 20/20 the QE bit is already zero when the one byte SR1 write is used, so the
> quad mode is not affected. 20/20 fixes indirectly the clearing of all the bits
> from SR2 but QE bit, because it's already zero. I would say it's not a partial
> fix, but a different bug.
> 

I was not suggesting on fixing up spi_nor_clear_sr_bp(), but pointing
out that single byte writes SR1 can happen until patch 20/20.

But now I see that these patches are fixing related but different bugs.

> There are different angles to look at this, I chose the modifying of the quad
> mode angle. Given the two arguments from above (avoid adding dead code and
> changing of quad mode approach), I would prefer to keep things as they are. 

Ok, sounds fine, no need to change...

> But I get your approach too, so if you still want yours, I can do it. Please let me
> know.
> 
>> Should this patch be rearranged to appear along with 20/20?
> 
> Not necessarily (different bugs) but I can bring 20/20 immediately after this
> one if you want.
> >>
>>
>>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 120 ++++++++++++++++++++++++++++++++++++++++--
>>>  include/linux/mtd/spi-nor.h   |   3 ++
>>>  2 files changed, 118 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 725dab241271..f96bc80c0ed1 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -959,12 +959,19 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
>>>  	return spi_nor_wait_till_ready(nor);
>>>  }
>>>  
>> [...]
>>> +/**
>>>   * spi_nor_write_sr2() - Write the Status Register 2 using the
>>>   * SPINOR_OP_WRSR2 (3eh) command.
>>>   * @nor:	pointer to 'struct spi_nor'.
>>> @@ -3634,19 +3723,38 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>>  		break;
>>>  
>>>  	case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
>>> +		/*
>>> +		 * Writing only one byte to the Status Register has the
>>> +		 * side-effect of clearing Status Register 2.
>>> +		 */
>>>  	case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
>>> +		/*
>>> +		 * Read Configuration Register (35h) instruction is not
>>> +		 * supported.
>>> +		 */
>>> +		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
>> Since SNOR_F_HAS_16BIT_SR is set by default in
>> spi_nor_info_init_params(), no need to set the flag here again
>>
> 
> I did this on purpose. I set SNOR_F_HAS_16BIT_SR here based on SFDP standard, I
> want to indicate where the standard requires the 16 bit SR write .
> spi_nor_info_init_params() initializes data based on info, but that data can be
> overwritten (even with the same data) when parsing SFDP.
> 

Alright.

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

Patch
diff mbox series

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 725dab241271..f96bc80c0ed1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -959,12 +959,19 @@  static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
 	return spi_nor_wait_till_ready(nor);
 }
 
-/* Write status register and ensure bits in mask match written values */
-static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
+ * ensure that the byte written match the received value.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @sr1:	byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
 {
 	int ret;
 
-	nor->bouncebuf[0] = status_new;
+	nor->bouncebuf[0] = sr1;
 
 	ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
 	if (ret)
@@ -974,8 +981,73 @@  static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
 	if (ret)
 		return ret;
 
-	if (nor->bouncebuf[0] != status_new) {
-		dev_dbg(nor->dev, "SR: read back test failed\n");
+	if (nor->bouncebuf[0] != sr1) {
+		dev_dbg(nor->dev, "SR1: read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/**
+ * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
+ * Status Register 2 in one shot. Ensure that the byte written in the Status
+ * Register 1 match the received value, and that the 16-bit Write did not
+ * affect what was already in the Status Register 2.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @sr1:	byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+	int ret;
+	u8 *sr_cr = nor->bouncebuf;
+	u8 cr_written;
+
+	/* Make sure we don't overwrite the contents of Status Register 2. */
+	if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+		ret = spi_nor_read_cr(nor, &sr_cr[1]);
+		if (ret)
+			return ret;
+	} else if (nor->params.quad_enable) {
+		/*
+		 * If the Status Register 2 Read command (35h) is not
+		 * supported, we should at least be sure we don't
+		 * change the value of the SR2 Quad Enable bit.
+		 *
+		 * We can safely assume that when the Quad Enable method is
+		 * set, the value of the QE bit is one, as a consequence of the
+		 * nor->params.quad_enable() call.
+		 *
+		 * We can safely assume that the Quad Enable bit is present in
+		 * the Status Register 2 at BIT(1). According to the JESD216
+		 * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
+		 * Write Status (01h) command is available just for the cases
+		 * in which the QE bit is described in SR2 at BIT(1).
+		 */
+		sr_cr[1] = CR_QUAD_EN_SPAN;
+	} else {
+		sr_cr[1] = 0;
+	}
+
+	sr_cr[0] = sr1;
+
+	ret = spi_nor_write_sr(nor, sr_cr, 2);
+	if (ret)
+		return ret;
+
+	if (nor->flags & SNOR_F_NO_READ_CR)
+		return 0;
+
+	cr_written = sr_cr[1];
+
+	ret = spi_nor_read_cr(nor, &sr_cr[1]);
+	if (ret)
+		return ret;
+
+	if (cr_written != sr_cr[1]) {
+		dev_dbg(nor->dev, "CR: read back test failed\n");
 		return -EIO;
 	}
 
@@ -983,6 +1055,23 @@  static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
 }
 
 /**
+ * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value without affecting other bits in the
+ * Status Register 1 and 2.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @sr1:	byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+	if (nor->flags & SNOR_F_HAS_16BIT_SR)
+		return spi_nor_write_16bit_sr_and_check(nor, sr1);
+
+	return spi_nor_write_sr1_and_check(nor, sr1);
+}
+
+/**
  * spi_nor_write_sr2() - Write the Status Register 2 using the
  * SPINOR_OP_WRSR2 (3eh) command.
  * @nor:	pointer to 'struct spi_nor'.
@@ -3634,19 +3723,38 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		break;
 
 	case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
+		/*
+		 * Writing only one byte to the Status Register has the
+		 * side-effect of clearing Status Register 2.
+		 */
 	case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+		/*
+		 * Read Configuration Register (35h) instruction is not
+		 * supported.
+		 */
+		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
 		params->quad_enable = spansion_no_read_cr_quad_enable;
 		break;
 
 	case BFPT_DWORD15_QER_SR1_BIT6:
+		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
 		params->quad_enable = macronix_quad_enable;
 		break;
 
 	case BFPT_DWORD15_QER_SR2_BIT7:
+		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
 		params->quad_enable = sr2_bit7_quad_enable;
 		break;
 
 	case BFPT_DWORD15_QER_SR2_BIT1:
+		/*
+		 * JESD216 rev B or later does not specify if writing only one
+		 * byte to the Status Register clears or not the Status
+		 * Register 2, so let's be cautious and keep the default
+		 * assumption of a 16-bit Write Status (01h) command.
+		 */
+		nor->flags |= SNOR_F_HAS_16BIT_SR;
+
 		params->quad_enable = spansion_read_cr_quad_enable;
 		break;
 
@@ -4613,6 +4721,8 @@  static void spi_nor_info_init_params(struct spi_nor *nor)
 	params->quad_enable = spansion_read_cr_quad_enable;
 	params->set_4byte = spansion_set_4byte;
 	params->setup = spi_nor_default_setup;
+	/* Default to 16-bit Write Status (01h) Command */
+	nor->flags |= SNOR_F_HAS_16BIT_SR;
 
 	/* Set SPI NOR sizes. */
 	params->size = (u64)info->sector_size * info->n_sectors;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d1d736d3c8ab..d6ec55cc6d97 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,9 @@  enum spi_nor_option_flags {
 	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),
+
 };
 
 /**