quoted

+     for (i = 0; i < ARRAY_SIZE(mbox->vchan_allocated); i++) {
+             if (mbox->vchan_allocated[i])
+                     mbox_chan_received_data(&mbox->mbox.chans[i], &msg);
+     }

It seems like the only reason why you need to explicitly check for an
allocated channel is that mbox_chan_received_data() would otherwise
crash. Are mailbox drivers really supposed to keep track of whether a
channel has been requested by a client? Isn't that something that should
be done in the core?

Yeah, I'd agree that this is something that should be handled by the core.

quoted

+static int tegra_xusb_mbox_probe(struct platform_device *pdev)
+{
+     struct tegra_xusb_mbox *mbox;
+     struct resource *res;
+     int ret;
+
+     mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+     if (!mbox)
+             return -ENOMEM;
+     platform_set_drvdata(pdev, mbox);
+     spin_lock_init(&mbox->lock);
+
+     mbox->mbox.dev = &pdev->dev;
+     mbox->mbox.chans = devm_kcalloc(&pdev->dev, TEGRA_XUSB_MBOX_NUM_CHANS,
+                                     sizeof(*mbox->mbox.chans), GFP_KERNEL);
+     if (!mbox->mbox.chans)
+             return -ENOMEM;
+     mbox->mbox.num_chans = TEGRA_XUSB_MBOX_NUM_CHANS;
+     mbox->mbox.ops = &tegra_xusb_mbox_chan_ops;
+     mbox->mbox.txdone_poll = true;
+     mbox->mbox.txpoll_period = 0; /* no need to actually poll */

Does the core perhaps need special handling for this? It seems like
poll_txdone() will always rearm the timer used to do the polling,
irrespective of whether the transfer is actually done or not.

Yeah, that doesn't seem quite right...

... but we still need to re-arm the timer if tx_tick() submits another
message.  Perhaps the better thing to do is to have msg_submit() arm
the timer.

quoted

+     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+     if (!res)
+             return -ENODEV;
+     mbox->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+     if (!mbox->regs)
+             return -ENOMEM;

This doesn't look right. Upon closer inspection, the reason why you
don't use devm_request_resource() is because these registers are shared
with the XHCI controller.

Perhaps a better design would be for the XHCI driver to expose the
mailbox rather than split it off into a separate driver.

Well that's what I had originally, but then it was suggested I make it
a separate driver.

Stephen also brought this up during review and suggested that some
sort of MFD would be the best way to structure this, but was fine with
the way I have it now.  I can move this driver around (again) if you
feel that strongly about it...

quoted

diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
new file mode 100644
index 0000000..cfe211d
--- /dev/null
+++ b/include/soc/tegra/xusb.h

Perhaps this should really be named xusb-mbox.h?

I'd prefer to leave it as xusb.h so that any other XUSB-related
definitions can be left here.