On 21-11-29 11:18:36, Dan Williams wrote:
[snip]
> > > > > > - rc = cxl_pci_mbox_wait_for_doorbell(cxlds);
> > > > > > - if (rc) {
> > > > > > - dev_warn(dev, "Mailbox interface not ready\n");
> > > > > > - goto out;
> > > > > > - }
> > > > > > -
> > > > > > md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
> > > > > > if (!(md_status & CXLMDEV_MBOX_IF_READY && CXLMDEV_READY(md_status))) {
> > > > > > dev_err(dev, "mbox: reported doorbell ready, but not mbox ready\n");
> > > > >
> > > > > This error message is obsolete since nothing is pre-checking the
> > > > > mailbox anymore, and per above I see no problem waiting to check the
> > > > > status until after the mailbox has failed to respond after a timeout.
> > > >
> > > > The message is wrong, but I think the logic is still valuable. How about:
> > > > "mbox: reported interface ready, but mbox not ready"
> > >
> > > You mean check this every time even though the spec says the driver
> > > only needs to check it once per-reset?
> >
> > Unfortunately it does not say that. "... it shall remain set until the next
> > reset or the device encounters an error that prevents any mailbox
> > communication."
> >
> > Once we have real error checking in place, this could go away, though I see no
> > harm in leaving it.
>
> Right, there's no harm in the check, it just seems overly paranoid to
> me if it was already checked once. Until a doorbell timeout happens
> it's an extra MMIO cycle that can saved for a "what happened?" check
> after a timeout.
Well I suspect we're just rearranging the deck chairs on the Titanic now, but...
I see doorbell timeouts as disconnected from whether or not the mailbox
interface is ready. If they were the same, we wouldn't need both bits and we
could just wait extra long for the doorbell when probing.
In other words, I expect if the interface goes unready, doorbell timeout will
occur, but I don't think we should assume if doorbell timeout occurs, the
interface is no longer ready. I don't purport to know why a doorbell timeout
might occur while the interface remains available (likely a firmware bug, I
presume).
It does seem interesting to check if the interface is no longer ready on timeout
though.
