Re: mmccam -> no more cards/sdio but "mmcprobe"

On Mon, 21 Jul 2025, Warner Losh wrote:

Hi Warner,

> On Mon, Jul 21, 2025 at 11:38 AM Warner Losh <imp@bsdimp.com> wrote:
>>
>> So, We're getting through the 'reset' and 'identify' states in the
>> state machine and entering the 'power off' state. And then progressing
>> no further. So we get into the 'done' routine, but don't progress to
>> sending the get ocr command. That's why we still have the probe
>> routine. We're not setting the probe into INVALID state, so we should
>> be calling xpt_schedule() to do the next single step of this process
>> at the end of mmc_done(), but that doesn't trigger a new call to
>> mmc_start().
>>
>> I did some CAM cleanups that shouldn't have broken this, but might
>> have (low probability, but with cam you never know, especially in the
>> single step phase we do to do the probing). I'll check those out.
>> Maybe it's easy.
>
> OK. It turned out that I had messed things up, unbeknownst to me. I've
> added a KASSERT that should catch problems like this in the future
> (and caught a second one that I also fixed). It should be safe to go
> back into the MMCCAM water after b4b166b8c46b8. It broke a few days
> ago in c6dc5d367681 (I just realised I forgot to add the fixes: tag to
> my commit in my rush to get a fix in).
>
> When we're running the single stepping engine to probe the device, all
> those commands are queued. And we'll not run through them if we queue
> something at CAM_PRIORITY_NONE. c6dc5d367681 made the ccb that ran for
> the xpt_path_inq() at the NONE priority. Normally, this is fine. All
> the instances in the tree but two use that the stack to pass in a CCB
> for this use. It doesn't matter the priority. But, xpt_start() did
> xpt_path_inq on the start_ccb passed in that was allocated in
> xpt_schedule which fills in the proper priority and other fields. It
> didn't matter for the xpt_path_inq, it completed just fine. However,
> for the XPT_MMC_IO commands that followed, the priority was wrong, so
> they'd never run. It turns out that we don't need or use the
> xpt_path_inq() results at all, so it was just having the side effect
> of initializing the CCB (bogusly and redundantly, it turns out. The
> other location that doesn't use the stack for xpt_path_inq() doesn't
> really reuse the CCB, but saves the results of the path_inq away (it
> could allocate less memory if it did a similar thing to the stack
> trick in its soft state structure, but doesn't).
>
> So tl;dr: Thanks for the heads up, this broke only a few days ago, and
> should be fixed now. My sdhci system with eMMC in it works with these
> changes. And I added a KASSERT to catch this problem in the future.

I can confirm that your fixes are working.

I had a look through the cam diff but wouldn't have spotted this.

I can also confirm that the KASSERT works:

panic: xpt_action: queued ccb and CAM_PRIORITY_NONE illegal.

Now to fix my own code ;-)


Thanks a lot for the investigation and quick fix!  Very much appreciated!
/bz

-- 
Bjoern A. Zeeb                                                     r15:7