diff mbox series

[v2,25/35] mtd: spi-nor: core: Move spi_nor_set_addr_width() in spi_nor_setup()

Message ID 20210727045222.905056-26-tudor.ambarus@microchip.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series mtd: spi-nor: Handle ID collisions and clean params init | expand

Commit Message

Tudor Ambarus July 27, 2021, 4:52 a.m. UTC 
spi_nor_setup() configures the SPI NOR memory. Setting the addr width
is too a configuration, hence we can move the spi_nor_set_addr_width()
in spi_nor_setup().

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/core.c | 101 +++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 49 deletions(-)

Comments

Pratyush Yadav Aug. 17, 2021, 4:52 p.m. UTC | #1 
On 27/07/21 07:52AM, Tudor Ambarus wrote:
> spi_nor_setup() configures the SPI NOR memory. Setting the addr width
> is too a configuration, hence we can move the spi_nor_set_addr_width()
> in spi_nor_setup().
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
Michael Walle Oct. 22, 2021, 12:12 p.m. UTC | #2 
Am 2021-07-27 06:52, schrieb Tudor Ambarus:
> spi_nor_setup() configures the SPI NOR memory. Setting the addr width
> is too a configuration, hence we can move the spi_nor_set_addr_width()
> in spi_nor_setup().
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

..

>  static int spi_nor_setup(struct spi_nor *nor,
>  			 const struct spi_nor_hwcaps *hwcaps)
>  {
> +	int ret;
> +
>  	if (!nor->params->setup)
> -		return 0;
> +		return spi_nor_set_addr_width(nor);
> 
> -	return nor->params->setup(nor, hwcaps);
> +	ret = nor->params->setup(nor, hwcaps);
> +	if (ret)
> +		return ret;
> +
> +	return spi_nor_set_addr_width(nor);
>  }

Why not

static int spi_nor_setup(struct spi_nor *nor,
			 const struct spi_nor_hwcaps *hwcaps)
{
	int ret;

	if (nor->params->setup) {
		ret = nor->params->setup(nor, hwcaps);
		if (ret)
			return ret;
	}

	return spi_nor_set_addr_width(nor)
}

-michael
Tudor Ambarus Oct. 22, 2021, 12:36 p.m. UTC | #3 
On 10/22/21 3:12 PM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-07-27 06:52, schrieb Tudor Ambarus:
>> spi_nor_setup() configures the SPI NOR memory. Setting the addr width
>> is too a configuration, hence we can move the spi_nor_set_addr_width()
>> in spi_nor_setup().
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> ..
> 
>>  static int spi_nor_setup(struct spi_nor *nor,
>>                        const struct spi_nor_hwcaps *hwcaps)
>>  {
>> +     int ret;
>> +
>>       if (!nor->params->setup)
>> -             return 0;
>> +             return spi_nor_set_addr_width(nor);
>>
>> -     return nor->params->setup(nor, hwcaps);
>> +     ret = nor->params->setup(nor, hwcaps);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return spi_nor_set_addr_width(nor);
>>  }
> 
> Why not
> 
> static int spi_nor_setup(struct spi_nor *nor,
>                         const struct spi_nor_hwcaps *hwcaps)
> {
>        int ret;
> 
>        if (nor->params->setup) {
>                ret = nor->params->setup(nor, hwcaps);
>                if (ret)
>                        return ret;
>        }
> 
>        return spi_nor_set_addr_width(nor)
> }
> 
> -michael

right, I'm reworking the series right now, that's how I updated it too.
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 83f27405a000..b3a01d7d6f0b 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2485,13 +2485,62 @@  static int spi_nor_default_setup(struct spi_nor *nor,
 	return 0;
 }
 
