You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "arjav1528 (via GitHub)" <gi...@apache.org> on 2026/03/16 06:52:17 UTC
[PR] Document CSD parsing expectations for SDIO lower-half [nuttx]
arjav1528 opened a new pull request, #18543:
URL: https://github.com/apache/nuttx/pull/18543
## Summary
Addressed the issue by documenting how SDIO lower-half drivers must provide R2 (CID/CSD) responses and by adding a small sanity check in the MMCSD upper-half. `include/nuttx/sdio.h` now explains the R2 format, states that the upper-half expects a 128‑bit CID/CSD payload in the standard layout, and notes that controllers which strip the CRC must shift the four 32‑bit words left by one byte (with `bcm2711_sdio.c` and `imx9_usdhc.c` as examples). `mmcsd_decode_csd` in `drivers/mmcsd/mmcsd_sdio.c` gains a `DEBUGASSERT(csd[0] != 0);` under `CONFIG_DEBUG_FEATURES` to catch obviously invalid or misaligned CSDs, and the SDIO/MMCSD docs now describe the SDIO lower-half call-flow and R2/CSD expectations and point implementers to this guidance.
## Impact
- **Runtime behavior**: Only a debug‑only `DEBUGASSERT` in `mmcsd_decode_csd` is added; there is **no behavior or performance change** in non‑debug builds.
- **API/ABI**: No changes to SDIO or MMCSD public interfaces; existing correct SDIO lower‑halves continue to work unchanged.
- **Documentation**: SDIO docs now explicitly describe `recv_r2`/R2 handling (including CRC‑stripping controllers), and MMCSD docs direct SDIO lower‑half implementers there.
- **Build / compatibility / security**: No new Kconfig options or dependencies beyond existing Sphinx for docs; no security impact; compatibility for `/dev/mmcsd*` users is unchanged.
## Testing
- `make html` in `Documentation/` to verify the updated RST documentation builds successfully.
- No runtime or hardware tests were performed because no SDIO/MMC-capable target hardware is available in this environment; the new `DEBUGASSERT` behavior is validated by static inspection only.
--
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: [PR] Documented CSD parsing expectations for SDIO lower-half [nuttx]
Posted by "arjav1528 (via GitHub)" <gi...@apache.org>.
arjav1528 commented on code in PR #18543:
URL: https://github.com/apache/nuttx/pull/18543#discussion_r2945523428
##########
Documentation/components/drivers/special/sdio.rst:
##########
@@ -31,3 +31,36 @@ SDIO Device Drivers
- **Examples**: ``arch/arm/src/stm32/stm32_sdio.c`` and
``drivers/mmcsd/mmcsd_sdio.c``
+
+Implementing an SDIO lower-half
+===============================
+
+When implementing a new SDMMC controller driver (SDIO lower-half), it must
+provide the interface defined in ``struct sdio_dev_s``.
+
+Call-flow
+---------
+
+The MMCSD upper-half (``drivers/mmcsd/mmcsd_sdio.c``) interacts with the
+lower-half by sending commands and receiving responses:
+
+1. ``SDIO_SENDCMD``: Send the command (e.g., CMD2, CMD9).
Review Comment:
correct, the original list read like a full call flow, but my intent was to only sketch the minimal interaction around CMD9/R2 and the CSD that the SDIO lower-half must return, however I'll update push them
--
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: [PR] Documented CSD parsing expectations for SDIO lower-half [nuttx]
Posted by "arjav1528 (via GitHub)" <gi...@apache.org>.
arjav1528 commented on code in PR #18543:
URL: https://github.com/apache/nuttx/pull/18543#discussion_r2945551975
##########
drivers/mmcsd/mmcsd_sdio.c:
##########
@@ -606,6 +606,12 @@ static void mmcsd_decode_csd(FAR struct mmcsd_state_s *priv, uint32_t csd[4])
bool permwriteprotect;
bool tmpwriteprotect;
+ /* Best-effort sanity check for misaligned or missing-CRC CSD; the
+ * lower-half is responsible for returning the correct 128-bit layout.
+ */
+
+ DEBUGASSERT(csd[0] != 0);
Review Comment:
my bad, I have removed the DEBUGASSERT and kept the PR on documenting the R@/CID/CSD expectations for SDIO lower halves
--
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: [PR] Documented CSD parsing expectations for SDIO lower-half [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18543:
URL: https://github.com/apache/nuttx/pull/18543#discussion_r2943060168
##########
Documentation/components/drivers/special/sdio.rst:
##########
@@ -31,3 +31,36 @@ SDIO Device Drivers
- **Examples**: ``arch/arm/src/stm32/stm32_sdio.c`` and
``drivers/mmcsd/mmcsd_sdio.c``
+
+Implementing an SDIO lower-half
+===============================
+
+When implementing a new SDMMC controller driver (SDIO lower-half), it must
+provide the interface defined in ``struct sdio_dev_s``.
+
+Call-flow
+---------
+
+The MMCSD upper-half (``drivers/mmcsd/mmcsd_sdio.c``) interacts with the
+lower-half by sending commands and receiving responses:
+
+1. ``SDIO_SENDCMD``: Send the command (e.g., CMD2, CMD9).
Review Comment:
I believe the call flow is more complex than this.
##########
drivers/mmcsd/mmcsd_sdio.c:
##########
@@ -606,6 +606,12 @@ static void mmcsd_decode_csd(FAR struct mmcsd_state_s *priv, uint32_t csd[4])
bool permwriteprotect;
bool tmpwriteprotect;
+ /* Best-effort sanity check for misaligned or missing-CRC CSD; the
+ * lower-half is responsible for returning the correct 128-bit layout.
+ */
+
+ DEBUGASSERT(csd[0] != 0);
Review Comment:
This is not correct. First , `csd[0]` is 32 bits, so this doesn't just check that the first byte is 0.
Second, is it ever possible that the first byte (the CRC) is 0 naturally?
Are you able to test this on any targets?
--
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