Fishwaldo commented on PR #19883:
URL: https://github.com/apache/nuttx/pull/19883#issuecomment-5317106392
Hi @jlaitine - Thanks for the feedback.
I've kept this change rather simplistic for a few reasons (including keeping
my sanity when debugging UHS modes!)..
I agree, that querying, then setting the "highest supported mode" is
something we need to do especially if we want to reach into UHS modes (SDR50,
104 or DDR50 ranges). But supporting those ranges are going to require a fair
bit of work to integrate - Driver Strength, power limits etc etc that the lower
half drivers would have to adapt to. (Id say a fair bit of churn on the driver
to get there). and to be honest, I'm not upto that challenge across all the
various boards NuttX supports. :)
With the current approach, we send CMD6 with Access Mode set to HS only, and
then read back the Status Mode to see if the card accepted the HS mode (if
there are cards out there that don't do it, they reject the setting, not fall
into some black hole on the bus):
` if ((status[MMCSD_SWITCH_STATUS_MODE] & 15) !=
MMCSD_SWITCH_MODE_HIGHSPEED)
{
finfo("Card declined high speed, staying at default speed\n");
return -EIO;
}
`
(if the card can't go into HS mode, it returns 0xF for Group 1, and we leave
it there, staying on Default Mode).
It just saves us 1 command instead of 2... A bit of a hammer, but as this
patch is only aiming for HS, it *works*.
For the bus speed - Not sure how many boards would come up > 25Mhz to start
with... and a non-HS capable card would probably never get to CMD6 anyway when
bus is above 25Mhz (it could... I just don't know???) My understanding of the
spec is everything comes out of reset at Default and 25Mhz.
How the driver should be architectured? - Yeah, its probably a good
discussion to take place, especially if you want the NuttX driver to support
UHS speeds as well.
@jerpelea Just for your info - All the code this patches is essentially
gated behind SDIO_CAPS_SD_HS_MODE, set by the lower half drivers... of which, 0
in tree drivers set. If you want to test this on other hardware (and not take a
"Trust me Bro - it works on my invisible board" promise) then you would need to
find another board with HS support and update the lower half driver. Also as
safety:
1. The CLOCK_SD_TRANSFER_4BIT_HS is put at the end of the enum sdio_clock_e
- Therefor it wont change any hardcoded constant orders (and lets hope lower
half drivers have a default: fallback block in case they encounter it) - I
didn't see any unhanded switch statements in my quick build tests here.
2. Although we try to set HS mode via CMD6 (instead of query support then
set) - R1 Errors, timeouts or declined CMD6 mode switches fall back to
CLOCK_SD_TRANSFER_4BIT
3. If someone does set MMCSD_SWITCH_MODE_HIGHSPEED on their lower half
driver, but doesn't update SDIO_CLOCK(), the card will run in HS mode at
25Mhz... thats still a legal configuration, but of course, no performance
improvements.
--
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]
