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

On Tue, Jul 22, 2025 at 11:01 AM Bjoern A. Zeeb
<bzeeb-lists@lists.zabbadoz.net> wrote:
>
> 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.

I need to add which CCB function was queued to this, I think...

> Now to fix my own code ;-)

It should be easy...

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

You bet. I was worried that I'd broken something with my CAM fixes,
despite testing it a lot locally. Then nobody said anything for a
couple of weeks and I thought I was in the clear... Then you popped up
just at the same time I had installed FreeBSD on an eMMC system I got
and noticed it wasn't using MMCCAM as I was about to update it, so the
timing was good...

Warner