nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
Hi all, I am not familiar with the code from nuttx\arch\arm\src\stm32h7\stm32_spi.c but I believe that there is an issue. "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is defined. But in the spi_interrupt function, the txresult is used regardless of whether CONFIG_STM32H7_SPI_DMA is defined or not. This generates a build issue when you configure SPI without CONFIG_STM32H7_SPI_DMA. Is there anyone that knows more about this? I implemented a fix and it seems to be working - but since I am not familiar with the code I am not sure if this is the right thing to do. I attached a patch. Thanks, Edi
Re: Cannot build H7 configuration using Ubuntu for Windows 10
I debugged and found what my problem was. I am using a custom configuration
for a custom board and did not notice that the Make.defs and flash.ld
changed for H7 between Nuttx 9.0 and Nuttx 9.1. My configuration was still
using the old ones from 9.0.
This is why even though the build worked it was crashing when running the
bin on the board. Using the new files from 9.1 fixed the problem.
Thank you all for your help,
Edi
On Wed, Oct 14, 2020 at 6:06 PM Gregory Nutt wrote:
>
> > You are right. I used your changes and build using -u and I still have
> the
> > same issue (the board does not bootup).
> > This is what I can see on the serial port:
> >
> > ADE
>
> This is debug output from arch/arm/src/stm32h7/start.c. The hang occurs
> somewhere after line 412 ( showprogress('E');), and probably after
> nx_start() is called. I would put breakpoints on nx_start(),
> up_initialize(), and nsh_main().
>
>
>
>
>
Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
Hi Eduard, Unfortunately the mailing list is refusing patches with extension .patch, we need to rename it to .txt to get it here. BTW, you can submit a Pull Request directly to https://github.com/apache/incubator-nuttx and we could review it. BR, Alan On 10/16/20, Eduard Niesner wrote: > Hi all, > > I am not familiar with the code from nuttx\arch\arm\src\stm32h7\stm32_spi.c > but I believe that there is an issue. > "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is > defined. > But in the spi_interrupt function, the txresult is used regardless of > whether CONFIG_STM32H7_SPI_DMA is defined or not. > > This generates a build issue when you configure SPI without > CONFIG_STM32H7_SPI_DMA. > > Is there anyone that knows more about this? > I implemented a fix and it seems to be working - but since I am not > familiar with the code I am not sure if this is the right thing to do. > I attached a patch. > > Thanks, > Edi >
Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
I attached it as .txt. *Please note:* I made the code build and the SPI seems to work as expected - I am communicating with an at45db flash over SPI and mounted smartFS on it and the communication seems to work. I am not sure if the changes that I did are enough or if the entire spi_interrupt function should be surrounded by the #ifdef CONFIG_STM32H7_SPI_DMA condition as well (and also where it is called from). It would be good if someone with more experience that understands the impact of the change could look into it. PS: If the changes look ok, I will register on github and create the pull request. Do I need to get any approvals to create branches or pull requests on nuttx incubator? Edi On Fri, Oct 16, 2020 at 7:28 PM Alan Carvalho de Assis wrote: > Hi Eduard, > > Unfortunately the mailing list is refusing patches with extension > .patch, we need to rename it to .txt to get it here. > > BTW, you can submit a Pull Request directly to > https://github.com/apache/incubator-nuttx and we could review it. > > BR, > > Alan > > On 10/16/20, Eduard Niesner wrote: > > Hi all, > > > > I am not familiar with the code from > nuttx\arch\arm\src\stm32h7\stm32_spi.c > > but I believe that there is an issue. > > "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is > > defined. > > But in the spi_interrupt function, the txresult is used regardless of > > whether CONFIG_STM32H7_SPI_DMA is defined or not. > > > > This generates a build issue when you configure SPI without > > CONFIG_STM32H7_SPI_DMA. > > > > Is there anyone that knows more about this? > > I implemented a fix and it seems to be working - but since I am not > > familiar with the code I am not sure if this is the right thing to do. > > I attached a patch. > > > > Thanks, > > Edi > > > diff --git a/arch/arm/src/stm32h7/stm32_spi.c b/arch/arm/src/stm32h7/stm32_spi.c index 268f22093b..78d96864cf 100644 --- a/arch/arm/src/stm32h7/stm32_spi.c +++ b/arch/arm/src/stm32h7/stm32_spi.c @@ -1022,9 +1022,10 @@ static int spi_interrupt(int irq, void *context, void *arg) spi_modifyreg(priv, STM32_SPI_IER_OFFSET, SPI_IER_EOTIE, 0); /* Set result and release wait semaphore */ - +#ifdef CONFIG_STM32H7_SPI_DMA priv->txresult = 0x80; nxsem_post(&priv->txsem); +#endif } return 0;
Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
On Fri, Oct 16, 2020 at 1:28 PM Eduard Niesner wrote: > I attached it as .txt. > > *Please note:* I made the code build and the SPI seems to work > as expected - I am communicating with an at45db flash over SPI and mounted > smartFS on it and the communication seems to work. I am not sure if > the changes that I did are enough or if the entire spi_interrupt function > should be surrounded by the #ifdef CONFIG_STM32H7_SPI_DMA condition as > well (and also where it is called from). > It would be good if someone with more experience that understands the > impact of the change could look into it. > > PS: If the changes look ok, I will register on github and create the pull > request. Do I need to get any approvals to create branches or pull > requests on nuttx incubator? > I haven't studied your patch but, regarding pull requests, it's better to fork the incubator-nuttx repo, create a branch in your own fork, and when ready, open a pull request from that. (Creating a branch allows you to continue making additional PRs from your fork, as GitHub doesn't allow to create multiple forks of the same thing.) You may find this helpful: https://nuttx.apache.org/docs/latest/contributing/index.html Cheers Nathan
RE: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
Hi Edi, You would fork https://github.com/apache/incubator-nuttx and submit a PR from your fork. I will have a look at the PR on Monday when I back at the computer. David *From:* Eduard Niesner [mailto:niesneredu...@gmail.com] *Sent:* Friday, October 16, 2020 10:28 AM *To:* dev@nuttx.apache.org *Subject:* Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined I attached it as .txt. *Please note:* I made the code build and the SPI seems to work as expected - I am communicating with an at45db flash over SPI and mounted smartFS on it and the communication seems to work. I am not sure if the changes that I did are enough or if the entire spi_interrupt function should be surrounded by the #ifdef CONFIG_STM32H7_SPI_DMA condition as well (and also where it is called from). It would be good if someone with more experience that understands the impact of the change could look into it. PS: If the changes look ok, I will register on github and create the pull request. Do I need to get any approvals to create branches or pull requests on nuttx incubator? Edi On Fri, Oct 16, 2020 at 7:28 PM Alan Carvalho de Assis wrote: Hi Eduard, Unfortunately the mailing list is refusing patches with extension .patch, we need to rename it to .txt to get it here. BTW, you can submit a Pull Request directly to https://github.com/apache/incubator-nuttx and we could review it. BR, Alan On 10/16/20, Eduard Niesner wrote: > Hi all, > > I am not familiar with the code from nuttx\arch\arm\src\stm32h7\stm32_spi.c > but I believe that there is an issue. > "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is > defined. > But in the spi_interrupt function, the txresult is used regardless of > whether CONFIG_STM32H7_SPI_DMA is defined or not. > > This generates a build issue when you configure SPI without > CONFIG_STM32H7_SPI_DMA. > > Is there anyone that knows more about this? > I implemented a fix and it seems to be working - but since I am not > familiar with the code I am not sure if this is the right thing to do. > I attached a patch. > > Thanks, > Edi >
Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
Hi Eduard, Yes, the modification appears correct. Also I noticed that the "nxsem_wait_uninterruptible(&priv->txsem)" is already inside a "#ifdef CONFIG_STM32H7_SPI_DMA", so your modification will make the logic symmetric. You just need to create a fork, do your modification and submit a PR. BR, Alan On 10/16/20, Eduard Niesner wrote: > I attached it as .txt. > > *Please note:* I made the code build and the SPI seems to work as expected > - I am communicating with an at45db flash over SPI and mounted smartFS on > it and the communication seems to work. I am not sure if the changes that I > did are enough or if the entire spi_interrupt function should be surrounded > by the #ifdef CONFIG_STM32H7_SPI_DMA condition as well (and also where it > is called from). > It would be good if someone with more experience that understands the > impact of the change could look into it. > > PS: If the changes look ok, I will register on github and create the pull > request. Do I need to get any approvals to create branches or pull > requests on nuttx incubator? > > Edi > > > > On Fri, Oct 16, 2020 at 7:28 PM Alan Carvalho de Assis > wrote: > >> Hi Eduard, >> >> Unfortunately the mailing list is refusing patches with extension >> .patch, we need to rename it to .txt to get it here. >> >> BTW, you can submit a Pull Request directly to >> https://github.com/apache/incubator-nuttx and we could review it. >> >> BR, >> >> Alan >> >> On 10/16/20, Eduard Niesner wrote: >> > Hi all, >> > >> > I am not familiar with the code from >> nuttx\arch\arm\src\stm32h7\stm32_spi.c >> > but I believe that there is an issue. >> > "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is >> > defined. >> > But in the spi_interrupt function, the txresult is used regardless of >> > whether CONFIG_STM32H7_SPI_DMA is defined or not. >> > >> > This generates a build issue when you configure SPI without >> > CONFIG_STM32H7_SPI_DMA. >> > >> > Is there anyone that knows more about this? >> > I implemented a fix and it seems to be working - but since I am not >> > familiar with the code I am not sure if this is the right thing to do. >> > I attached a patch. >> > >> > Thanks, >> > Edi >> > >> >
Re: nuttx\arch\arm\src\stm32h7\stm32_spi.c uses txresult even when SPI_DMA is not defined
Thank you for your feedback, Alan! Alan, David, I created the pull request: https://github.com/apache/incubator-nuttx/pull/2010/ Edi On Fri, Oct 16, 2020 at 9:17 PM Alan Carvalho de Assis wrote: > Hi Eduard, > > Yes, the modification appears correct. > > Also I noticed that the "nxsem_wait_uninterruptible(&priv->txsem)" is > already inside a "#ifdef CONFIG_STM32H7_SPI_DMA", so your modification > will make the logic symmetric. > > You just need to create a fork, do your modification and submit a PR. > > BR, > > Alan > > On 10/16/20, Eduard Niesner wrote: > > I attached it as .txt. > > > > *Please note:* I made the code build and the SPI seems to work as > expected > > - I am communicating with an at45db flash over SPI and mounted smartFS on > > it and the communication seems to work. I am not sure if the changes > that I > > did are enough or if the entire spi_interrupt function should be > surrounded > > by the #ifdef CONFIG_STM32H7_SPI_DMA condition as well (and also where it > > is called from). > > It would be good if someone with more experience that understands the > > impact of the change could look into it. > > > > PS: If the changes look ok, I will register on github and create the pull > > request. Do I need to get any approvals to create branches or pull > > requests on nuttx incubator? > > > > Edi > > > > > > > > On Fri, Oct 16, 2020 at 7:28 PM Alan Carvalho de Assis < > acas...@gmail.com> > > wrote: > > > >> Hi Eduard, > >> > >> Unfortunately the mailing list is refusing patches with extension > >> .patch, we need to rename it to .txt to get it here. > >> > >> BTW, you can submit a Pull Request directly to > >> https://github.com/apache/incubator-nuttx and we could review it. > >> > >> BR, > >> > >> Alan > >> > >> On 10/16/20, Eduard Niesner wrote: > >> > Hi all, > >> > > >> > I am not familiar with the code from > >> nuttx\arch\arm\src\stm32h7\stm32_spi.c > >> > but I believe that there is an issue. > >> > "txresult" is defined and used only if the CONFIG_STM32H7_SPI_DMA is > >> > defined. > >> > But in the spi_interrupt function, the txresult is used regardless of > >> > whether CONFIG_STM32H7_SPI_DMA is defined or not. > >> > > >> > This generates a build issue when you configure SPI without > >> > CONFIG_STM32H7_SPI_DMA. > >> > > >> > Is there anyone that knows more about this? > >> > I implemented a fix and it seems to be working - but since I am not > >> > familiar with the code I am not sure if this is the right thing to do. > >> > I attached a patch. > >> > > >> > Thanks, > >> > Edi > >> > > >> > > >
