diff mbox series

[v15,4/8] mtd: spi-nor: Do not change nor->addr_nbytes at SFDP parsing time

Message ID ccff78b42bab8d6eb0ffa2127f4fe6c4daddf6af.1652063152.git.Takahiro.Kuwano@infineon.com
State Superseded
Delegated to: Pratyush Yadav
Headers show
Series mtd: spi-nor: Add support for Infineon s25hl-t/s25hs-t | expand

Commit Message

Takahiro Kuwano May 9, 2022, 10:10 p.m. UTC
From: Tudor Ambarus <tudor.ambarus@microchip.com>

At the SFDP parsing time we should not change members of struct spi_nor,
but instead fill members of struct spi_nor_flash_parameters which could
leter on be used by callers. The caller will then decide if SFDP params
should be used and more importantly when they should be used. Clean the
code flow and don't initialize nor->addr_nbytes at SFDP parsing time.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Tested-By: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c |  5 ++---
 drivers/mtd/spi-nor/core.h |  2 ++
 drivers/mtd/spi-nor/sfdp.c | 18 ++++++------------
 3 files changed, 10 insertions(+), 15 deletions(-)

Comments

Michael Walle May 12, 2022, 9:45 p.m. UTC | #1
Am 2022-05-10 00:10, schrieb tkuw584924@gmail.com:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> At the SFDP parsing time we should not change members of struct 
> spi_nor,
> but instead fill members of struct spi_nor_flash_parameters which could
> leter on be used by callers. The caller will then decide if SFDP params
> should be used and more importantly when they should be used. Clean the
> code flow and don't initialize nor->addr_nbytes at SFDP parsing time.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Tested-By: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Nice!

Reviewed-by: Michael Walle <michael@walle.cc>
Pratyush Yadav May 31, 2022, 11:30 a.m. UTC | #2
On 10/05/22 07:10AM, tkuw584924@gmail.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> At the SFDP parsing time we should not change members of struct spi_nor,
> but instead fill members of struct spi_nor_flash_parameters which could
> leter on be used by callers. The caller will then decide if SFDP params

s/leter/later/

