Re: [PATCH] mmc: core: Initial support for SD express card/host

From: Arnd Bergmann <arnd@arndb.de>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc <linux-mmc@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Rui Feng <rui_feng@realsil.com.cn>,
	linux-nvme@lists.infradead.org,
	linux-pci <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] mmc: core: Initial support for SD express card/host
Date: Fri, 24 Jul 2020 15:35:47 +0200	[thread overview]
Message-ID: <CAK8P3a3wLPv58uqTqyXk7+0Cxoe4vdfahzCxXOp2pdGZDkeFsw@mail.gmail.com> (raw)
In-Reply-To: <CAPDyKFp3D8xZCSGNMm2ZOpa5f5wMwderCuAA5yLMXdjoKFoxQw@mail.gmail.com>

On Fri, Jul 24, 2020 at 12:06 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Thu, 16 Jul 2020 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Jul 16, 2020 at 4:16 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > > +       /* Continue with the SD express init, if the card supports it. */
> > > +       resp &= 0x3000;
> > > +       if (pcie_bits && resp) {
> > > +               if (resp == 0x3000)
> > > +                       host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
> > > +               else
> > > +                       host->ios.timing = MMC_TIMING_SD_EXP;
> > > +
> > > +               /*
> > > +                * According to the spec the clock shall also be gated, but
> > > +                * let's leave this to the host driver for more flexibility.
> > > +                */
> > > +               return host->ops->init_sd_express(host, &host->ios);
> > > +       }
> > > +
> > >         return 0;
> > >  }
> >
> > Does this need an additional handshake or timeout to see if the
> > device was successfully probed by the nvme driver?
> >
> > It looks like the card would just disappear here if e.g. the nvme driver
> > is not loaded or runs into a runtime error.
>
> You are correct! In principle, the card would not be detected (it
> doesn't disappear as it never gets registered). Instead, it's left in
> "half-initialized" state, waiting for the nvme driver to take over.
>
> I simply didn't want to go that far, to introduce synchronizations
> steps between the nvme driver and mmc driver, but rather started
> simple. Perhaps we can discuss these things as improvements on top
> instead?
>
> What do you think?

Starting simple is generally a good idea, yes.

It would be good to have feedback from the nvme driver maintainers.

One way I can see the handshake working would be to have
an sdexpress class_driver that provides interfaces for both mmc
and nvme to link against. The mmc core can then create a
class device when it finds an sd-express device and that
class device contains a simple state machine that keeps track of
what either side think is going on, possibly also providing
a way to perform callbacks between the two sides.

      Arnd