Re: [PATCH v2 01/10] mailbox: Support blocking transfers in atomic context

Thierry Reding
2018-11-23T11:17:00Z

[-- Attachment #1.1: Type: text/plain, Size: 4941 bytes --]

On Thu, Nov 22, 2018 at 09:47:12AM +0100, Thierry Reding wrote:
[...]
> Perhaps you'd be less concerned about such a change if it was perhaps
> more explicit? Just throwing ideas around, I think something that could
> also work is if we explicitly add a mbox_flush() function that would
> basically be calling ->flush(). That way users of the mailbox can make
> their requirement very explicit. I haven't actually tested that, but I
> think it would work. Does that sound more acceptable to you?

I tried implementing the explicit flushing on top of this series and it
would look roughly like the below. What do you think?

Thierry

--->8---
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 3e7e2c4358aa..fbdcc82a61ae 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -267,14 +267,6 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
unsigned long wait;
int ret;

- if (irqs_disabled() && chan->mbox->ops->flush) {
- ret = chan->mbox->ops->flush(chan, chan->cl->tx_tout);
- if (ret < 0)
- tx_tick(chan, ret);
-
- return ret;
- }
-
if (!chan->cl->tx_tout) /* wait forever */
wait = msecs_to_jiffies(3600000);
else
@@ -291,6 +283,34 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
}
EXPORT_SYMBOL_GPL(mbox_send_message);

+/**
+ * mbox_flush - flush a mailbox channel
+ * @chan: mailbox channel to flush
+ * @timeout: time, in milliseconds, to allow the flush operation to succeed
+ *
+ * Mailbox controllers that need to work in atomic context can implement the
+ * ->flush() callback to busy loop until a transmission has been completed.
+ * The implementation must call mbox_chan_txdone() upon success. Clients can
+ * call the mbox_flush() function at any time after mbox_send_message() to
+ * flush the transmission. After the function returns success, the mailbox
+ * transmission is guaranteed to have completed.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int mbox_flush(struct mbox_chan *chan, unsigned long timeout)
+{
+ int ret;
+
+ if (!chan->mbox->ops->flush)
+ return -ENOTSUPP;
+
+ ret = chan->mbox->ops->flush(chan, timeout);
+ if (ret < 0)
+ tx_tick(chan, ret);
+
+ return ret;
+}
+
/**
* mbox_request_channel - Request a mailbox channel.
* @cl: Identity of the client requesting the channel.
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index f99c406cb3cc..e443f6a2ec4b 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -387,15 +387,13 @@ static int tegra_hsp_mailbox_send_data(struct mbox_chan *chan, void *data)

tegra_hsp_channel_writel(&mb->channel, value, HSP_SM_SHRD_MBOX);

- if (!irqs_disabled()) {
- /* enable EMPTY interrupt for the shared mailbox */
- spin_lock_irqsave(&hsp->lock, flags);
+ /* enable EMPTY interrupt for the shared mailbox */
+ spin_lock_irqsave(&hsp->lock, flags);

- hsp->mask |= BIT(HSP_INT_EMPTY_SHIFT + mb->index);
- tegra_hsp_writel(hsp, hsp->mask, HSP_INT_IE(hsp->shared_irq));
+ hsp->mask |= BIT(HSP_INT_EMPTY_SHIFT + mb->index);
+ tegra_hsp_writel(hsp, hsp->mask, HSP_INT_IE(hsp->shared_irq));

- spin_unlock_irqrestore(&hsp->lock, flags);
- }
+ spin_unlock_irqrestore(&hsp->lock, flags);

return 0;
}
diff --git a/drivers/tty/serial/tegra-tcu.c b/drivers/tty/serial/tegra-tcu.c
index 1d360cd03b18..59eaa13e169e 100644
--- a/drivers/tty/serial/tegra-tcu.c
+++ b/drivers/tty/serial/tegra-tcu.c
@@ -57,6 +57,7 @@ static void tegra_tcu_write_one(struct tegra_tcu *tcu, u32 value,
value |= TCU_MBOX_NUM_BYTES(count);
msg = (void *)(unsigned long)value;
mbox_send_message(tcu->tx, msg);
+ mbox_flush(tcu->tx, 1000);
}

static void tegra_tcu_write(struct tegra_tcu *tcu, const char *s,
@@ -184,9 +185,6 @@ static int tegra_tcu_probe(struct platform_device *pdev)
return -ENOMEM;

tcu->tx_client.dev = &pdev->dev;
- tcu->tx_client.tx_block = true;
- tcu->tx_client.tx_tout = 10000;
- tcu->rx_client.dev = &pdev->dev;
tcu->rx_client.rx_callback = tegra_tcu_receive;

tcu->tx = mbox_request_channel_byname(&tcu->tx_client, "tx");
diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
index 44348710953f..faa7da3c9c8b 100644
--- a/include/linux/mailbox_client.h
+++ b/include/linux/mailbox_client.h
@@ -44,6 +44,7 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
const char *name);
struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index);
int mbox_send_message(struct mbox_chan *chan, void *mssg);
+int mbox_flush(struct mbox_chan *chan, unsigned long timeout);
void mbox_client_txdone(struct mbox_chan *chan, int r); /* atomic */
bool mbox_client_peek_data(struct mbox_chan *chan); /* atomic */
void mbox_free_channel(struct mbox_chan *chan); /* may sleep */

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org