> should be used and more importantly when they should be used. Clean the
> code flow and don't initialize nor->addr_nbytes at SFDP parsing time.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Tested-By: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
>  drivers/mtd/spi-nor/core.c |  5 ++---
>  drivers/mtd/spi-nor/core.h |  2 ++
>  drivers/mtd/spi-nor/sfdp.c | 18 ++++++------------
>  3 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 7db6b41d7c30..dd71deba9f11 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2272,8 +2272,8 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>  
>  static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
>  {
> -	if (nor->addr_nbytes) {
> -		/* already configured from SFDP */
> +	if (nor->params->addr_nbytes) {
> +		nor->addr_nbytes = nor->params->addr_nbytes;
>  	} else if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
>  		/*
>  		 * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
> @@ -2518,7 +2518,6 @@ static void spi_nor_sfdp_init_params_deprecated(struct spi_nor *nor)
>  
>  	if (spi_nor_parse_sfdp(nor)) {
>  		memcpy(nor->params, &sfdp_params, sizeof(*nor->params));
> -		nor->addr_nbytes = 0;
>  		nor->flags &= ~SNOR_F_4B_OPCODES;
>  	}
>  }
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index fe7683fe1b4d..41df8bc8e008 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -345,6 +345,7 @@ struct spi_nor_otp {
>   * @writesize		Minimal writable flash unit size. Defaults to 1. Set to
>   *			ECC unit size for ECC-ed flashes.
>   * @page_size:		the page size of the SPI NOR flash memory.
> + * @addr_nbytes:	number of address bytes to send.
>   * @rdsr_dummy:		dummy cycles needed for Read Status Register command
>   *			in octal DTR mode.
>   * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
> @@ -377,6 +378,7 @@ struct spi_nor_flash_parameter {
>  	u64				size;
>  	u32				writesize;
>  	u32				page_size;
> +	u8				addr_nbytes;
>  	u8				rdsr_dummy;
>  	u8				rdsr_addr_nbytes;
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 058ce218d2af..90d7c25f7281 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -178,19 +178,16 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
>  static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
>  			     size_t len, void *buf)
>  {
> -	u8 addr_nbytes;
>  	int ret;
>  
> -	addr_nbytes = nor->addr_nbytes;
> -
>  	nor->read_opcode = SPINOR_OP_RDSFDP;
>  	nor->addr_nbytes = 3;
>  	nor->read_dummy = 8;
>  
>  	ret = spi_nor_read_raw(nor, addr, len, buf);
>  
> -	nor->addr_nbytes = addr_nbytes;
>  	/* Restore setup to its uninitialized state. */
> +	nor->addr_nbytes = 0;
>  	nor->read_opcode = 0;
>  	nor->read_dummy = 0;
>  
> @@ -461,11 +458,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>  	case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>  	case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> -		nor->addr_nbytes = 3;
> +		params->addr_nbytes = 3;
>  		break;
>  
>  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> -		nor->addr_nbytes = 4;
> +		params->addr_nbytes = 4;
>  		break;
>  
>  	default:
> @@ -652,7 +649,7 @@ static u8 spi_nor_smpt_addr_nbytes(const struct spi_nor *nor, const u32 settings
>  		return 4;
>  	case SMPT_CMD_ADDRESS_LEN_USE_CURRENT:
>  	default:
> -		return nor->addr_nbytes;
> +		return nor->params->addr_nbytes;
>  	}
>  }
>  
> @@ -689,7 +686,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  	u32 addr;
>  	int err;
>  	u8 i;
> -	u8 addr_nbytes;
>  	u8 read_data_mask, map_id;
>  
>  	/* Use a kmalloc'ed bounce buffer to guarantee it is DMA-able. */
> @@ -697,8 +693,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  	if (!buf)
>  		return ERR_PTR(-ENOMEM);
>  
> -	addr_nbytes = nor->addr_nbytes;
> -
>  	map_id = 0;
>  	/* Determine if there are any optional Detection Command Descriptors */
>  	for (i = 0; i < smpt_len; i += 2) {
> @@ -753,8 +747,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  	/* fall through */
>  out:
>  	kfree(buf);
> -	nor->addr_nbytes = addr_nbytes;
>  	/* Restore setup to its uninitialized state. */
> +	nor->addr_nbytes = 0;

Same as before, I don't think this function should know or care about 
the default or uninitialised values.

>  	nor->read_dummy = 0;
>  	nor->read_opcode = 0;
>  	return ret;
> @@ -1096,7 +1090,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>  	 * Spansion memory. However this quirk is no longer needed with new
>  	 * SFDP compliant memories.
>  	 */
> -	nor->addr_nbytes = 4;
> +	params->addr_nbytes = 4;

Patch looks good at first glance but I have not looked very carefully if 
it might cause some small issues.

>  	nor->flags |= SNOR_F_4B_OPCODES | SNOR_F_HAS_4BAIT;
>  
>  	/* fall through */
> -- 
> 2.25.1
> 

issi.c's is25lp256_post_bfpt_fixups() also sets nor->addr_nbytes. Should 
that be updated to use params->addr_nbytes instead?
Tudor Ambarus July 21, 2022, 2:32 p.m. UTC | #3
On 5/31/22 14:30, Pratyush Yadav wrote:
> issi.c's is25lp256_post_bfpt_fixups() also sets nor->addr_nbytes. Should
> that be updated to use params->addr_nbytes instead?

Yes, it should, good catch! I've revisited this patch, otherwise
it looks good.
ta
Tudor Ambarus July 21, 2022, 3:02 p.m. UTC | #4
On 5/31/22 14:30, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/05/22 07:10AM, tkuw584924@gmail.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> At the SFDP parsing time we should not change members of struct spi_nor,
>> but instead fill members of struct spi_nor_flash_parameters which could
>> leter on be used by callers. The caller will then decide if SFDP params
> 
> s/leter/later/
> 
>> should be used and more importantly when they should be used. Clean the
>> code flow and don't initialize nor->addr_nbytes at SFDP parsing time.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> Tested-By: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> ---
>>  drivers/mtd/spi-nor/core.c |  5 ++---
>>  drivers/mtd/spi-nor/core.h |  2 ++
>>  drivers/mtd/spi-nor/sfdp.c | 18 ++++++------------
>>  3 files changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 7db6b41d7c30..dd71deba9f11 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2272,8 +2272,8 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>>
>>  static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
>>  {
>> -     if (nor->addr_nbytes) {
>> -             /* already configured from SFDP */
>> +     if (nor->params->addr_nbytes) {
>> +             nor->addr_nbytes = nor->params->addr_nbytes;
>>       } else if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
>>               /*
>>                * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
>> @@ -2518,7 +2518,6 @@ static void spi_nor_sfdp_init_params_deprecated(struct spi_nor *nor)
>>
>>       if (spi_nor_parse_sfdp(nor)) {
>>               memcpy(nor->params, &sfdp_params, sizeof(*nor->params));
>> -             nor->addr_nbytes = 0;
>>               nor->flags &= ~SNOR_F_4B_OPCODES;
>>       }
>>  }
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index fe7683fe1b4d..41df8bc8e008 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -345,6 +345,7 @@ struct spi_nor_otp {
>>   * @writesize                Minimal writable flash unit size. Defaults to 1. Set to
>>   *                   ECC unit size for ECC-ed flashes.
>>   * @page_size:               the page size of the SPI NOR flash memory.
>> + * @addr_nbytes:     number of address bytes to send.
>>   * @rdsr_dummy:              dummy cycles needed for Read Status Register command
>>   *                   in octal DTR mode.
>>   * @rdsr_addr_nbytes:        dummy address bytes needed for Read Status Register
>> @@ -377,6 +378,7 @@ struct spi_nor_flash_parameter {
>>       u64                             size;
>>       u32                             writesize;
>>       u32                             page_size;
>> +     u8                              addr_nbytes;
>>       u8                              rdsr_dummy;
>>       u8                              rdsr_addr_nbytes;
>>
>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>> index 058ce218d2af..90d7c25f7281 100644
>> --- a/drivers/mtd/spi-nor/sfdp.c
>> +++ b/drivers/mtd/spi-nor/sfdp.c
>> @@ -178,19 +178,16 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
>>  static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
>>                            size_t len, void *buf)
>>  {
>> -     u8 addr_nbytes;
>>       int ret;
>>
>> -     addr_nbytes = nor->addr_nbytes;
>> -
>>       nor->read_opcode = SPINOR_OP_RDSFDP;
>>       nor->addr_nbytes = 3;
>>       nor->read_dummy = 8;
>>
>>       ret = spi_nor_read_raw(nor, addr, len, buf);
>>
>> -     nor->addr_nbytes = addr_nbytes;
>>       /* Restore setup to its uninitialized state. */
>> +     nor->addr_nbytes = 0;
>>       nor->read_opcode = 0;
>>       nor->read_dummy = 0;
>>
>> @@ -461,11 +458,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>       switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>       case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>       case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>> -             nor->addr_nbytes = 3;
>> +             params->addr_nbytes = 3;
>>               break;
>>
>>       case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>> -             nor->addr_nbytes = 4;
>> +             params->addr_nbytes = 4;
>>               break;
>>
>>       default:
>> @@ -652,7 +649,7 @@ static u8 spi_nor_smpt_addr_nbytes(const struct spi_nor *nor, const u32 settings
>>               return 4;
>>       case SMPT_CMD_ADDRESS_LEN_USE_CURRENT:
>>       default:
>> -             return nor->addr_nbytes;
>> +             return nor->params->addr_nbytes;
>>       }
>>  }
>>
>> @@ -689,7 +686,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>       u32 addr;
>>       int err;
>>       u8 i;
>> -     u8 addr_nbytes;
>>       u8 read_data_mask, map_id;
>>
>>       /* Use a kmalloc'ed bounce buffer to guarantee it is DMA-able. */
>> @@ -697,8 +693,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>       if (!buf)
>>               return ERR_PTR(-ENOMEM);
>>
>> -     addr_nbytes = nor->addr_nbytes;
>> -
>>       map_id = 0;
>>       /* Determine if there are any optional Detection Command Descriptors */
>>       for (i = 0; i < smpt_len; i += 2) {
>> @@ -753,8 +747,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>       /* fall through */
>>  out:
>>       kfree(buf);
>> -     nor->addr_nbytes = addr_nbytes;
>>       /* Restore setup to its uninitialized state. */
>> +     nor->addr_nbytes = 0;
> 
> Same as before, I don't think this function should know or care about
> the default or uninitialised values.
> 

well, maybe. nor->addr_nbytes comes with value zero when this method is called.
We gratuitously save it's zero value in a local variable and then we restore
it. The restore of the value could signify to the reader that nor->addr_nbytes
is already initialized with a sane value, which is not the case, so I find the
code more readable in my version. But I don't mind too much, so I'll drop this
particular change.

Thanks,
ta
 
>>       nor->read_dummy = 0;
>>       nor->read_opcode = 0;
>>       return ret;
>> @@ -1096,7 +1090,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>>        * Spansion memory. However this quirk is no longer needed with new
>>        * SFDP compliant memories.
>>        */
>> -     nor->addr_nbytes = 4;
>> +     params->addr_nbytes = 4;
> 
> Patch looks good at first glance but I have not looked very carefully if
> it might cause some small issues.
> 
>>       nor->flags |= SNOR_F_4B_OPCODES | SNOR_F_HAS_4BAIT;
>>
>>       /* fall through */
>> --
>> 2.25.1
>>
> 
> issi.c's is25lp256_post_bfpt_fixups() also sets nor->addr_nbytes. Should
> that be updated to use params->addr_nbytes instead?
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 7db6b41d7c30..dd71deba9f11 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2272,8 +2272,8 @@  static int spi_nor_default_setup(struct spi_nor *nor,
 
 static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
 {
-	if (nor->addr_nbytes) {
-		/* already configured from SFDP */
+	if (nor->params->addr_nbytes) {
+		nor->addr_nbytes = nor->params->addr_nbytes;
 	} else if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
 		/*
 		 * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
@@ -2518,7 +2518,6 @@  static void spi_nor_sfdp_init_params_deprecated(struct spi_nor *nor)
 
 	if (spi_nor_parse_sfdp(nor)) {
 		memcpy(nor->params, &sfdp_params, sizeof(*nor->params));
-		nor->addr_nbytes = 0;
 		nor->flags &= ~SNOR_F_4B_OPCODES;
 	}
 }
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index fe7683fe1b4d..41df8bc8e008 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -345,6 +345,7 @@  struct spi_nor_otp {
  * @writesize		Minimal writable flash unit size. Defaults to 1. Set to
  *			ECC unit size for ECC-ed flashes.
  * @page_size:		the page size of the SPI NOR flash memory.
+ * @addr_nbytes:	number of address bytes to send.
  * @rdsr_dummy:		dummy cycles needed for Read Status Register command
  *			in octal DTR mode.
  * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
@@ -377,6 +378,7 @@  struct spi_nor_flash_parameter {
 	u64				size;
 	u32				writesize;
 	u32				page_size;
+	u8				addr_nbytes;
 	u8				rdsr_dummy;
 	u8				rdsr_addr_nbytes;
 
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 058ce218d2af..90d7c25f7281 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -178,19 +178,16 @@  static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
 static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
 			     size_t len, void *buf)
 {
-	u8 addr_nbytes;
 	int ret;
 
-	addr_nbytes = nor->addr_nbytes;
-
 	nor->read_opcode = SPINOR_OP_RDSFDP;
 	nor->addr_nbytes = 3;
 	nor->read_dummy = 8;
 
 	ret = spi_nor_read_raw(nor, addr, len, buf);
 
-	nor->addr_nbytes = addr_nbytes;
 	/* Restore setup to its uninitialized state. */
+	nor->addr_nbytes = 0;
 	nor->read_opcode = 0;
 	nor->read_dummy = 0;
 
@@ -461,11 +458,11 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
 	case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
 	case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
-		nor->addr_nbytes = 3;
+		params->addr_nbytes = 3;
 		break;
 
 	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
-		nor->addr_nbytes = 4;
+		params->addr_nbytes = 4;
 		break;
 
 	default:
@@ -652,7 +649,7 @@  static u8 spi_nor_smpt_addr_nbytes(const struct spi_nor *nor, const u32 settings
 		return 4;
 	case SMPT_CMD_ADDRESS_LEN_USE_CURRENT:
 	default:
-		return nor->addr_nbytes;
+		return nor->params->addr_nbytes;
 	}
 }
 
@@ -689,7 +686,6 @@  static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 	u32 addr;
 	int err;
 	u8 i;
-	u8 addr_nbytes;
 	u8 read_data_mask, map_id;
 
 	/* Use a kmalloc'ed bounce buffer to guarantee it is DMA-able. */
@@ -697,8 +693,6 @@  static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 	if (!buf)
 		return ERR_PTR(-ENOMEM);
 
-	addr_nbytes = nor->addr_nbytes;
-
 	map_id = 0;
 	/* Determine if there are any optional Detection Command Descriptors */
 	for (i = 0; i < smpt_len; i += 2) {
@@ -753,8 +747,8 @@  static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 	/* fall through */
 out:
 	kfree(buf);
-	nor->addr_nbytes = addr_nbytes;
 	/* Restore setup to its uninitialized state. */
+	nor->addr_nbytes = 0;
 	nor->read_dummy = 0;
 	nor->read_opcode = 0;
 	return ret;
@@ -1096,7 +1090,7 @@  static int spi_nor_parse_4bait(struct spi_nor *nor,
 	 * Spansion memory. However this quirk is no longer needed with new
 	 * SFDP compliant memories.
 	 */
-	nor->addr_nbytes = 4;
+	params->addr_nbytes = 4;
 	nor->flags |= SNOR_F_4B_OPCODES | SNOR_F_HAS_4BAIT;
 
 	/* fall through */