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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to