You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/18 19:52:23 UTC
[GitHub] [incubator-nuttx] davids5 opened a new pull request #1604: mmcsd_sdio:Fix breakage from 997d4 SD not functional
davids5 opened a new pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604
## Summary
When CONFIG_MMCSD_MULTIBLOCK_DISABLE is lit, all SD
read/write failed. The function return the number of
blocks in `ret`, set on entry to nsectors. That was
then wiped out by using the ret for the mmcsd_takesem.
Since the code had many path setting return, the choices for
the fix could have been add a new varaible or simple init it
were used. I choose the latter.
## Impact
Since 2020-03-31 SD cards not using multi block (i.e with CONFIG_MMCSD_MULTIBLOCK_DISABLE) are broken.
## Testing
was done on nxp_fmuk66-v3
Before PR
After PR:
mount is fine
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4 SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-675681137
@Ouss4 - I had a look a the commit that introduced this bug this. I checked both the mmcsd_{spi|}sdio} drivers, but there may be more of this class of error.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-677509553
@Ouss4 No worries and no apology needed. Do you feel we should audit that set of PR's or do you feel this was just an outlier?
BTW Thank you for the hosting on the conference. It was great to attach a voice to the name and the git user name!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 edited a comment on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676088243
@xiaoxiang781216 These sweeping changes, without any functional testing are effecting the quality of NuttX. Currently our release process will not catch this. This bug has been in 2 releases.
Do you, or your team, have time to [audit these](https://github.com/apache/incubator-nuttx/search?q=Check+return+from+nxsem_wait_uninterruptible&type=Commits), if not we should divide them up and look for the same type of mistake.
@btashton Should this be back ported?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676114253
Our test team is porting LTP project:
https://github.com/linux-test-project/ltp
and enhance the workflow to enable the automation test. Hopefully, this type of regression can be catched by the daily automation test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676806950
Thanks for catching that! Particular care had been taken to ensure that those changes didn't break anything, corner cases with the return of read() and write() had been double checked, it wasn't just a dummy search and replace, but it looks like one of them slipped. Apologies for that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-677634412
> > Yes, we will make a golden config for test.
>
> Given the number of config variables, and states for those variables and dependency, I think we need a **large set of** golden configs (not just one ) and tooling to build test a new/deleted config var.
>
The golden config is justed for automation test, each arch should have at least one config for virtual platform(qemu or sim) and run LTP test suite to ensure the basic quality.
> When I am messing with Kconfig I build test all the permutation of the settings change. This finds all sorts of subtle problems: like unused vars that the codding style tends to cause because we can not define were used.
> It also finds the C&P errors that are common with N = {1..9} of something.
>
> I want be clear, the build testing has come a long way thanks yourself to @btashton @liuguo09 @yamt @lulingar and @patacongo - Thank you all. We just need to get better coverage. It would be nice to have this on the road map and define some tasks that we can all help with.
Yes, we need extend the build matrix to coverage more combination for build, but ranconfig like linux can help a lot:
https://elinux.org/Ktest
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-677641203
> @Ouss4 No worries and no apology needed. Do you feel we should audit that set of PR's or do you feel this was just an outlier?
I'd like to believe so. As I said we were careful the first time when those changes were introduced but as you can see one of them passed our checks. I did a very quick review last night and this morning, I couldn't find any issue but that wasn't a real audit.
> BTW Thank you for the hosting on the conference. It was great to attach a voice to the name and the git user name!
My pleasure! It was great having you.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-677520669
> Yes, we will make a golden config for test.
Given the number of config variables, and states for those variables and dependency, I think we need a **large set of** golden configs (not just one ) and tooling to build test a new/deleted config var.
When I am messing with Kconfig I build test all the permutation of the settings change. This finds all sorts of subtle problems: like unused vars that the codding style tends to cause because we can not define were used.
It also finds the C&P errors that are common with N = {1..9} of something.
I want be clear, the build testing has come a long way thanks yourself to @btashton @liuguo09 @yamt @lulingar and @patacongo - Thank you all. We just need to get better coverage. It would be nice to have this on the road map and define some tasks that we can all help with.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #1604: mmcsd_sdio:Fix breakage from 997d4 SD not functional
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676088243
@xiaoxiang781216 These sweeping changes, without any functional testing are effecting the quality of NuttX. Currently our release process will not catch this. This bug has been in 2 releases. This bug has been in 2 releases.
Do you, or your team, have time to [audit these](https://github.com/apache/incubator-nuttx/search?q=Check+return+from+nxsem_wait_uninterruptible&type=Commits), if not we should divide them up and look for the same type of mistake.
@btashton Should this be back ported?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676326031
Yes, we will make a golden config for test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1604: mmcsd_sdio:Fix breakage from 997d4fabb062276c9cf10cd43b0c950a54e0ca7d SD not functional
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1604:
URL: https://github.com/apache/incubator-nuttx/pull/1604#issuecomment-676117718
@xiaoxiang781216 - that is great! We also need the board configs to be updated to get complete test vectors.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org