* [bug report] mtd: spi-nor: Introduce 'struct spi_nor_controller_ops'
@ 2019-10-25 9:03 Dan Carpenter
2019-10-25 11:24 ` Tudor.Ambarus
0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2019-10-25 9:03 UTC (permalink / raw)
To: tudor.ambarus; +Cc: linux-mtd
Hello Tudor Ambarus,
This is a semi-automatic email about new static checker warnings.
The patch 453977875364: "mtd: spi-nor: Introduce 'struct
spi_nor_controller_ops'" from Sep 24, 2019, leads to the following
Smatch complaint:
drivers/mtd/spi-nor/spi-nor.c:967 spi_nor_erase_sector()
error: we previously assumed 'nor->controller_ops' could be null (see line 945)
drivers/mtd/spi-nor/spi-nor.c
944
945 if (nor->controller_ops && nor->controller_ops->erase)
^^^^^^^^^^^^^^^^^^^
Can this really be NULL? Probably this check can be removed...
946 return nor->controller_ops->erase(nor, addr);
947
948 if (nor->spimem) {
949 struct spi_mem_op op =
950 SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
951 SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
952 SPI_MEM_OP_NO_DUMMY,
953 SPI_MEM_OP_NO_DATA);
954
955 return spi_mem_exec_op(nor->spimem, &op);
956 }
957
958 /*
959 * Default implementation, if driver doesn't have a specialized HW
960 * control
961 */
962 for (i = nor->addr_width - 1; i >= 0; i--) {
963 nor->bouncebuf[i] = addr & 0xff;
964 addr >>= 8;
965 }
966
967 return nor->controller_ops->write_reg(nor, nor->erase_opcode,
^^^^^^^^^^^^^^^^^^^^^
The patch adds a new unchecked dereference.
968 nor->bouncebuf, nor->addr_width);
969 }
regards,
dan carpenter
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [bug report] mtd: spi-nor: Introduce 'struct spi_nor_controller_ops'
2019-10-25 9:03 [bug report] mtd: spi-nor: Introduce 'struct spi_nor_controller_ops' Dan Carpenter
@ 2019-10-25 11:24 ` Tudor.Ambarus
2019-10-25 12:32 ` [PATCH] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
0 siblings, 1 reply; 12+ messages in thread
From: Tudor.Ambarus @ 2019-10-25 11:24 UTC (permalink / raw)
To: dan.carpenter; +Cc: linux-mtd
On 10/25/2019 12:03 PM, Dan Carpenter wrote:
> External E-Mail
>
>
> Hello Tudor Ambarus,
Hi,
The code should be fine, see below.
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 453977875364: "mtd: spi-nor: Introduce 'struct
> spi_nor_controller_ops'" from Sep 24, 2019, leads to the following
> Smatch complaint:
>
> drivers/mtd/spi-nor/spi-nor.c:967 spi_nor_erase_sector()
> error: we previously assumed 'nor->controller_ops' could be null (see line 945)
>
> drivers/mtd/spi-nor/spi-nor.c
> 944
> 945 if (nor->controller_ops && nor->controller_ops->erase)
> ^^^^^^^^^^^^^^^^^^^
> Can this really be NULL? Probably this check can be removed...
nor->controller_ops can be NULL when the controller is under the SPI-MEM
interface, i.e. nor->spimem != NULL.
If not under SPI-MEM, the controller implements its own operations.
controller_ops->erase is optional, can be NULL. This check verifies if the
controller implements its own operations and if the optional one (erase) is
present or not.
>
> 946 return nor->controller_ops->erase(nor, addr);
> 947
> 948 if (nor->spimem) {
> 949 struct spi_mem_op op =
> 950 SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
> 951 SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
> 952 SPI_MEM_OP_NO_DUMMY,
> 953 SPI_MEM_OP_NO_DATA);
> 954
> 955 return spi_mem_exec_op(nor->spimem, &op);
> 956 }
> 957
> 958 /*
> 959 * Default implementation, if driver doesn't have a specialized HW
> 960 * control
> 961 */
> 962 for (i = nor->addr_width - 1; i >= 0; i--) {
> 963 nor->bouncebuf[i] = addr & 0xff;
> 964 addr >>= 8;
> 965 }
> 966
> 967 return nor->controller_ops->write_reg(nor, nor->erase_opcode,> ^^^^^^^^^^^^^^^^^^^^^
> The patch adds a new unchecked dereference.
When the controller implements its own ops, nor->controller_ops->write_reg is
mandatory and backed-up by the:
static int spi_nor_check(struct spi_nor *nor)
{
if (!nor->dev ||
(!nor->spimem && nor->controller_ops &&
(!nor->controller_ops->read ||
!nor->controller_ops->write ||
!nor->controller_ops->read_reg ||
!nor->controller_ops->write_reg))) {
pr_err("spi-nor: please fill all the necessary fields!\n");
return -EINVAL;
}
Cheers,
ta
>
> 968 nor->bouncebuf, nor->addr_width);
> 969 }
>
> regards,
> dan carpenter
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mtd: spi-nor: Move condition to avoid a NULL check
2019-10-25 11:24 ` Tudor.Ambarus
@ 2019-10-25 12:32 ` Tudor.Ambarus
2019-10-25 12:37 ` Dan Carpenter
2019-10-25 14:28 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Tudor.Ambarus
0 siblings, 2 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-10-25 12:32 UTC (permalink / raw)
To: dan.carpenter, linux-mtd; +Cc: Tudor.Ambarus
From: Tudor Ambarus <tudor.ambarus@microchip.com>
When the controller is not under the SPI-MEM interface it may implement
the optional controller_ops->erase() method.
nor->spimem and nor->controller_ops are disjunctive. Move the
nor->controller_ops->erase != NULL check as an 'else if' case to
nor->spimem, in order to avoid the nor->controller_ops != NULL
check.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
Intended for better readability.
drivers/mtd/spi-nor/spi-nor.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 07ec7841ec32..d0e1502d21ce 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1118,9 +1118,6 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
addr = spi_nor_convert_addr(nor, addr);
- if (nor->controller_ops && nor->controller_ops->erase)
- return nor->controller_ops->erase(nor, addr);
-
if (nor->spimem) {
struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
@@ -1129,6 +1126,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
SPI_MEM_OP_NO_DATA);
return spi_mem_exec_op(nor->spimem, &op);
+ } else if (nor->controller_ops->erase) {
+ return nor->controller_ops->erase(nor, addr);
}
/*
--
2.9.5
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mtd: spi-nor: Move condition to avoid a NULL check
2019-10-25 12:32 ` [PATCH] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
@ 2019-10-25 12:37 ` Dan Carpenter
2019-10-25 14:28 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Tudor.Ambarus
1 sibling, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2019-10-25 12:37 UTC (permalink / raw)
To: Tudor.Ambarus; +Cc: linux-mtd
On Fri, Oct 25, 2019 at 12:32:25PM +0000, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> When the controller is not under the SPI-MEM interface it may implement
> the optional controller_ops->erase() method.
>
> nor->spimem and nor->controller_ops are disjunctive. Move the
> nor->controller_ops->erase != NULL check as an 'else if' case to
> nor->spimem, in order to avoid the nor->controller_ops != NULL
> check.
>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> Intended for better readability.
Thanks!
regards,
dan carpenter
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive
2019-10-25 12:32 ` [PATCH] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
2019-10-25 12:37 ` Dan Carpenter
@ 2019-10-25 14:28 ` Tudor.Ambarus
2019-10-25 14:28 ` [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
2019-11-11 9:02 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Boris Brezillon
1 sibling, 2 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-10-25 14:28 UTC (permalink / raw)
To: dan.carpenter, linux-mtd; +Cc: Tudor.Ambarus
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Expand the spi_nor_check() to make sure that nor->spimem and
nor->controller_ops are mutual exclusive.
Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: new patch
drivers/mtd/spi-nor/spi-nor.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a6f9f833c862..b452d3d0de28 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2741,6 +2741,12 @@ static int spi_nor_check(struct spi_nor *nor)
return -EINVAL;
}
+ if ((nor->spimem && nor->controller_ops) ||
+ (!nor->spimem && !nor->controller_ops)) {
+ dev_err(nor->dev, "nor->spimem and nor->controller_ops are mutual exclusive\n");
+ return -EINVAL;
+ }
+
return 0;
}
--
2.9.5
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check
2019-10-25 14:28 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Tudor.Ambarus
@ 2019-10-25 14:28 ` Tudor.Ambarus
2019-11-11 9:08 ` Boris Brezillon
2019-11-11 19:28 ` Tudor.Ambarus
2019-11-11 9:02 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Boris Brezillon
1 sibling, 2 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-10-25 14:28 UTC (permalink / raw)
To: dan.carpenter, linux-mtd; +Cc: Tudor.Ambarus
From: Tudor Ambarus <tudor.ambarus@microchip.com>
When the controller is not under the SPI-MEM interface it may implement
the optional controller_ops->erase() method.
nor->spimem and nor->controller_ops are mutual exclusive. Move the
nor->controller_ops->erase != NULL check as an 'else if' case to
nor->spimem, in order to avoid the nor->controller_ops != NULL
check.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: add Reported-by tag, amend commit description.
drivers/mtd/spi-nor/spi-nor.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b452d3d0de28..8eaf097098d9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -942,9 +942,6 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
addr = spi_nor_convert_addr(nor, addr);
- if (nor->controller_ops && nor->controller_ops->erase)
- return nor->controller_ops->erase(nor, addr);
-
if (nor->spimem) {
struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
@@ -953,6 +950,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
SPI_MEM_OP_NO_DATA);
return spi_mem_exec_op(nor->spimem, &op);
+ } else if (nor->controller_ops->erase) {
+ return nor->controller_ops->erase(nor, addr);
}
/*
--
2.9.5
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive
2019-10-25 14:28 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Tudor.Ambarus
2019-10-25 14:28 ` [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
@ 2019-11-11 9:02 ` Boris Brezillon
2019-11-11 9:44 ` [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive Tudor.Ambarus
1 sibling, 1 reply; 12+ messages in thread
From: Boris Brezillon @ 2019-11-11 9:02 UTC (permalink / raw)
To: Tudor.Ambarus; +Cc: linux-mtd, dan.carpenter
On Fri, 25 Oct 2019 14:28:34 +0000
<Tudor.Ambarus@microchip.com> wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> Expand the spi_nor_check() to make sure that nor->spimem and
> nor->controller_ops are mutual exclusive.
^mutually
>
> Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2: new patch
>
> drivers/mtd/spi-nor/spi-nor.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index a6f9f833c862..b452d3d0de28 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2741,6 +2741,12 @@ static int spi_nor_check(struct spi_nor *nor)
> return -EINVAL;
> }
>
> + if ((nor->spimem && nor->controller_ops) ||
> + (!nor->spimem && !nor->controller_ops)) {
> + dev_err(nor->dev, "nor->spimem and nor->controller_ops are mutual exclusive\n");
Hm, the test does more than checking that only one of nor->spimem
nor->controller_ops is set, it also checks that at least one of them is
provided. Maybe you should split those tests and have a different error
message for each, or make the error message more generic.
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check
2019-10-25 14:28 ` [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
@ 2019-11-11 9:08 ` Boris Brezillon
2019-11-11 19:28 ` Tudor.Ambarus
1 sibling, 0 replies; 12+ messages in thread
From: Boris Brezillon @ 2019-11-11 9:08 UTC (permalink / raw)
To: Tudor.Ambarus; +Cc: linux-mtd, dan.carpenter
On Fri, 25 Oct 2019 14:28:36 +0000
<Tudor.Ambarus@microchip.com> wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> When the controller is not under the SPI-MEM interface it may implement
> the optional controller_ops->erase() method.
>
> nor->spimem and nor->controller_ops are mutual exclusive. Move the
^mutually
> nor->controller_ops->erase != NULL check as an 'else if' case to
> nor->spimem, in order to avoid the nor->controller_ops != NULL
> check.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> v2: add Reported-by tag, amend commit description.
>
> drivers/mtd/spi-nor/spi-nor.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index b452d3d0de28..8eaf097098d9 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -942,9 +942,6 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>
> addr = spi_nor_convert_addr(nor, addr);
>
> - if (nor->controller_ops && nor->controller_ops->erase)
> - return nor->controller_ops->erase(nor, addr);
> -
> if (nor->spimem) {
> struct spi_mem_op op =
> SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
> @@ -953,6 +950,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
> SPI_MEM_OP_NO_DATA);
>
> return spi_mem_exec_op(nor->spimem, &op);
> + } else if (nor->controller_ops->erase) {
> + return nor->controller_ops->erase(nor, addr);
> }
>
> /*
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive
2019-11-11 9:02 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Boris Brezillon
@ 2019-11-11 9:44 ` Tudor.Ambarus
2019-11-11 15:59 ` Boris Brezillon
2019-11-11 19:26 ` Tudor.Ambarus
0 siblings, 2 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-11-11 9:44 UTC (permalink / raw)
To: boris.brezillon; +Cc: linux-mtd, dan.carpenter, Tudor.Ambarus
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Expand the spi_nor_check() to make sure that nor->spimem and
nor->controller_ops are mutually exclusive.
Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v3:
- split checks for better error accuracy
- s/mutual/mutually
drivers/mtd/spi-nor/spi-nor.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 824649eecd59..f5d24ccf5108 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2878,6 +2878,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
static int spi_nor_check(struct spi_nor *nor)
{
if (!nor->dev ||
+ (!nor->spimem && !nor->controller_ops) ||
(!nor->spimem && nor->controller_ops &&
(!nor->controller_ops->read ||
!nor->controller_ops->write ||
@@ -2887,6 +2888,11 @@ static int spi_nor_check(struct spi_nor *nor)
return -EINVAL;
}
+ if (nor->spimem && nor->controller_ops) {
+ dev_err(nor->dev, "nor->spimem and nor->controller_ops are mutually exclusive, please set just one of them.\n");
+ return -EINVAL;
+ }
+
return 0;
}
--
2.9.5
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive
2019-11-11 9:44 ` [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive Tudor.Ambarus
@ 2019-11-11 15:59 ` Boris Brezillon
2019-11-11 19:26 ` Tudor.Ambarus
1 sibling, 0 replies; 12+ messages in thread
From: Boris Brezillon @ 2019-11-11 15:59 UTC (permalink / raw)
To: Tudor.Ambarus; +Cc: linux-mtd, dan.carpenter
On Mon, 11 Nov 2019 09:44:08 +0000
<Tudor.Ambarus@microchip.com> wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> Expand the spi_nor_check() to make sure that nor->spimem and
> nor->controller_ops are mutually exclusive.
>
> Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> v3:
> - split checks for better error accuracy
> - s/mutual/mutually
>
> drivers/mtd/spi-nor/spi-nor.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 824649eecd59..f5d24ccf5108 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2878,6 +2878,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
> static int spi_nor_check(struct spi_nor *nor)
> {
> if (!nor->dev ||
> + (!nor->spimem && !nor->controller_ops) ||
> (!nor->spimem && nor->controller_ops &&
> (!nor->controller_ops->read ||
> !nor->controller_ops->write ||
> @@ -2887,6 +2888,11 @@ static int spi_nor_check(struct spi_nor *nor)
> return -EINVAL;
> }
>
> + if (nor->spimem && nor->controller_ops) {
> + dev_err(nor->dev, "nor->spimem and nor->controller_ops are mutually exclusive, please set just one of them.\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive
2019-11-11 9:44 ` [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive Tudor.Ambarus
2019-11-11 15:59 ` Boris Brezillon
@ 2019-11-11 19:26 ` Tudor.Ambarus
1 sibling, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-11-11 19:26 UTC (permalink / raw)
To: boris.brezillon; +Cc: linux-mtd, dan.carpenter
On 11/11/2019 11:44 AM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> Expand the spi_nor_check() to make sure that nor->spimem and
> nor->controller_ops are mutually exclusive.
>
> Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v3:
> - split checks for better error accuracy
> - s/mutual/mutually
>
> drivers/mtd/spi-nor/spi-nor.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Applied to spi-nor/next.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check
2019-10-25 14:28 ` [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
2019-11-11 9:08 ` Boris Brezillon
@ 2019-11-11 19:28 ` Tudor.Ambarus
1 sibling, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2019-11-11 19:28 UTC (permalink / raw)
To: dan.carpenter, linux-mtd
On 10/25/2019 05:28 PM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> When the controller is not under the SPI-MEM interface it may implement
> the optional controller_ops->erase() method.
>
> nor->spimem and nor->controller_ops are mutual exclusive. Move the
> nor->controller_ops->erase != NULL check as an 'else if' case to
> nor->spimem, in order to avoid the nor->controller_ops != NULL
> check.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2: add Reported-by tag, amend commit description.
>
> drivers/mtd/spi-nor/spi-nor.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Amended commit description, s/mutual/mutually. Patch applied to spi-nor/next.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-11 19:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25 9:03 [bug report] mtd: spi-nor: Introduce 'struct spi_nor_controller_ops' Dan Carpenter
2019-10-25 11:24 ` Tudor.Ambarus
2019-10-25 12:32 ` [PATCH] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
2019-10-25 12:37 ` Dan Carpenter
2019-10-25 14:28 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Tudor.Ambarus
2019-10-25 14:28 ` [PATCH v2 2/2] mtd: spi-nor: Move condition to avoid a NULL check Tudor.Ambarus
2019-11-11 9:08 ` Boris Brezillon
2019-11-11 19:28 ` Tudor.Ambarus
2019-11-11 9:02 ` [PATCH v2 1/2] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutual exclusive Boris Brezillon
2019-11-11 9:44 ` [PATCH] mtd: spi-nor: Make sure nor->spimem and nor->controller_ops are mutually exclusive Tudor.Ambarus
2019-11-11 15:59 ` Boris Brezillon
2019-11-11 19:26 ` Tudor.Ambarus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).