All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
@ 2020-09-22 11:42 ` Chuanhong Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:42 UTC (permalink / raw)
  To: linux-spi
  Cc: bayi.cheng, Chuanhong Guo, stable, Mark Brown, Matthias Brugger,
	linux-arm-kernel, linux-mediatek, linux-kernel

CLK_TO_US macro is used to calculate potential transfer time for various
timeout handling. However it overflows on transfer bigger than 512 bytes
because it first did (len * 8 * 1000000).
This controller typically operates at 45MHz. This patch did 2 things:
1. calculate clock / 1000000 first
2. add a 4M transfer size cap so that the final timeout in DMA reading
   doesn't overflow

Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 6e6ca2b8e6c82..619313db42c0e 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -89,7 +89,7 @@
 // Buffered page program can do one 128-byte transfer
 #define MTK_NOR_PP_SIZE			128
 
-#define CLK_TO_US(sp, clkcnt)		((clkcnt) * 1000000 / sp->spi_freq)
+#define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
 
 struct mtk_nor {
 	struct spi_controller *ctlr;
@@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
 		if ((op->data.dir == SPI_MEM_DATA_IN) &&
 		    mtk_nor_match_read(op)) {
+			// limit size to prevent timeout calculation overflow
+			if (op->data.nbytes > 0x2000000)
+				op->data.nbytes = 0x2000000;
+
 			if ((op->addr.val & MTK_NOR_DMA_ALIGN_MASK) ||
 			    (op->data.nbytes < MTK_NOR_DMA_ALIGN))
 				op->data.nbytes = 1;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
@ 2020-09-22 11:42 ` Chuanhong Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:42 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, stable, Mark Brown, linux-mediatek, bayi.cheng,
	Matthias Brugger, Chuanhong Guo, linux-arm-kernel

CLK_TO_US macro is used to calculate potential transfer time for various
timeout handling. However it overflows on transfer bigger than 512 bytes
because it first did (len * 8 * 1000000).
This controller typically operates at 45MHz. This patch did 2 things:
1. calculate clock / 1000000 first
2. add a 4M transfer size cap so that the final timeout in DMA reading
   doesn't overflow

Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 6e6ca2b8e6c82..619313db42c0e 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -89,7 +89,7 @@
 // Buffered page program can do one 128-byte transfer
 #define MTK_NOR_PP_SIZE			128
 
