diff mbox series

spi: spi-mem: ease checks in dtr_supports_op()

Message ID 20221020083424.86848-1-d-gole@ti.com
State Changes Requested
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series spi: spi-mem: ease checks in dtr_supports_op() | expand

Commit Message

Dhruva Gole Oct. 20, 2022, 8:34 a.m. UTC
Remove the extra conditions that cause some cases to fail prematurely
like if the data number of bytes is odd. The controller can handle odd
number of bytes data read in DTR Mode. Don't fail supports op for this
condition.

Signed-off-by: Dhruva Gole <d-gole@ti.com>
---

For a deeper context, refer to a cover letter from an earlier patch
series:
https://lore.kernel.org/u-boot/20221019064759.493607-1-d-gole@ti.com/T/#m3fd93bdcc30b3b5faada6abe45a4104388afc300
Here, I am trying to boot from OSPI Flash present on board the AM62-SK
EVM. However, despite enabling the necessary configs and DT nodes, my
boot flow seemed to be failing. I then came to know that the condition
causing this failure was that my generated DTB was of odd number of
bytes.
So, while loading the dtb from the Octal SPI NOR Flash to the memory,
the data.nbytes was odd which made the supports_op
function return false. This check feels a little too strict at the
spi-mem stage and we should let the controller decide what to do in case
of odd bytes in DTR. After applying this patch, I was able to
succesfully boot the AM62SK EVM without any issues.

I would also like to justify this patch by pointing the community to the
equivalent code in the linux kernel, in drivers/spi/spi-mem.c, where this check
is absent as well.

The controller does work with odd number of bytes and I have not seen
any sort of bugs in the absence of this supports_op check. Hence, feel
that it is safe enough to discard this check from here.

 drivers/spi/spi-mem.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Dhruva Gole Oct. 20, 2022, 8:48 a.m. UTC | #1
Forgot to add in CC,
+ Jagan Teki <jagan@amarulasolutions.com> (maintainer:SPI)

On 20/10/22 14:04, Dhruva Gole wrote:
> Remove the extra conditions that cause some cases to fail prematurely
> like if the data number of bytes is odd. The controller can handle odd
> number of bytes data read in DTR Mode. Don't fail supports op for this
> condition.
> 
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
> 
> For a deeper context, refer to a cover letter from an earlier patch
> series:
> https://lore.kernel.org/u-boot/20221019064759.493607-1-d-gole@ti.com/T/#m3fd93bdcc30b3b5faada6abe45a4104388afc300
> Here, I am trying to boot from OSPI Flash present on board the AM62-SK
> EVM. However, despite enabling the necessary configs and DT nodes, my
> boot flow seemed to be failing. I then came to know that the condition
> causing this failure was that my generated DTB was of odd number of
> bytes.
> So, while loading the dtb from the Octal SPI NOR Flash to the memory,
> the data.nbytes was odd which made the supports_op
> function return false. This check feels a little too strict at the
> spi-mem stage and we should let the controller decide what to do in case
> of odd bytes in DTR. After applying this patch, I was able to
> succesfully boot the AM62SK EVM without any issues.
> 
> I would also like to justify this patch by pointing the community to the
> equivalent code in the linux kernel, in drivers/spi/spi-mem.c, where this check
> is absent as well.
> 
> The controller does work with odd number of bytes and I have not seen
> any sort of bugs in the absence of this supports_op check. Hence, feel
> that it is safe enough to discard this check from here.
> 
>   drivers/spi/spi-mem.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 8e8995fc537f..eecc13bec90d 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -181,10 +181,6 @@ bool spi_mem_dtr_supports_op(struct spi_slave *slave,
>   	if (op->dummy.nbytes && op->dummy.buswidth == 8 && op->dummy.nbytes % 2)
>   		return false;
>   
> -	if (op->data.dir != SPI_MEM_NO_DATA &&
> -	    op->dummy.buswidth == 8 && op->data.nbytes % 2)
> -		return false;
> -
>   	return spi_mem_check_buswidth(slave, op);
>   }
>   EXPORT_SYMBOL_GPL(spi_mem_dtr_supports_op);
Jagan Teki Oct. 23, 2022, 5:29 a.m. UTC | #2
On Thu, Oct 20, 2022 at 2:04 PM Dhruva Gole <d-gole@ti.com> wrote:
>
> Remove the extra conditions that cause some cases to fail prematurely
> like if the data number of bytes is odd. The controller can handle odd
> number of bytes data read in DTR Mode. Don't fail supports op for this
> condition.
>
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>
> For a deeper context, refer to a cover letter from an earlier patch
> series:
> https://lore.kernel.org/u-boot/20221019064759.493607-1-d-gole@ti.com/T/#m3fd93bdcc30b3b5faada6abe45a4104388afc300
> Here, I am trying to boot from OSPI Flash present on board the AM62-SK
> EVM. However, despite enabling the necessary configs and DT nodes, my
> boot flow seemed to be failing. I then came to know that the condition
> causing this failure was that my generated DTB was of odd number of
> bytes.
> So, while loading the dtb from the Octal SPI NOR Flash to the memory,
> the data.nbytes was odd which made the supports_op
> function return false. This check feels a little too strict at the
> spi-mem stage and we should let the controller decide what to do in case
> of odd bytes in DTR. After applying this patch, I was able to
> succesfully boot the AM62SK EVM without any issues.
>
> I would also like to justify this patch by pointing the community to the
> equivalent code in the linux kernel, in drivers/spi/spi-mem.c, where this check
> is absent as well.
>
> The controller does work with odd number of bytes and I have not seen
> any sort of bugs in the absence of this supports_op check. Hence, feel
> that it is safe enough to discard this check from here.

Require better wrapup of above text to be part of commit body.
diff mbox series

Patch

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 8e8995fc537f..eecc13bec90d 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -181,10 +181,6 @@  bool spi_mem_dtr_supports_op(struct spi_slave *slave,
 	if (op->dummy.nbytes && op->dummy.buswidth == 8 && op->dummy.nbytes % 2)
 		return false;
 
-	if (op->data.dir != SPI_MEM_NO_DATA &&
-	    op->dummy.buswidth == 8 && op->data.nbytes % 2)
-		return false;
-
 	return spi_mem_check_buswidth(slave, op);
 }
 EXPORT_SYMBOL_GPL(spi_mem_dtr_supports_op);