On 13/11/2018 13:09, Thierry Reding wrote:
> On Tue, Nov 13, 2018 at 11:09:22AM +0000, Jon Hunter wrote:
>>
>> On 12/11/2018 15:18, Thierry Reding wrote:
>>> From: Thierry Reding <treding@xxxxxxxxxx>
>>>
>>> The Tegra HSP block supports 'shared mailboxes' that are simple 32-bit
>>> registers consisting of a FULL bit in MSB position and 31 bits of data.
>>> The hardware can be configured to trigger interrupts when a mailbox
>>> is empty or full. Add support for these shared mailboxes to the HSP
>>> driver.
>>>
>>> The initial use for the mailboxes is the Tegra Combined UART. For this
>>> purpose, we use interrupts to receive data, and spinning to wait for
>>> the transmit mailbox to be emptied to minimize unnecessary overhead.
>>>
>>> Based on work by Mikko Perttunen <mperttunen@xxxxxxxxxx>.
>>>
>>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
>>> ---
>>> Changes in v2:
>>> - do not write per-mailbox interrupt enable registers on Tegra186
>>> - merge handlers for empty and full interrupts
>>> - track direction of shared mailboxes
>>>
>>> drivers/mailbox/tegra-hsp.c | 498 +++++++++++++++++++++++++++++++-----
>>> 1 file changed, 437 insertions(+), 61 deletions(-)
>>>
>>> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
>>> index 0cde356c11ab..0100a974149b 100644
>>> --- a/drivers/mailbox/tegra-hsp.c
>>> +++ b/drivers/mailbox/tegra-hsp.c
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
>>> + * Copyright (c) 2016-2018, NVIDIA CORPORATION. All rights reserved.
>>> *
>>> * This program is free software; you can redistribute it and/or modify it
>>> * under the terms and conditions of the GNU General Public License,
>>> @@ -11,6 +11,7 @@
>>> * more details.
>>> */
>>>
>>> +#include <linux/delay.h>
>>> #include <linux/interrupt.h>
>>> #include <linux/io.h>
>>> #include <linux/mailbox_controller.h>
>>> @@ -21,6 +22,17 @@
>>>
>>> #include <dt-bindings/mailbox/tegra186-hsp.h>
>>>
>>> +#include "mailbox.h"
>>> +
>>> +#define HSP_INT_IE(x) (0x100 + ((x) * 4))
>>> +#define HSP_INT_IV 0x300
>>> +#define HSP_INT_IR 0x304
>>> +
>>> +#define HSP_INT_EMPTY_SHIFT 0
>>> +#define HSP_INT_EMPTY_MASK 0xff
>>> +#define HSP_INT_FULL_SHIFT 8
>>> +#define HSP_INT_FULL_MASK 0xff
>>> +
>>> #define HSP_INT_DIMENSIONING 0x380
>>> #define HSP_nSM_SHIFT 0
>>> #define HSP_nSS_SHIFT 4
>>> @@ -34,6 +46,11 @@
>>> #define HSP_DB_RAW 0x8
>>> #define HSP_DB_PENDING 0xc
>>>
>>> +#define HSP_SM_SHRD_MBOX 0x0
>>> +#define HSP_SM_SHRD_MBOX_FULL BIT(31)
>>> +#define HSP_SM_SHRD_MBOX_FULL_INT_IE 0x04
>>> +#define HSP_SM_SHRD_MBOX_EMPTY_INT_IE 0x08
>>> +
>>> #define HSP_DB_CCPLEX 1
>>> #define HSP_DB_BPMP 3
>>> #define HSP_DB_MAX 7
>>> @@ -55,6 +72,12 @@ struct tegra_hsp_doorbell {
>>> unsigned int index;
>>> };
>>>
>>> +struct tegra_hsp_mailbox {
>>> + struct tegra_hsp_channel channel;
>>> + unsigned int index;
>>> + bool producer;
>>> +};
>>> +
>>> struct tegra_hsp_db_map {
>>> const char *name;
>>> unsigned int master;
>>> @@ -63,13 +86,18 @@ struct tegra_hsp_db_map {
>>>
>>> struct tegra_hsp_soc {
>>> const struct tegra_hsp_db_map *map;
>>> + bool has_per_mb_ie;
>>> };
>>>
>>> struct tegra_hsp {
>>> + struct device *dev;
>>> const struct tegra_hsp_soc *soc;
>>> - struct mbox_controller mbox;
>>> + struct mbox_controller mbox_db;
>>> + struct mbox_controller mbox_sm;
>>> void __iomem *regs;
>>> - unsigned int irq;
>>> + unsigned int doorbell_irq;
>>> + unsigned int *shared_irqs;
>>> + unsigned int shared_irq;
>>> unsigned int num_sm;
>>> unsigned int num_as;
>>> unsigned int num_ss;
>>> @@ -78,13 +106,10 @@ struct tegra_hsp {
>>> spinlock_t lock;
>>>
>>> struct list_head doorbells;
>>> -};
>>> + struct tegra_hsp_mailbox *mailboxes;
>>>
>>> -static inline struct tegra_hsp *
>>> -to_tegra_hsp(struct mbox_controller *mbox)
>>> -{
>>> - return container_of(mbox, struct tegra_hsp, mbox);
>>> -}
>>> + unsigned long mask;
>>> +};
>>>
>>> static inline u32 tegra_hsp_readl(struct tegra_hsp *hsp, unsigned int offset)
>>> {
>>> @@ -158,7 +183,7 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data)
>>>
>>> spin_lock(&hsp->lock);
>>>
>>> - for_each_set_bit(master, &value, hsp->mbox.num_chans) {
>>> + for_each_set_bit(master, &value, hsp->mbox_db.num_chans) {
>>> struct tegra_hsp_doorbell *db;
>>>
>>> db = __tegra_hsp_doorbell_get(hsp, master);
>>> @@ -182,6 +207,71 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data)
>>> return IRQ_HANDLED;
>>> }
>>>
>>> +static irqreturn_t tegra_hsp_shared_irq(int irq, void *data)
>>> +{
>>> + struct tegra_hsp *hsp = data;
>>> + unsigned long bit, mask;
>>> + u32 status, value;
>>> + void *msg;
>>> +
>>> + status = tegra_hsp_readl(hsp, HSP_INT_IR) & hsp->mask;
>>> +
>>> + /* process EMPTY interrupts first */
>>> + mask = (status >> HSP_INT_EMPTY_SHIFT) & HSP_INT_EMPTY_MASK;
>>> +
>>> + for_each_set_bit(bit, &mask, hsp->num_sm) {
>>> + struct tegra_hsp_mailbox *mb = &hsp->mailboxes[bit];
>>> +
>>> + if (mb->producer) {
>>> + /*
>>> + * Disable EMPTY interrupts until data is sent with
>>> + * the next message. These interrupts are level-
>>> + * triggered, so if we kept them enabled they would
>>> + * constantly trigger until we next write data into
>>> + * the message.
>>> + */
>>> + spin_lock(&hsp->lock);
>>> +
>>> + hsp->mask &= ~BIT(HSP_INT_EMPTY_SHIFT + mb->index);
>>> + tegra_hsp_writel(hsp, hsp->mask,
>>> + HSP_INT_IE(hsp->shared_irq));
>>> +
>>> + spin_unlock(&hsp->lock);
>>> +
>>> + mbox_chan_txdone(mb->channel.chan, 0);
>>> + }
>>> + }
>>> +
>>> + /* process FULL interrupts */
>>> + mask = (status >> HSP_INT_FULL_SHIFT) & HSP_INT_FULL_MASK;
>>> +
>>> + for_each_set_bit(bit, &mask, hsp->num_sm) {
>>> + struct tegra_hsp_mailbox *mb = &hsp->mailboxes[bit];
>>> +
>>> + if (!mb->producer) {
>>> + value = tegra_hsp_channel_readl(&mb->channel,
>>> + HSP_SM_SHRD_MBOX);
>>> + value &= ~HSP_SM_SHRD_MBOX_FULL;
>>> + msg = (void *)(unsigned long)value;
>>> + mbox_chan_received_data(mb->channel.chan, msg);
>>> +
>>> + /*
>>> + * Need to clear all bits here since some producers,
>>> + * such as TCU, depend on fields in the register
>>> + * getting cleared by the consumer.
>>> + *
>>> + * The mailbox API doesn't give the consumers a way
>>> + * of doing that explicitly, so we have to make sure
>>> + * we cover all possible cases.
>>> + */
>>> + tegra_hsp_channel_writel(&mb->channel, 0x0,
>>> + HSP_SM_SHRD_MBOX);
>>> + }
>>> + }
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> static struct tegra_hsp_channel *
>>> tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
>>> unsigned int master, unsigned int index)
>>> @@ -194,7 +284,7 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
>>> if (!db)
>>> return ERR_PTR(-ENOMEM);
>>>
>>> - offset = (1 + (hsp->num_sm / 2) + hsp->num_ss + hsp->num_as) << 16;
>>> + offset = (1 + (hsp->num_sm / 2) + hsp->num_ss + hsp->num_as) * SZ_64K;
>>> offset += index * 0x100;
>>>
>>> db->channel.regs = hsp->regs + offset;
>>> @@ -235,8 +325,8 @@ static int tegra_hsp_doorbell_startup(struct mbox_chan *chan)
>>> unsigned long flags;
>>> u32 value;
>>>
>>> - if (db->master >= hsp->mbox.num_chans) {
>>> - dev_err(hsp->mbox.dev,
>>> + if (db->master >= chan->mbox->num_chans) {
>>> + dev_err(chan->mbox->dev,
>>> "invalid master ID %u for HSP channel\n",
>>> db->master);
>>> return -EINVAL;
>>> @@ -281,46 +371,168 @@ static void tegra_hsp_doorbell_shutdown(struct mbox_chan *chan)
>>> spin_unlock_irqrestore(&hsp->lock, flags);
>>> }
>>>
>>> -static const struct mbox_chan_ops tegra_hsp_doorbell_ops = {
>>> +static const struct mbox_chan_ops tegra_hsp_db_ops = {
>>> .send_data = tegra_hsp_doorbell_send_data,
>>> .startup = tegra_hsp_doorbell_startup,
>>> .shutdown = tegra_hsp_doorbell_shutdown,
>>> };
>>>
>>> -static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,
>>> +static int tegra_hsp_mailbox_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> + struct tegra_hsp_mailbox *mb = chan->con_priv;
>>> + struct tegra_hsp *hsp = mb->channel.hsp;
>>> + unsigned long flags;
>>> + u32 value;
>>> +
>>> + WARN_ON(!mb->producer);
>>
>> Should we return here?
>
> Yeah, that's a good idea. I made this return -EPERM for lack of a better
> error code.
>
>>> +
>>> + /* copy data and mark mailbox full */
>>> + value = (u32)(unsigned long)data;
>>> + value |= HSP_SM_SHRD_MBOX_FULL;
>>> +
>>> + 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);
>>> +
>>> + 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);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra_hsp_mailbox_flush(struct mbox_chan *chan,
>>> + unsigned long timeout)
>>> +{
>>> + struct tegra_hsp_mailbox *mb = chan->con_priv;
>>> + struct tegra_hsp_channel *ch = &mb->channel;
>>> + u32 value;
>>> +
>>> + timeout = jiffies + msecs_to_jiffies(timeout);
>>> +
>>> + while (time_before(jiffies, timeout)) {
>>> + value = tegra_hsp_channel_readl(ch, HSP_SM_SHRD_MBOX);
>>> + if ((value & HSP_SM_SHRD_MBOX_FULL) == 0) {
>>> + mbox_chan_txdone(chan, 0);
>>> + return 0;
>>> + }
>>> +
>>> + udelay(1);
>>> + }
>>> +
>>> + return -ETIME;
>>> +}
>>> +
>>> +static int tegra_hsp_mailbox_startup(struct mbox_chan *chan)
>>> +{
>>> + struct tegra_hsp_mailbox *mb = chan->con_priv;
>>> + struct tegra_hsp_channel *ch = &mb->channel;
>>> + struct tegra_hsp *hsp = mb->channel.hsp;
>>> + unsigned long flags;
>>> +
>>> + chan->txdone_method = TXDONE_BY_IRQ;
>>> +
>>> + /*
>>> + * Shared mailboxes start out as consumers by default. FULL and EMPTY
>>> + * interrupts are coalesced at the same shared interrupt.
>>> + *
>>> + * Keep EMPTY interrupts disabled at startup and only enable them when
>>> + * the mailbox is actually full. This is required because the FULL and
>>> + * EMPTY interrupts are level-triggered, so keeping EMPTY interrupts
>>> + * enabled all the time would cause an interrupt storm while mailboxes
>>> + * are idle.
>>> + */
>>> +
>>> + spin_lock_irqsave(&hsp->lock, flags);
>>> +
>>> + if (mb->producer)
>>> + hsp->mask &= ~BIT(HSP_INT_EMPTY_SHIFT + mb->index);
>>> + else
>>> + hsp->mask |= BIT(HSP_INT_FULL_SHIFT + mb->index);
>>> +
>>> + tegra_hsp_writel(hsp, hsp->mask, HSP_INT_IE(hsp->shared_irq));
>>> +
>>> + spin_unlock_irqrestore(&hsp->lock, flags);
>>> +
>>> + if (hsp->soc->has_per_mb_ie) {
>>> + if (mb->producer)
>>> + tegra_hsp_channel_writel(ch, 0x0,
>>> + HSP_SM_SHRD_MBOX_EMPTY_INT_IE);
>>> + else
>>> + tegra_hsp_channel_writel(ch, 0x1,
>>> + HSP_SM_SHRD_MBOX_FULL_INT_IE);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void tegra_hsp_mailbox_shutdown(struct mbox_chan *chan)
>>> +{
>>> + struct tegra_hsp_mailbox *mb = chan->con_priv;
>>> + struct tegra_hsp_channel *ch = &mb->channel;
>>> + struct tegra_hsp *hsp = mb->channel.hsp;
>>> + unsigned long flags;
>>> +
>>> + if (hsp->soc->has_per_mb_ie) {
>>> + if (mb->producer)
>>> + tegra_hsp_channel_writel(ch, 0x0,
>>> + HSP_SM_SHRD_MBOX_EMPTY_INT_IE);
>>> + else
>>> + tegra_hsp_channel_writel(ch, 0x0,
>>> + HSP_SM_SHRD_MBOX_FULL_INT_IE);
>>> + }
>>> +
>>> + spin_lock_irqsave(&hsp->lock, flags);
>>> +
>>> + if (mb->producer)
>>> + hsp->mask &= ~BIT(HSP_INT_EMPTY_SHIFT + mb->index);
>>> + else
>>> + hsp->mask &= ~BIT(HSP_INT_FULL_SHIFT + mb->index);
>>> +
>>> + tegra_hsp_writel(hsp, hsp->mask, HSP_INT_IE(hsp->shared_irq));
>>> +
>>> + spin_unlock_irqrestore(&hsp->lock, flags);
>>> +}
>>> +
>>> +static const struct mbox_chan_ops tegra_hsp_sm_ops = {
>>> + .send_data = tegra_hsp_mailbox_send_data,
>>> + .flush = tegra_hsp_mailbox_flush,
>>> + .startup = tegra_hsp_mailbox_startup,
>>> + .shutdown = tegra_hsp_mailbox_shutdown,
>>> +};
>>> +
>>> +static struct mbox_chan *tegra_hsp_db_xlate(struct mbox_controller *mbox,
>>> const struct of_phandle_args *args)
>>> {
>>> + struct tegra_hsp *hsp = container_of(mbox, struct tegra_hsp, mbox_db);
>>> + unsigned int type = args->args[0], master = args->args[1];
>>> struct tegra_hsp_channel *channel = ERR_PTR(-ENODEV);
>>> - struct tegra_hsp *hsp = to_tegra_hsp(mbox);
>>> - unsigned int type = args->args[0];
>>> - unsigned int master = args->args[1];
>>> struct tegra_hsp_doorbell *db;
>>> struct mbox_chan *chan;
>>> unsigned long flags;
>>> unsigned int i;
>>>
>>> - switch (type) {
>>> - case TEGRA_HSP_MBOX_TYPE_DB:
>>> - db = tegra_hsp_doorbell_get(hsp, master);
>>> - if (db)
>>> - channel = &db->channel;
>>> + if (type != TEGRA_HSP_MBOX_TYPE_DB || !hsp->doorbell_irq)
>>> + return ERR_PTR(-ENODEV);
>>>
>>> - break;
>>> -
>>> - default:
>>> - break;
>>> - }
>>> + db = tegra_hsp_doorbell_get(hsp, master);
>>> + if (db)
>>> + channel = &db->channel;
>>>
>>> if (IS_ERR(channel))
>>> return ERR_CAST(channel);
>>>
>>> spin_lock_irqsave(&hsp->lock, flags);
>>>
>>> - for (i = 0; i < hsp->mbox.num_chans; i++) {
>>> - chan = &hsp->mbox.chans[i];
>>> + for (i = 0; i < mbox->num_chans; i++) {
>>> + chan = &mbox->chans[i];
>>> if (!chan->con_priv) {
>>> - chan->con_priv = channel;
>>> channel->chan = chan;
>>> + chan->con_priv = db;
>>> break;
>>> }
>>>
>>> @@ -332,6 +544,29 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,
>>> return chan ?: ERR_PTR(-EBUSY);
>>> }
>>>
>>> +static struct mbox_chan *tegra_hsp_sm_xlate(struct mbox_controller *mbox,
>>> + const struct of_phandle_args *args)
>>> +{
>>> + struct tegra_hsp *hsp = container_of(mbox, struct tegra_hsp, mbox_sm);
>>> + unsigned int type = args->args[0], index;
>>> + struct tegra_hsp_mailbox *mb;
>>> +
>>> + index = args->args[1] & TEGRA_HSP_SM_MASK;
>>> +
>>> + if (type != TEGRA_HSP_MBOX_TYPE_SM || !hsp->shared_irqs ||
>>> + index >= hsp->num_sm)
>>> + return ERR_PTR(-ENODEV);
>>> +
>>> + mb = &hsp->mailboxes[index];
>>> +
>>> + if ((args->args[1] & TEGRA_HSP_SM_FLAG_TX) == 0)
>>> + mb->producer = false;
>>> + else
>>> + mb->producer = true;
>>> +
>>> + return mb->channel.chan;
>>> +}
>>> +
>>> static void tegra_hsp_remove_doorbells(struct tegra_hsp *hsp)
>>> {
>>> struct tegra_hsp_doorbell *db, *tmp;
>>> @@ -364,10 +599,65 @@ static int tegra_hsp_add_doorbells(struct tegra_hsp *hsp)
>>> return 0;
>>> }
>>>
>>> +static int tegra_hsp_add_mailboxes(struct tegra_hsp *hsp, struct device *dev)
>>> +{
>>> + int i;
>>> +
>>> + hsp->mailboxes = devm_kcalloc(dev, hsp->num_sm, sizeof(*hsp->mailboxes),
>>> + GFP_KERNEL);
>>> + if (!hsp->mailboxes)
>>> + return -ENOMEM;
>>> +
>>> + for (i = 0; i < hsp->num_sm; i++) {
>>> + struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
>>> +
>>> + mb->index = i;
>>> +
>>> + mb->channel.hsp = hsp;
>>> + mb->channel.regs = hsp->regs + SZ_64K + i * SZ_32K;
>>> + mb->channel.chan = &hsp->mbox_sm.chans[i];
>>> + mb->channel.chan->con_priv = mb;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra_hsp_request_shared_irqs(struct tegra_hsp *hsp)
>>> +{
>>> + unsigned int i, irq = 0;
>>> + int err;
>>> +
>>> + for (i = 0; i < hsp->num_si; i++) {
>>> + if (hsp->shared_irq == 0 && hsp->shared_irqs[i] > 0) {
>>> + irq = hsp->shared_irqs[i];
>>> + hsp->shared_irq = i;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (irq > 0) {
>>> + err = devm_request_irq(hsp->dev, irq, tegra_hsp_shared_irq, 0,
>>> + dev_name(hsp->dev), hsp);
>>> + if (err < 0) {
>>> + dev_err(hsp->dev, "failed to request interrupt: %d\n",
>>> + err);
>>> + return err;
>>> + }
>>
>> Are we suppose to loop through all the shared interrupts and find one
>> that is available? Looking at the above it seems that we will try to use
>> the first shared interrupt we have a valid mapping for, regardless of if
>> it is available/in-use.
>>
>> Otherwise I am not sure why it is necessary to stored all the shared
>> irqs, because AFAICT we only use the first we have a valid mapping for.
>> Maybe I am missing something ...
>
> Yeah, that's a good idea. In practice I don't think this matters at all
> because there just isn't another user of these interrupts, but it might
> be more explicit and self-explanatory if we try all of the interrupts
> in turn until we can request one.
By the way, any reason why we could not put the call to
platform_get_irq_byname() in the above function? May simplify the code a
bit to have a single loop.
Cheers
Jon
--
nvpublic