-#define CLK_TO_US(sp, clkcnt)		((clkcnt) * 1000000 / sp->spi_freq)
+#define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
 
 struct mtk_nor {
 	struct spi_controller *ctlr;
@@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
 		if ((op->data.dir == SPI_MEM_DATA_IN) &&
 		    mtk_nor_match_read(op)) {
+			// limit size to prevent timeout calculation overflow
+			if (op->data.nbytes > 0x2000000)
+				op->data.nbytes = 0x2000000;
+
 			if ((op->addr.val & MTK_NOR_DMA_ALIGN_MASK) ||
 			    (op->data.nbytes < MTK_NOR_DMA_ALIGN))
 				op->data.nbytes = 1;
-- 
2.26.2


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
@ 2020-09-22 11:42 ` Chuanhong Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:42 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, stable, Mark Brown, linux-mediatek, bayi.cheng,
	Matthias Brugger, Chuanhong Guo, linux-arm-kernel

CLK_TO_US macro is used to calculate potential transfer time for various
timeout handling. However it overflows on transfer bigger than 512 bytes
because it first did (len * 8 * 1000000).
This controller typically operates at 45MHz. This patch did 2 things:
1. calculate clock / 1000000 first
2. add a 4M transfer size cap so that the final timeout in DMA reading
   doesn't overflow

Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 6e6ca2b8e6c82..619313db42c0e 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -89,7 +89,7 @@
 // Buffered page program can do one 128-byte transfer
 #define MTK_NOR_PP_SIZE			128
 
-#define CLK_TO_US(sp, clkcnt)		((clkcnt) * 1000000 / sp->spi_freq)
+#define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
 
 struct mtk_nor {
 	struct spi_controller *ctlr;
@@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
 		if ((op->data.dir == SPI_MEM_DATA_IN) &&
 		    mtk_nor_match_read(op)) {
+			// limit size to prevent timeout calculation overflow
+			if (op->data.nbytes > 0x2000000)
+				op->data.nbytes = 0x2000000;
+
 			if ((op->addr.val & MTK_NOR_DMA_ALIGN_MASK) ||
 			    (op->data.nbytes < MTK_NOR_DMA_ALIGN))
 				op->data.nbytes = 1;
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
  2020-09-22 11:42 ` Chuanhong Guo
  (?)
@ 2020-09-22 11:47   ` Chuanhong Guo
  -1 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:47 UTC (permalink / raw)
  To: linux-spi
  Cc: Bayi Cheng (程八意),
	stable, Mark Brown, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support, open list

On Tue, Sep 22, 2020 at 7:43 PM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> CLK_TO_US macro is used to calculate potential transfer time for various
> timeout handling. However it overflows on transfer bigger than 512 bytes
> because it first did (len * 8 * 1000000).
> This controller typically operates at 45MHz. This patch did 2 things:
> 1. calculate clock / 1000000 first
> 2. add a 4M transfer size cap so that the final timeout in DMA reading
>    doesn't overflow
>
> Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  drivers/spi/spi-mtk-nor.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index 6e6ca2b8e6c82..619313db42c0e 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -89,7 +89,7 @@
>  // Buffered page program can do one 128-byte transfer
>  #define MTK_NOR_PP_SIZE                        128
>
> -#define CLK_TO_US(sp, clkcnt)          ((clkcnt) * 1000000 / sp->spi_freq)
> +#define CLK_TO_US(sp, clkcnt)          DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
>
>  struct mtk_nor {
>         struct spi_controller *ctlr;
> @@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>         if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
>                 if ((op->data.dir == SPI_MEM_DATA_IN) &&
>                     mtk_nor_match_read(op)) {
> +                       // limit size to prevent timeout calculation overflow
> +                       if (op->data.nbytes > 0x2000000)
> +                               op->data.nbytes = 0x2000000;
> +

Sorry, wrong patch. This cap should be 4M not 32M. I'll send a v2 immediately.

-- 
Regards,
Chuanhong Guo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
@ 2020-09-22 11:47   ` Chuanhong Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:47 UTC (permalink / raw)
  To: linux-spi
  Cc: open list, stable, Mark Brown,
	moderated list:ARM/Mediatek SoC support,
	Bayi Cheng (程八意),
	Matthias Brugger, moderated list:ARM/Mediatek SoC support

On Tue, Sep 22, 2020 at 7:43 PM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> CLK_TO_US macro is used to calculate potential transfer time for various
> timeout handling. However it overflows on transfer bigger than 512 bytes
> because it first did (len * 8 * 1000000).
> This controller typically operates at 45MHz. This patch did 2 things:
> 1. calculate clock / 1000000 first
> 2. add a 4M transfer size cap so that the final timeout in DMA reading
>    doesn't overflow
>
> Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  drivers/spi/spi-mtk-nor.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index 6e6ca2b8e6c82..619313db42c0e 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -89,7 +89,7 @@
>  // Buffered page program can do one 128-byte transfer
>  #define MTK_NOR_PP_SIZE                        128
>
> -#define CLK_TO_US(sp, clkcnt)          ((clkcnt) * 1000000 / sp->spi_freq)
> +#define CLK_TO_US(sp, clkcnt)          DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
>
>  struct mtk_nor {
>         struct spi_controller *ctlr;
> @@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>         if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
>                 if ((op->data.dir == SPI_MEM_DATA_IN) &&
>                     mtk_nor_match_read(op)) {
> +                       // limit size to prevent timeout calculation overflow
> +                       if (op->data.nbytes > 0x2000000)
> +                               op->data.nbytes = 0x2000000;
> +

Sorry, wrong patch. This cap should be 4M not 32M. I'll send a v2 immediately.

-- 
Regards,
Chuanhong Guo

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow
@ 2020-09-22 11:47   ` Chuanhong Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Chuanhong Guo @ 2020-09-22 11:47 UTC (permalink / raw)
  To: linux-spi
  Cc: open list, stable, Mark Brown,
	moderated list:ARM/Mediatek SoC support,
	Bayi Cheng (程八意),
	Matthias Brugger, moderated list:ARM/Mediatek SoC support

On Tue, Sep 22, 2020 at 7:43 PM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> CLK_TO_US macro is used to calculate potential transfer time for various
> timeout handling. However it overflows on transfer bigger than 512 bytes
> because it first did (len * 8 * 1000000).
> This controller typically operates at 45MHz. This patch did 2 things:
> 1. calculate clock / 1000000 first
> 2. add a 4M transfer size cap so that the final timeout in DMA reading
>    doesn't overflow
>
> Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  drivers/spi/spi-mtk-nor.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index 6e6ca2b8e6c82..619313db42c0e 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -89,7 +89,7 @@
>  // Buffered page program can do one 128-byte transfer
>  #define MTK_NOR_PP_SIZE                        128
>
> -#define CLK_TO_US(sp, clkcnt)          ((clkcnt) * 1000000 / sp->spi_freq)
> +#define CLK_TO_US(sp, clkcnt)          DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
>
>  struct mtk_nor {
>         struct spi_controller *ctlr;
> @@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>         if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
>                 if ((op->data.dir == SPI_MEM_DATA_IN) &&
>                     mtk_nor_match_read(op)) {
> +                       // limit size to prevent timeout calculation overflow
> +                       if (op->data.nbytes > 0x2000000)
> +                               op->data.nbytes = 0x2000000;
> +

Sorry, wrong patch. This cap should be 4M not 32M. I'll send a v2 immediately.

-- 
Regards,
Chuanhong Guo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-22 11:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 11:42 [PATCH] spi: spi-mtk-nor: fix timeout calculation overflow Chuanhong Guo
2020-09-22 11:42 ` Chuanhong Guo
2020-09-22 11:42 ` Chuanhong Guo
2020-09-22 11:47 ` Chuanhong Guo
2020-09-22 11:47   ` Chuanhong Guo
2020-09-22 11:47   ` Chuanhong Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.