You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "FelipeMdeO (via GitHub)" <gi...@apache.org> on 2024/09/01 23:54:20 UTC
[I] [BUG] SPI Slave driver issue [nuttx]
FelipeMdeO opened a new issue, #13260:
URL: https://github.com/apache/nuttx/issues/13260
### Description / Steps to reproduce the issue
Hello All.
I am Facing some issues during slave driver development.
Here I can report and talk about change done in drivers/spi/spi_slave_driver.c. PR is: https://github.com/apache/nuttx/pull/10727
This PR do `bind` during file open and `unbind` during file close. The issues is that the bind is necessary to configure configure slave controller, so, in current master, all spi transaction when file is closed will be lost.
Before this PR all spi message is saved in lower half buffer, when we do read in top half, we get data from lower half, now lower half only get spi data while top half is waiting for.
@Donny9 let's talk about this PR, please?
@acassis Can you help me to check about expected behavior, Do we need save information after driver initialization or only during open execution?
### On which OS does this issue occur?
[Linux]
### What is the version of your OS?
Ubuntu 24.0
### NuttX Version
master
### Issue Architecture
[all]
### Issue Area
[Drivers]
### Verification
- [X] I have verified before submitting the report.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "FelipeMdeO (via GitHub)" <gi...@apache.org>.
FelipeMdeO commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2336796097
Hi @Donny9,
Sorry to insist, but I still haven't figured out how the read function works.
Look:
Let's consider that I'm trying to get 30 bytes, so `priv->rx_length = 30;`
`while (priv->rx_length == 0)` will be false, so we jump for:
now, `read_bytes = 30;`
But priv->rx_buffer isn't updated, because it is necessary call `SPIS_CTRLR_QPOLL(priv->ctrlr);` what does not happen.
You explaned:
"However, if there is no data available, it needs to invoke SPIS_CTRLR_QPOLL to fetch data from the lower half."
I understand `priv->rx_length` always will different of zero, maybe the mistake is because we are doing sizeof instead just get value:
`priv->rx_length = MIN(buflen, **sizeof**(priv->rx_buffer));`
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2336821990
@Donny9 I think @FelipeMdeO is right, there is a logic error here:
```
priv->rx_length = MIN(buflen, sizeof(priv->rx_buffer));
ret = nxmutex_lock(&priv->lock);
if (ret < 0)
{
spierr("Failed to get exclusive access: %d\n", ret);
return ret;
}
while (priv->rx_length == 0)
```
The `MIN(buflen, sizeof(priv->rx_buffer));` never will return 0, unless the user ask to read 0 bytes, what makes no sense either.
Please review the code or submit a minimum example showing that it will work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "Donny9 (via GitHub)" <gi...@apache.org>.
Donny9 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2335195884
> @Donny9 all drivers on NuttX needs some example application to prove it is working and to help to test. At this moment we don't know if the BUG is on ESP32-C6 or in the SPI Slave driver.
>
> Could you please submit a simple/minimal apps/testing/spislave/ example just to receive data from other board (using spitool) and transmit back to the board?
okay, i will submit later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2334126382
> @Donny9 all drivers on NuttX needs some example application to prove it is working and to help to test. At this moment we don't know if the BUG is on ESP32-C6 or in the SPI Slave driver.
>
> Could you please submit a simple/minimal apps/testing/spislave/ example just to receive data from other board (using spitool) and transmit back to the board?
@acassis we will provide a new rpmsg transport on top of spi master and slave, after that you can test the basic communication through `rpmsg ping` command from nsh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2334130227
@xiaoxiang781216 sounds good, but as you said this rpmsg will delay to come, so maybe a simple sample code is enough for let @FelipeMdeO to test it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "Donny9 (via GitHub)" <gi...@apache.org>.
Donny9 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2333052348
> @Donny9 , Thank you very much for your explanation.
>
> I understand the implementation now and I agree, it makes sense, initialize function configure spi peripheral, open enable and close disable it.
>
> Donny, I cannot do it work yet, can help me to understand other point, please? In the PR: #10782 We have the change below: 
>
> but: `priv->rx_length = MIN(buflen, sizeof(priv->rx_buffer));`
>
> I think that: `while (priv->rx_length == 0)` always will be false. What did I miss?
@FelipeMdeO Hello:
The spi_slave_read function is used by an application to read data from the top half. It first checks if there is any data available (by verifying if priv->rx_length is not equal to 0):
If there is data available, it directly copies the data from priv->rx_buffer to the user's buffer using memcpy.
However, if there is no data available, it needs to invoke SPIS_CTRLR_QPOLL to fetch data from the lower half.
The SPIS_CTRLR_QPOLL function will call SPIS_DEV_RECEIVE to push data from the lower half into the priv->rx_buffer of the top half and updates priv->rx_length accordingly. Once this is done, the while loop in spi_slave_read will exit , and memcpy data, finish read~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "Donny9 (via GitHub)" <gi...@apache.org>.
Donny9 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2330597553
@FelipeMdeO hello～
The purpose of this PR modification is to disable interrupts and the SPI slave controller when the application has not opened the SPI slave device, in order to conserve power consumption.
Before the device is opened by the user application, any data sent by the host is of no concern to the user application.
Even without this PR, if the host continuously sends data, the rxbuffer may overflow, resulting in data loss.
So, if the application does care about the data, it should open the device and use the poll interface to monitor the underlying data, thus preventing data loss.
Therefore, I recommend opening the device node to prevent data loss.
If you do not want to do so, you can, in your board code, initialize your SPI slave code while enabling interrupts by default. This will ensure that when the host sends data, it is received by the SPI slave lower half driver, and when you read it, the data is copied from the lower half to the top half.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2334057029
@Donny9 all drivers on NuttX needs some example application to prove it is working and to help to test. At this moment we don't know if the BUG is on ESP32-C6 or in the SPI Slave driver.
Could you please submit a simple/minimal apps/testing/spislave/ example just to receive data from other board (using spitool) and transmit back to the board?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2325184025
@xiaoxiang781216 is there some example using SPI Master <---> SPI Slave communication (maybe using RPMSG) ? It should be something useful to validate the driver and could help Felipe to reach his goal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis closed issue #13260: [BUG] SPI Slave driver issue
URL: https://github.com/apache/nuttx/issues/13260
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2330548782
> @xiaoxiang781216 is there some example using SPI Master <---> SPI Slave communication (maybe using RPMSG) ? It should be something useful to validate the driver and could help Felipe to reach his goal
Yes, we made rpmsg over spi master/slave recently, but it make the significant change to rptun driver framework., so it need more time to prepare the patch.
For spi slave change, @Donny9 will take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2324759273
@FelipeMdeO I think you are right, the modification seems suspicious! This is issue when someone adds a driver and don't add a testing application!
@xiaoxiang781216 could you please help and ask your team to review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "Donny9 (via GitHub)" <gi...@apache.org>.
Donny9 commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2336983808
> @Donny9 I think @FelipeMdeO is right, there is a logic error here:
>
> ```
> priv->rx_length = MIN(buflen, sizeof(priv->rx_buffer));
> ret = nxmutex_lock(&priv->lock);
> if (ret < 0)
> {
> spierr("Failed to get exclusive access: %d\n", ret);
> return ret;
> }
>
> while (priv->rx_length == 0)
> ```
>
> The `MIN(buflen, sizeof(priv->rx_buffer));` never will return 0, unless the user ask to read 0 bytes, what makes no sense either.
>
> Please review the code or submit a minimum example showing that it will work.
@acassis @FelipeMdeO You are right, sorry, We have reverted this PR（https://github.com/apache/nuttx/pull/8769) internally, but it hasn't been committed yet. It's true that it would cause the read function to stop working. Revert PR: https://github.com/apache/nuttx/pull/13337, @FelipeMdeO you can try with this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] SPI Slave driver issue [nuttx]
Posted by "FelipeMdeO (via GitHub)" <gi...@apache.org>.
FelipeMdeO commented on issue #13260:
URL: https://github.com/apache/nuttx/issues/13260#issuecomment-2332725144
@Donny9 , Thank you very much for your explanation.
I understand the implementation now and I agree, it makes sense, initialize function configure spi peripheral, open enable and close disable it.
Donny, I cannot do it work yet, can help me to understand other point, please?
In the PR: https://github.com/apache/nuttx/pull/10782
We have the change below:
but:
`priv->rx_length = MIN(buflen, sizeof(priv->rx_buffer));`
I think that:
`while (priv->rx_length == 0)` always will be false. What did I miss?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org