+static int spi_nor_set_addr_width(struct spi_nor *nor)
+{
+	if (nor->addr_width) {
+		/* already configured from SFDP */
+	} 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
+		 * in this protocol an odd address width cannot be used because
+		 * then the address phase would only span a cycle and a half.
+		 * Half a cycle would be left over. We would then have to start
+		 * the dummy phase in the middle of a cycle and so too the data
+		 * phase, and we will end the transaction with half a cycle left
+		 * over.
+		 *
+		 * Force all 8D-8D-8D flashes to use an address width of 4 to
+		 * avoid this situation.
+		 */
+		nor->addr_width = 4;
+	} else if (nor->info->addr_width) {
+		nor->addr_width = nor->info->addr_width;
+	} else {
+		nor->addr_width = 3;
+	}
+
+	if (nor->addr_width == 3 && nor->mtd.size > 0x1000000) {
+		/* enable 4-byte addressing if the device exceeds 16MiB */
+		nor->addr_width = 4;
+	}
+
+	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
+		dev_dbg(nor->dev, "address width is too large: %u\n",
+			nor->addr_width);
+		return -EINVAL;
+	}
+
+	/* Set 4byte opcodes when possible. */
+	if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
+	    !(nor->flags & SNOR_F_HAS_4BAIT))
+		spi_nor_set_4byte_opcodes(nor);
+
+	return 0;
+}
+
 static int spi_nor_setup(struct spi_nor *nor,
 			 const struct spi_nor_hwcaps *hwcaps)
 {
+	int ret;
+
 	if (!nor->params->setup)
-		return 0;
+		return spi_nor_set_addr_width(nor);
 
-	return nor->params->setup(nor, hwcaps);
+	ret = nor->params->setup(nor, hwcaps);
+	if (ret)
+		return ret;
+
+	return spi_nor_set_addr_width(nor);
 }
 
 /**
@@ -3031,49 +3080,6 @@  static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
 	return NULL;
 }
 
-static int spi_nor_set_addr_width(struct spi_nor *nor)
-{
-	if (nor->addr_width) {
-		/* already configured from SFDP */
-	} 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
-		 * in this protocol an odd address width cannot be used because
-		 * then the address phase would only span a cycle and a half.
-		 * Half a cycle would be left over. We would then have to start
-		 * the dummy phase in the middle of a cycle and so too the data
-		 * phase, and we will end the transaction with half a cycle left
-		 * over.
-		 *
-		 * Force all 8D-8D-8D flashes to use an address width of 4 to
-		 * avoid this situation.
-		 */
-		nor->addr_width = 4;
-	} else if (nor->info->addr_width) {
-		nor->addr_width = nor->info->addr_width;
-	} else {
-		nor->addr_width = 3;
-	}
-
-	if (nor->addr_width == 3 && nor->mtd.size > 0x1000000) {
-		/* enable 4-byte addressing if the device exceeds 16MiB */
-		nor->addr_width = 4;
-	}
-
-	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
-		dev_dbg(nor->dev, "address width is too large: %u\n",
-			nor->addr_width);
-		return -EINVAL;
-	}
-
-	/* Set 4byte opcodes when possible. */
-	if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
-	    !(nor->flags & SNOR_F_HAS_4BAIT))
-		spi_nor_set_4byte_opcodes(nor);
-
-	return 0;
-}
-
 static void spi_nor_debugfs_init(struct spi_nor *nor,
 				 const struct flash_info *info)
 {
@@ -3204,15 +3210,12 @@  int spi_nor_scan(struct spi_nor *nor, const char *name,
 	 * - select op codes for (Fast) Read, Page Program and Sector Erase.
 	 * - set the number of dummy cycles (mode cycles + wait states).
 	 * - set the SPI protocols for register and memory accesses.
+	 * - set the address width.
 	 */
 	ret = spi_nor_setup(nor, hwcaps);
 	if (ret)
 		return ret;
 
-	ret = spi_nor_set_addr_width(nor);
-	if (ret)
-		return ret;
-
 	/* Send all the required SPI flash commands to initialize device */
 	ret = spi_nor_init(nor);
 	if (ret)