* [PATCH 1/3] serial: imx: fix error handling in console_setup
From: Stefan Agner @ 2018-11-14 17:49 UTC (permalink / raw)
To: gregkh, jslaby
Cc: fabio.estevam, u.kleine-koenig, s.hauer, linux-serial,
linux-kernel, Stefan Agner
The ipg clock only needs to be unprepared in case preparing
per clock fails. The ipg clock has already disabled at the point.
Fixes: 1cf93e0d5488 ("serial: imx: remove the uart_console() check")
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
drivers/tty/serial/imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0f67197a3783..313c3b1900a8 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2068,7 +2068,7 @@ imx_uart_console_setup(struct console *co, char *options)
retval = clk_prepare(sport->clk_per);
if (retval)
- clk_disable_unprepare(sport->clk_ipg);
+ clk_unprepare(sport->clk_ipg);
error_console:
return retval;
--
2.19.1
^ permalink raw reply related
* [PATCH 4/4] serdev: document the write functions using kernel-doc
From: Johan Hovold @ 2018-11-14 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Andrey Smirnov,
linux-serial, linux-kernel
In-Reply-To: <20181114150904.19653-1-johan@kernel.org>
Document the asynchronous serdev_device_write_buf() and synchronous
serdev_device_write() functions using kernel-doc.
Specifically, mention that writing data only means that data has been
buffered by the controller, and that the synchronous helper depends on
serdev_device_write_wakeup() being called in the driver write_wakeup()
callback.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serdev/core.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index c7006bbb793a..a0ac16ee6575 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -217,6 +217,21 @@ void serdev_device_write_wakeup(struct serdev_device *serdev)
}
EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
+/**
+ * serdev_device_write_buf() - write data asynchronously
+ * @serdev: serdev device
+ * @buf: data to be written
+ * @count: number of bytes to write
+ *
+ * Write data to the device asynchronously.
+ *
+ * Note that any accepted data has only been buffered by the controller; use
+ * serdev_device_wait_until_sent() to make sure the controller write buffer
+ * has actually been emptied.
+ *
+ * Return: The number of bytes written (less than count if not enough room in
+ * the write buffer), or a negative errno on errors.
+ */
int serdev_device_write_buf(struct serdev_device *serdev,
const unsigned char *buf, size_t count)
{
@@ -229,6 +244,28 @@ int serdev_device_write_buf(struct serdev_device *serdev,
}
EXPORT_SYMBOL_GPL(serdev_device_write_buf);
+/**
+ * serdev_device_write() - write data synchronously
+ * @serdev: serdev device
+ * @buf: data to be written
+ * @count: number of bytes to write
+ * @timeout: timeout in jiffies, or 0 to wait indefinitely
+ *
+ * Write data to the device synchronously by repeatedly calling
+ * serdev_device_write() until the controller has accepted all data (unless
+ * interrupted by a timeout or a signal).
+ *
+ * Note that any accepted data has only been buffered by the controller; use
+ * serdev_device_wait_until_sent() to make sure the controller write buffer
+ * has actually been emptied.
+ *
+ * Note that this function depends on serdev_device_write_wakeup() being
+ * called in the serdev driver write_wakeup() callback.
+ *
+ * Return: The number of bytes written (less than count if interrupted),
+ * -ETIMEDOUT or -ERESTARTSYS if interrupted before any bytes were written, or
+ * a negative errno on errors.
+ */
int serdev_device_write(struct serdev_device *serdev,
const unsigned char *buf, size_t count,
long timeout)
--
2.19.1
^ permalink raw reply related
* [PATCH 3/4] serdev: make synchronous write helper interruptible
From: Johan Hovold @ 2018-11-14 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Andrey Smirnov,
linux-serial, linux-kernel
In-Reply-To: <20181114150904.19653-1-johan@kernel.org>
Allow the synchronous serdev_device_write() helper to be interrupted.
This is useful for cases where I/O is performed on behalf of user space
and we don't want to block indefinitely when using flow control.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serdev/core.c | 20 ++++++++++++++------
include/linux/serdev.h | 2 +-
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index ee4c40336633..c7006bbb793a 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -231,7 +231,7 @@ EXPORT_SYMBOL_GPL(serdev_device_write_buf);
int serdev_device_write(struct serdev_device *serdev,
const unsigned char *buf, size_t count,
- unsigned long timeout)
+ long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
int written = 0;
@@ -254,16 +254,24 @@ int serdev_device_write(struct serdev_device *serdev,
written += ret;
buf += ret;
count -= ret;
- } while (count &&
- (timeout = wait_for_completion_timeout(&serdev->write_comp,
- timeout)));
+
+ if (count == 0)
+ break;
+
+ timeout = wait_for_completion_interruptible_timeout(&serdev->write_comp,
+ timeout);
+ } while (timeout > 0);
mutex_unlock(&serdev->write_lock);
if (ret < 0)
return ret;
- if (timeout == 0 && written == 0)
- return -ETIMEDOUT;
+ if (timeout <= 0 && written == 0) {
+ if (timeout == -ERESTARTSYS)
+ return -ERESTARTSYS;
+ else
+ return -ETIMEDOUT;
+ }
return written;
}
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index f153b2c7f0cd..070bf4e92df7 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -210,7 +210,7 @@ void serdev_device_wait_until_sent(struct serdev_device *, long);
int serdev_device_get_tiocm(struct serdev_device *);
int serdev_device_set_tiocm(struct serdev_device *, int, int);
void serdev_device_write_wakeup(struct serdev_device *);
-int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, unsigned long);
+int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, long);
void serdev_device_write_flush(struct serdev_device *);
int serdev_device_write_room(struct serdev_device *);
--
2.19.1
^ permalink raw reply related
* [PATCH 2/4] serdev: make synchronous write return bytes written
From: Johan Hovold @ 2018-11-14 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Andrey Smirnov,
linux-serial, linux-kernel
In-Reply-To: <20181114150904.19653-1-johan@kernel.org>
Make the synchronous serdev_device_write() helper behave analogous to
the asynchronous serdev_device_write_buf() by returning the number of
bytes written (or rather buffered) also on timeout.
This will allow drivers to distinguish the case where data was partially
written from the case where no data was written.
Also update the only two users that checked the return value.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/gnss/serial.c | 2 +-
drivers/gnss/sirf.c | 2 +-
drivers/tty/serdev/core.c | 12 ++++++++++--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gnss/serial.c b/drivers/gnss/serial.c
index 31e891f00175..def64b36d994 100644
--- a/drivers/gnss/serial.c
+++ b/drivers/gnss/serial.c
@@ -65,7 +65,7 @@ static int gnss_serial_write_raw(struct gnss_device *gdev,
/* write is only buffered synchronously */
ret = serdev_device_write(serdev, buf, count, MAX_SCHEDULE_TIMEOUT);
- if (ret < 0)
+ if (ret < 0 || ret < count)
return ret;
/* FIXME: determine if interrupted? */
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
index 71d014edd167..b3a4c0e91947 100644
--- a/drivers/gnss/sirf.c
+++ b/drivers/gnss/sirf.c
@@ -85,7 +85,7 @@ static int sirf_write_raw(struct gnss_device *gdev, const unsigned char *buf,
/* write is only buffered synchronously */
ret = serdev_device_write(serdev, buf, count, MAX_SCHEDULE_TIMEOUT);
- if (ret < 0)
+ if (ret < 0 || ret < count)
return ret;
/* FIXME: determine if interrupted? */
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index c7d637d2bc56..ee4c40336633 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -234,6 +234,7 @@ int serdev_device_write(struct serdev_device *serdev,
unsigned long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
+ int written = 0;
int ret;
if (!ctrl || !ctrl->ops->write_buf || !serdev->ops->write_wakeup)
@@ -250,14 +251,21 @@ int serdev_device_write(struct serdev_device *serdev,
if (ret < 0)
break;
+ written += ret;
buf += ret;
count -= ret;
-
} while (count &&
(timeout = wait_for_completion_timeout(&serdev->write_comp,
timeout)));
mutex_unlock(&serdev->write_lock);
- return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
+
+ if (ret < 0)
+ return ret;
+
+ if (timeout == 0 && written == 0)
+ return -ETIMEDOUT;
+
+ return written;
}
EXPORT_SYMBOL_GPL(serdev_device_write);
--
2.19.1
^ permalink raw reply related
* [PATCH 1/4] serdev: use zero to indicate infinite write timeout
From: Johan Hovold @ 2018-11-14 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Andrey Smirnov,
linux-serial, linux-kernel
In-Reply-To: <20181114150904.19653-1-johan@kernel.org>
Use zero to indicate infinite timeout for the synchronous
serdev_device_write() helper.
This allows drivers to specify an infinite timeout without knowing about
serdev implementation details, while also allowing the same timeout
argument to be used for both serdev_device_write() and
serdev_device_wait_until_sent().
Note that passing zero to the current helper makes no sense; just call
the asynchronous serdev_device_write_buf() directly instead.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serdev/core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 9db93f500b4e..c7d637d2bc56 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -15,6 +15,7 @@
#include <linux/of_device.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
+#include <linux/sched.h>
#include <linux/serdev.h>
#include <linux/slab.h>
@@ -235,10 +236,12 @@ int serdev_device_write(struct serdev_device *serdev,
struct serdev_controller *ctrl = serdev->ctrl;
int ret;
- if (!ctrl || !ctrl->ops->write_buf ||
- (timeout && !serdev->ops->write_wakeup))
+ if (!ctrl || !ctrl->ops->write_buf || !serdev->ops->write_wakeup)
return -EINVAL;
+ if (timeout == 0)
+ timeout = MAX_SCHEDULE_TIMEOUT;
+
mutex_lock(&serdev->write_lock);
do {
reinit_completion(&serdev->write_comp);
--
2.19.1
^ permalink raw reply related
* [PATCH 0/4] serdev: make serdev_device_write() more usable
From: Johan Hovold @ 2018-11-14 15:09 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Andrey Smirnov,
linux-serial, linux-kernel
This series make the synchronous serdev_device_write() helper more
usable by
1) allowing drivers to pass a zero timeout to indicate that they
want to wait forever;
2) returning the number of bytes actually written (buffered)
if the helper is interrupted;
3) make the helper use interruptible wait so that the helper can
be used on behalf of user space.
Finally, the two write functions are documented using kernel-doc.
Turns out I was using the wrong timeout for two gnss drivers that
expected the helper to wait indefinitely. I've fixed up those separately
(by using MAX_SCHEDULE_TIMEOUT for now), but for the helper to be usable
when using flow control we really want it to be interruptible.
Besides the two gnss drivers, there's currently only one other in-kernel
user of this helper and that driver (rave-sp) uses a non-zero timeout
and doesn't check the return value and therefore does not need to be
updated.
Note that this series depends on the two above mentioned GNSS fixes
(submitted for v4.20-rc3).
Johan
Johan Hovold (4):
serdev: use zero to indicate infinite write timeout
serdev: make synchronous write return bytes written
serdev: make synchronous write helper interruptible
serdev: document the write functions using kernel-doc
drivers/gnss/serial.c | 2 +-
drivers/gnss/sirf.c | 2 +-
drivers/tty/serdev/core.c | 70 +++++++++++++++++++++++++++++++++++----
include/linux/serdev.h | 2 +-
4 files changed, 66 insertions(+), 10 deletions(-)
--
2.19.1
^ permalink raw reply
* Re: [PATCH v2 04/10] mailbox: tegra-hsp: Add support for shared mailboxes
From: Jon Hunter @ 2018-11-13 19:24 UTC (permalink / raw)
To: Thierry Reding
Cc: devicetree, Greg Kroah-Hartman, Jassi Brar, Mika Liljeberg,
Mikko Perttunen, Timo Alho, linux-serial, Jiri Slaby, linux-tegra,
Pekka Pessi, linux-arm-kernel
In-Reply-To: <20181113130917.GA2724@ulmo>
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@nvidia.com>
>>>
>>> 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@nvidia.com>.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> 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
^ permalink raw reply
* Re: [PATCH v2 04/10] mailbox: tegra-hsp: Add support for shared mailboxes
From: Thierry Reding @ 2018-11-13 13:09 UTC (permalink / raw)
To: Jon Hunter
Cc: devicetree, Greg Kroah-Hartman, Jassi Brar, Mika Liljeberg,
Mikko Perttunen, Timo Alho, linux-serial, Jiri Slaby, linux-tegra,
Pekka Pessi, linux-arm-kernel
In-Reply-To: <fe9e6383-3735-0d07-d8d1-01199190b3a4@nvidia.com>
[-- Attachment #1.1: Type: text/plain, Size: 22776 bytes --]
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@nvidia.com>
> >
> > 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@nvidia.com>.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > 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.
> > +
> > + /* disable all interrupts */
> > + tegra_hsp_writel(hsp, 0, HSP_INT_IE(hsp->shared_irq)> +
> > + dev_dbg(hsp->dev, "interrupt requested: %u\n", irq);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int tegra_hsp_probe(struct platform_device *pdev)
> > {
> > struct tegra_hsp *hsp;
> > struct resource *res;
> > + unsigned int i;
> > u32 value;
> > int err;
> >
> > @@ -375,6 +665,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> > if (!hsp)
> > return -ENOMEM;
> >
> > + hsp->dev = &pdev->dev;
> > hsp->soc = of_device_get_match_data(&pdev->dev);
> > INIT_LIST_HEAD(&hsp->doorbells);
> > spin_lock_init(&hsp->lock);
> > @@ -392,58 +683,136 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> > hsp->num_si = (value >> HSP_nSI_SHIFT) & HSP_nINT_MASK;
> >
> > err = platform_get_irq_byname(pdev, "doorbell");
> > - if (err < 0) {
> > - dev_err(&pdev->dev, "failed to get doorbell IRQ: %d\n", err);
> > - return err;
> > + if (err >= 0)
> > + hsp->doorbell_irq = err;
> > +
> > + if (hsp->num_si > 0) {
> > + unsigned int count = 0;
> > +
> > + hsp->shared_irqs = devm_kcalloc(&pdev->dev, hsp->num_si,
> > + sizeof(*hsp->shared_irqs),
> > + GFP_KERNEL);
> > + if (!hsp->shared_irqs)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < hsp->num_si; i++) {
> > + char *name;
> > +
> > + name = kasprintf(GFP_KERNEL, "shared%u", i);
> > + if (!name)
> > + return -ENOMEM;
> > +
> > + err = platform_get_irq_byname(pdev, name);
> > + if (err >= 0) {
> > + hsp->shared_irqs[i] = err;
> > + count++;
> > + }
> > +
> > + kfree(name);
> > + }
> > +
> > + if (count == 0) {
> > + devm_kfree(&pdev->dev, hsp->shared_irqs);
> > + hsp->shared_irqs = NULL;
> > + }
> > + }
> > +
> > + /* setup the doorbell controller */
> > + hsp->mbox_db.of_xlate = tegra_hsp_db_xlate;
> > + hsp->mbox_db.num_chans = 32;
> > + hsp->mbox_db.dev = &pdev->dev;
> > + hsp->mbox_db.ops = &tegra_hsp_db_ops;
> > +
> > + hsp->mbox_db.chans = devm_kcalloc(&pdev->dev, hsp->mbox_db.num_chans,
> > + sizeof(*hsp->mbox_db.chans),
> > + GFP_KERNEL);
> > + if (!hsp->mbox_db.chans)
> > + return -ENOMEM;
> > +
> > + if (hsp->doorbell_irq) {
> > + err = tegra_hsp_add_doorbells(hsp);
> > + if (err < 0) {
> > + dev_err(&pdev->dev, "failed to add doorbells: %d\n",
> > + err);
> > + return err;
> > + }
> > }
> >
> > - hsp->irq = err;
> > + err = mbox_controller_register(&hsp->mbox_db);
> > + if (err < 0) {
> > + dev_err(&pdev->dev, "failed to register doorbell mailbox: %d\n", err);
> > + goto remove_doorbells;
> > + }
> >
> > - hsp->mbox.of_xlate = of_tegra_hsp_xlate;
> > - hsp->mbox.num_chans = 32;
> > - hsp->mbox.dev = &pdev->dev;
> > - hsp->mbox.txdone_irq = false;
> > - hsp->mbox.txdone_poll = false;
> > - hsp->mbox.ops = &tegra_hsp_doorbell_ops;
> > + /* setup the shared mailbox controller */
> > + hsp->mbox_sm.of_xlate = tegra_hsp_sm_xlate;
> > + hsp->mbox_sm.num_chans = hsp->num_sm;
> > + hsp->mbox_sm.dev = &pdev->dev;
> > + hsp->mbox_sm.ops = &tegra_hsp_sm_ops;
> >
> > - hsp->mbox.chans = devm_kcalloc(&pdev->dev, hsp->mbox.num_chans,
> > - sizeof(*hsp->mbox.chans),
> > - GFP_KERNEL);
> > - if (!hsp->mbox.chans)
> > + hsp->mbox_sm.chans = devm_kcalloc(&pdev->dev, hsp->mbox_sm.num_chans,
> > + sizeof(*hsp->mbox_sm.chans),
> > + GFP_KERNEL);
> > + if (!hsp->mbox_sm.chans)
> > return -ENOMEM;
> >
> > - err = tegra_hsp_add_doorbells(hsp);
> > + if (hsp->shared_irqs) {
> > + err = tegra_hsp_add_mailboxes(hsp, &pdev->dev);
> > + if (err < 0) {
> > + dev_err(&pdev->dev, "failed to add mailboxes: %d\n",
> > + err);
> > + goto unregister_mbox_db;
> > + }
> > + }
> > +
> > + err = mbox_controller_register(&hsp->mbox_sm);
> > if (err < 0) {
> > - dev_err(&pdev->dev, "failed to add doorbells: %d\n", err);
> > - return err;
> > + dev_err(&pdev->dev, "failed to register shared mailbox: %d\n", err);
> > + goto unregister_mbox_db;
> > }
> >
> > platform_set_drvdata(pdev, hsp);
> >
> > - err = mbox_controller_register(&hsp->mbox);
> > - if (err) {
> > - dev_err(&pdev->dev, "failed to register mailbox: %d\n", err);
> > - tegra_hsp_remove_doorbells(hsp);
> > - return err;
> > + if (hsp->doorbell_irq) {
> > + err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> > + tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> > + dev_name(&pdev->dev), hsp);
> > + if (err < 0) {
> > + dev_err(&pdev->dev,
> > + "failed to request doorbell IRQ#%u: %d\n",
> > + hsp->doorbell_irq, err);
> > + goto unregister_mbox_sm;
> > + }
> > }
> >
> > - err = devm_request_irq(&pdev->dev, hsp->irq, tegra_hsp_doorbell_irq,
> > - IRQF_NO_SUSPEND, dev_name(&pdev->dev), hsp);
> > - if (err < 0) {
> > - dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n",
> > - hsp->irq, err);
> > - return err;
> > + if (hsp->shared_irqs) {
> > + err = tegra_hsp_request_shared_irqs(hsp);
> > + if (err < 0)
> > + goto unregister_mbox_sm;
>
> Any reason why we don't request the doorbell and shared irqs earlier?
> Given we use devm seems it could simplify the clean and avoid
> unregistering the mailbox.
The interrupt handlers need to access the fields that are set up in the
above sequence, so if we request them earlier they may run without this
data being initialized and crash. Now, since we disable the shared
interrupts that shouldn't happen, but better to be safe than sorry.
Thierry
[-- 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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 05/10] mailbox: tegra-hsp: Add suspend/resume support
From: Jon Hunter @ 2018-11-13 11:17 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-6-thierry.reding@gmail.com>
On 12/11/2018 15:18, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Upon resuming from a system sleep state, the interrupts for all active
> shared mailboxes need to be reenabled, otherwise they will not work.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/mailbox/tegra-hsp.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> index 0100a974149b..1259abf3542f 100644
> --- a/drivers/mailbox/tegra-hsp.c
> +++ b/drivers/mailbox/tegra-hsp.c
> @@ -18,6 +18,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
>
> #include <dt-bindings/mailbox/tegra186-hsp.h>
> @@ -817,6 +818,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int tegra_hsp_resume(struct device *dev)
> +{
> + struct tegra_hsp *hsp = dev_get_drvdata(dev);
> + unsigned int i;
> +
> + for (i = 0; i < hsp->num_sm; i++) {
> + struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
> +
> + if (mb->channel.chan->cl)
> + tegra_hsp_mailbox_startup(mb->channel.chan);
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
Is it worth disabling interrupts on suspend to avoid any in-flight
interrupt triggering a bad access on entering suspend? I assume that the
context of the mailbox registers get cleared/lost at some point and so I
was not sure if there is a time where they have a bad state or are
inaccessible?
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 04/10] mailbox: tegra-hsp: Add support for shared mailboxes
From: Jon Hunter @ 2018-11-13 11:09 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-5-thierry.reding@gmail.com>
On 12/11/2018 15:18, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> 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@nvidia.com>.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> 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?
> +
> + /* 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 ...
> +
> + /* disable all interrupts */
> + tegra_hsp_writel(hsp, 0, HSP_INT_IE(hsp->shared_irq)> +
> + dev_dbg(hsp->dev, "interrupt requested: %u\n", irq);
> + }
> +
> + return 0;
> +}
> +
> static int tegra_hsp_probe(struct platform_device *pdev)
> {
> struct tegra_hsp *hsp;
> struct resource *res;
> + unsigned int i;
> u32 value;
> int err;
>
> @@ -375,6 +665,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> if (!hsp)
> return -ENOMEM;
>
> + hsp->dev = &pdev->dev;
> hsp->soc = of_device_get_match_data(&pdev->dev);
> INIT_LIST_HEAD(&hsp->doorbells);
> spin_lock_init(&hsp->lock);
> @@ -392,58 +683,136 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> hsp->num_si = (value >> HSP_nSI_SHIFT) & HSP_nINT_MASK;
>
> err = platform_get_irq_byname(pdev, "doorbell");
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to get doorbell IRQ: %d\n", err);
> - return err;
> + if (err >= 0)
> + hsp->doorbell_irq = err;
> +
> + if (hsp->num_si > 0) {
> + unsigned int count = 0;
> +
> + hsp->shared_irqs = devm_kcalloc(&pdev->dev, hsp->num_si,
> + sizeof(*hsp->shared_irqs),
> + GFP_KERNEL);
> + if (!hsp->shared_irqs)
> + return -ENOMEM;
> +
> + for (i = 0; i < hsp->num_si; i++) {
> + char *name;
> +
> + name = kasprintf(GFP_KERNEL, "shared%u", i);
> + if (!name)
> + return -ENOMEM;
> +
> + err = platform_get_irq_byname(pdev, name);
> + if (err >= 0) {
> + hsp->shared_irqs[i] = err;
> + count++;
> + }
> +
> + kfree(name);
> + }
> +
> + if (count == 0) {
> + devm_kfree(&pdev->dev, hsp->shared_irqs);
> + hsp->shared_irqs = NULL;
> + }
> + }
> +
> + /* setup the doorbell controller */
> + hsp->mbox_db.of_xlate = tegra_hsp_db_xlate;
> + hsp->mbox_db.num_chans = 32;
> + hsp->mbox_db.dev = &pdev->dev;
> + hsp->mbox_db.ops = &tegra_hsp_db_ops;
> +
> + hsp->mbox_db.chans = devm_kcalloc(&pdev->dev, hsp->mbox_db.num_chans,
> + sizeof(*hsp->mbox_db.chans),
> + GFP_KERNEL);
> + if (!hsp->mbox_db.chans)
> + return -ENOMEM;
> +
> + if (hsp->doorbell_irq) {
> + err = tegra_hsp_add_doorbells(hsp);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to add doorbells: %d\n",
> + err);
> + return err;
> + }
> }
>
> - hsp->irq = err;
> + err = mbox_controller_register(&hsp->mbox_db);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to register doorbell mailbox: %d\n", err);
> + goto remove_doorbells;
> + }
>
> - hsp->mbox.of_xlate = of_tegra_hsp_xlate;
> - hsp->mbox.num_chans = 32;
> - hsp->mbox.dev = &pdev->dev;
> - hsp->mbox.txdone_irq = false;
> - hsp->mbox.txdone_poll = false;
> - hsp->mbox.ops = &tegra_hsp_doorbell_ops;
> + /* setup the shared mailbox controller */
> + hsp->mbox_sm.of_xlate = tegra_hsp_sm_xlate;
> + hsp->mbox_sm.num_chans = hsp->num_sm;
> + hsp->mbox_sm.dev = &pdev->dev;
> + hsp->mbox_sm.ops = &tegra_hsp_sm_ops;
>
> - hsp->mbox.chans = devm_kcalloc(&pdev->dev, hsp->mbox.num_chans,
> - sizeof(*hsp->mbox.chans),
> - GFP_KERNEL);
> - if (!hsp->mbox.chans)
> + hsp->mbox_sm.chans = devm_kcalloc(&pdev->dev, hsp->mbox_sm.num_chans,
> + sizeof(*hsp->mbox_sm.chans),
> + GFP_KERNEL);
> + if (!hsp->mbox_sm.chans)
> return -ENOMEM;
>
> - err = tegra_hsp_add_doorbells(hsp);
> + if (hsp->shared_irqs) {
> + err = tegra_hsp_add_mailboxes(hsp, &pdev->dev);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to add mailboxes: %d\n",
> + err);
> + goto unregister_mbox_db;
> + }
> + }
> +
> + err = mbox_controller_register(&hsp->mbox_sm);
> if (err < 0) {
> - dev_err(&pdev->dev, "failed to add doorbells: %d\n", err);
> - return err;
> + dev_err(&pdev->dev, "failed to register shared mailbox: %d\n", err);
> + goto unregister_mbox_db;
> }
>
> platform_set_drvdata(pdev, hsp);
>
> - err = mbox_controller_register(&hsp->mbox);
> - if (err) {
> - dev_err(&pdev->dev, "failed to register mailbox: %d\n", err);
> - tegra_hsp_remove_doorbells(hsp);
> - return err;
> + if (hsp->doorbell_irq) {
> + err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> + tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> + dev_name(&pdev->dev), hsp);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed to request doorbell IRQ#%u: %d\n",
> + hsp->doorbell_irq, err);
> + goto unregister_mbox_sm;
> + }
> }
>
> - err = devm_request_irq(&pdev->dev, hsp->irq, tegra_hsp_doorbell_irq,
> - IRQF_NO_SUSPEND, dev_name(&pdev->dev), hsp);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n",
> - hsp->irq, err);
> - return err;
> + if (hsp->shared_irqs) {
> + err = tegra_hsp_request_shared_irqs(hsp);
> + if (err < 0)
> + goto unregister_mbox_sm;
Any reason why we don't request the doorbell and shared irqs earlier?
Given we use devm seems it could simplify the clean and avoid
unregistering the mailbox.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 06/10] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Jon Hunter @ 2018-11-13 10:11 UTC (permalink / raw)
To: Thierry Reding
Cc: devicetree, Greg Kroah-Hartman, Jassi Brar, Mika Liljeberg,
Mikko Perttunen, Timo Alho, linux-serial, Jiri Slaby, linux-tegra,
Pekka Pessi, linux-arm-kernel
In-Reply-To: <20181113100355.GA15642@ulmo>
On 13/11/2018 10:03, Thierry Reding wrote:
> On Tue, Nov 13, 2018 at 09:39:08AM +0000, Jon Hunter wrote:
>>
>> On 12/11/2018 15:18, Thierry Reding wrote:
>>> From: Mikko Perttunen <mperttunen@nvidia.com>
>>>
>>> Add bindings for the Tegra Combined UART device used to talk to the
>>> UART console on Tegra194 systems.
>>>
>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> .../bindings/serial/nvidia,tegra194-tcu.txt | 35 +++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
>>> new file mode 100644
>>> index 000000000000..085a8591accd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
>>> @@ -0,0 +1,35 @@
>>> +NVIDIA Tegra Combined UART (TCU)
>>> +
>>> +The TCU is a system for sharing a hardware UART instance among multiple
>>> +systems within the Tegra SoC. It is implemented through a mailbox-
>>> +based protocol where each "virtual UART" has a pair of mailboxes, one
>>> +for transmitting and one for receiving, that is used to communicate
>>> +with the hardware implementing the TCU.
>>> +
>>> +Required properties:
>>> +- name : Should be tcu
>>> +- compatible
>>> + Array of strings
>>> + One of:
>>> + - "nvidia,tegra194-tcu"
>>> +- mbox-names:
>>> + "rx" - Mailbox for receiving data from hardware UART
>>> + "tx" - Mailbox for transmitting data to hardware UART
>>> +- mboxes: Mailboxes corresponding to the mbox-names.
>>
>> Looks like there is some trailing white-space in the above line and git
>> warns after applying this patch.
>
> I'm not seeing it. I remember seeing that trailing space in an earlier
> version of this patch and fixing it up. Did you perhaps end up applying
> the wrong version?
Yes you are right. This version does not have it. Seems that thunderbird
does not correctly overwrite the previous mbox but once I deleted and
saved again, I no longer see this.
Sorry for the noise. Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 06/10] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Thierry Reding @ 2018-11-13 10:03 UTC (permalink / raw)
To: Jon Hunter
Cc: devicetree, Greg Kroah-Hartman, Jassi Brar, Mika Liljeberg,
Mikko Perttunen, Timo Alho, linux-serial, Jiri Slaby, linux-tegra,
Pekka Pessi, linux-arm-kernel
In-Reply-To: <f7d19f6e-0dcd-753c-a407-fcec4fa1474b@nvidia.com>
[-- Attachment #1.1: Type: text/plain, Size: 2155 bytes --]
On Tue, Nov 13, 2018 at 09:39:08AM +0000, Jon Hunter wrote:
>
> On 12/11/2018 15:18, Thierry Reding wrote:
> > From: Mikko Perttunen <mperttunen@nvidia.com>
> >
> > Add bindings for the Tegra Combined UART device used to talk to the
> > UART console on Tegra194 systems.
> >
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Acked-by: Jon Hunter <jonathanh@nvidia.com>
> > Acked-by: Thierry Reding <treding@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > .../bindings/serial/nvidia,tegra194-tcu.txt | 35 +++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> > new file mode 100644
> > index 000000000000..085a8591accd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> > @@ -0,0 +1,35 @@
> > +NVIDIA Tegra Combined UART (TCU)
> > +
> > +The TCU is a system for sharing a hardware UART instance among multiple
> > +systems within the Tegra SoC. It is implemented through a mailbox-
> > +based protocol where each "virtual UART" has a pair of mailboxes, one
> > +for transmitting and one for receiving, that is used to communicate
> > +with the hardware implementing the TCU.
> > +
> > +Required properties:
> > +- name : Should be tcu
> > +- compatible
> > + Array of strings
> > + One of:
> > + - "nvidia,tegra194-tcu"
> > +- mbox-names:
> > + "rx" - Mailbox for receiving data from hardware UART
> > + "tx" - Mailbox for transmitting data to hardware UART
> > +- mboxes: Mailboxes corresponding to the mbox-names.
>
> Looks like there is some trailing white-space in the above line and git
> warns after applying this patch.
I'm not seeing it. I remember seeing that trailing space in an earlier
version of this patch and fixing it up. Did you perhaps end up applying
the wrong version?
Thierry
[-- 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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Srinivas Kandagatla @ 2018-11-13 9:44 UTC (permalink / raw)
To: saiprakash.ranjan, Steven Rostedt, Stephen Boyd
Cc: Joel Fernandes, Bjorn Andersson, Andy Gross, David Brown,
Jiri Slaby, Kees Cook, Geliang Tang, Greg Kroah-Hartman,
Pramod Gurav, linux-arm-msm, linux-soc, linux-serial,
linux-kernel, Rajendra Nayak, Vivek Gautam, Sibi Sankar
In-Reply-To: <1e6cc1fa5263b9edfcf7567d3f9f65fd@codeaurora.org>
Hi Sai,
On 25/10/18 15:36, saiprakash.ranjan@codeaurora.org wrote:
> "If I disable dma node and LS-UART0, then I don't see any crash and
> ftrace also works fine"
>
> And one more observation is that even without ftrace cmdline, if I use
> earlycon and disable dma, I face the same crash.
>
> So basically this seems to be some kind of earlycon and dma issue and
> not ftrace(I can be wrong).
>
> So adding Srinivas for more info on this dma node.
Its Interesting that my old email conversations with SBoyd show that I
have investigated this issue in early 2016!
My analysis so far:
This reason for such behavior is due the common iface clock
(GCC_BLSP1_AHB_CLK) across multiple drivers(serial ports, bam dma
and other low speed devices).
The code flow in DB410C is bit different, as the uart0 is first
attempted to set as console and then uart1, this ordering triggers
pm state change uart_change_pm(state, UART_PM_STATE_OFF) from serial
core while setting up uart0, this would go and disable all the
clocks for uart0.
As uart1 is not setup Yet, and earlycon is still active, any
attempts by earlycon to write to registers would trigger a system
reboot as the clock was just disabled by uart0 change_pm code.
This can even be triggered with any drivers like spi which uses same
clock I guess.
Hope it helps,
Either earlycon needs to reference the clocks or those clocks needs to
be marked always-on (but only with earlycon).
>
> Also just for a note: apq8096-db820c.dtsi shows UART0 is disabled because
> bootloader does not allow access to it. Could this also be the case for
> db410c?
No, this is not the case with DB410c. DB820c has added restrictions in
TZ, I think new booloaders should have solved this issue.
Thanks,
srini
^ permalink raw reply
* [PATCH][serial-next][V2] serial-uartlite: fix null pointer dereference on pointer port
From: Colin King @ 2018-11-13 9:43 UTC (permalink / raw)
To: Peter Korsgaard, Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer port is dereferenced on port->private_data when assigning pointer
pdata before port is null checked, leading to a potential null pointer
dereference. Fix this by assigning pdata after the null pointer check on
port.
Detected by CoverityScan, CID#1475434 ("Dereference before null check")
Fixes: 3b209d253e7f ("serial-uartlite: Do not use static struct uart_driver out of probe()")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/tty/serial/uartlite.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 4a7989df5ff5..b8b912b5a8b9 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -694,10 +694,11 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
static int ulite_release(struct device *dev)
{
struct uart_port *port = dev_get_drvdata(dev);
- struct uartlite_data *pdata = port->private_data;
int rc = 0;
if (port) {
+ struct uartlite_data *pdata = port->private_data;
+
rc = uart_remove_one_port(pdata->ulite_uart_driver, port);
dev_set_drvdata(dev, NULL);
port->mapbase = 0;
@@ -715,10 +716,12 @@ static int ulite_release(struct device *dev)
static int __maybe_unused ulite_suspend(struct device *dev)
{
struct uart_port *port = dev_get_drvdata(dev);
- struct uartlite_data *pdata = port->private_data;
- if (port)
+ if (port) {
+ struct uartlite_data *pdata = port->private_data;
+
uart_suspend_port(pdata->ulite_uart_driver, port);
+ }
return 0;
}
@@ -732,10 +735,12 @@ static int __maybe_unused ulite_suspend(struct device *dev)
static int __maybe_unused ulite_resume(struct device *dev)
{
struct uart_port *port = dev_get_drvdata(dev);
- struct uartlite_data *pdata = port->private_data;
- if (port)
+ if (port) {
+ struct uartlite_data *pdata = port->private_data;
+
uart_resume_port(pdata->ulite_uart_driver, port);
+ }
return 0;
}
--
2.19.1
^ permalink raw reply related
* NACK: [PATCH][serial-next] serial-uartlite: fix null pointer dereference on pointer port
From: Colin Ian King @ 2018-11-13 9:39 UTC (permalink / raw)
To: Peter Korsgaard, Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20181113093814.27598-1-colin.king@canonical.com>
On 13/11/2018 09:38, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer port is dereferenced on port->private_data when assigning pointer
> pdata before port is null checked, leading to a potential null pointer
> dereference. Fix this by assigning pdata after the null pointer check on
> port.
>
> Detected by CoverityScan, CID#1475434 ("Dereference before null check")
>
> Fixes: 3b209d253e7f ("serial-uartlite: Do not use static struct uart_driver out of probe()")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/tty/serial/uartlite.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
> index 4a7989df5ff5..0eff33cd9f24 100644
> --- a/drivers/tty/serial/uartlite.c
> +++ b/drivers/tty/serial/uartlite.c
> @@ -715,10 +715,12 @@ static int ulite_release(struct device *dev)
> static int __maybe_unused ulite_suspend(struct device *dev)
> {
> struct uart_port *port = dev_get_drvdata(dev);
> - struct uartlite_data *pdata = port->private_data;
>
> - if (port)
> + if (port) {
> + struct uartlite_data *pdata = port->private_data;
> +
> uart_suspend_port(pdata->ulite_uart_driver, port);
> + }
>
> return 0;
> }
>
Sorry for the noise, I sent the wrong fix. V2 coming soon.
^ permalink raw reply
* Re: [PATCH v2 06/10] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Jon Hunter @ 2018-11-13 9:39 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-7-thierry.reding@gmail.com>
On 12/11/2018 15:18, Thierry Reding wrote:
> From: Mikko Perttunen <mperttunen@nvidia.com>
>
> Add bindings for the Tegra Combined UART device used to talk to the
> UART console on Tegra194 systems.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> .../bindings/serial/nvidia,tegra194-tcu.txt | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> new file mode 100644
> index 000000000000..085a8591accd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> @@ -0,0 +1,35 @@
> +NVIDIA Tegra Combined UART (TCU)
> +
> +The TCU is a system for sharing a hardware UART instance among multiple
> +systems within the Tegra SoC. It is implemented through a mailbox-
> +based protocol where each "virtual UART" has a pair of mailboxes, one
> +for transmitting and one for receiving, that is used to communicate
> +with the hardware implementing the TCU.
> +
> +Required properties:
> +- name : Should be tcu
> +- compatible
> + Array of strings
> + One of:
> + - "nvidia,tegra194-tcu"
> +- mbox-names:
> + "rx" - Mailbox for receiving data from hardware UART
> + "tx" - Mailbox for transmitting data to hardware UART
> +- mboxes: Mailboxes corresponding to the mbox-names.
Looks like there is some trailing white-space in the above line and git
warns after applying this patch.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH][serial-next] serial-uartlite: fix null pointer dereference on pointer port
From: Colin King @ 2018-11-13 9:38 UTC (permalink / raw)
To: Peter Korsgaard, Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer port is dereferenced on port->private_data when assigning pointer
pdata before port is null checked, leading to a potential null pointer
dereference. Fix this by assigning pdata after the null pointer check on
port.
Detected by CoverityScan, CID#1475434 ("Dereference before null check")
Fixes: 3b209d253e7f ("serial-uartlite: Do not use static struct uart_driver out of probe()")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/tty/serial/uartlite.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 4a7989df5ff5..0eff33cd9f24 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -715,10 +715,12 @@ static int ulite_release(struct device *dev)
static int __maybe_unused ulite_suspend(struct device *dev)
{
struct uart_port *port = dev_get_drvdata(dev);
- struct uartlite_data *pdata = port->private_data;
- if (port)
+ if (port) {
+ struct uartlite_data *pdata = port->private_data;
+
uart_suspend_port(pdata->ulite_uart_driver, port);
+ }
return 0;
}
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 7/9] serial: Add Tegra Combined UART driver
From: Greg Kroah-Hartman @ 2018-11-12 18:03 UTC (permalink / raw)
To: Thierry Reding
Cc: Jassi Brar, Jiri Slaby, Mikko Perttunen, Jon Hunter, Timo Alho,
Pekka Pessi, Mika Liljeberg, linux-tegra, linux-serial,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20181112153014.GA29481@ulmo>
On Mon, Nov 12, 2018 at 04:30:14PM +0100, Thierry Reding wrote:
> On Fri, Nov 09, 2018 at 09:05:26AM -0800, Greg Kroah-Hartman wrote:
> > On Fri, Oct 26, 2018 at 01:16:36PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
> > > multiplexing multiple "virtual UARTs" into a single hardware serial
> > > port. The TCU is the primary serial port on Tegra194 devices.
> > >
> > > Add a TCU driver utilizing the mailbox framework, as the used mailboxes
> > > are part of Tegra HSP blocks that are already controlled by the Tegra
> > > HSP mailbox driver.
> > >
> > > Based on work by Mikko Perttunen <mperttunen@nvidia.com>.
> > >
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> >
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Thanks. I just sent out v2 addressing Pekka's comments on the series.
> But I'm slightly confused now. Are you expecting anyone else to pick
> this up? There are technically no build time dependencies between any
> of these patches, so it should be fine to pick them all into the
> corresponding subsystem trees.
Ah, I thought there was a built-time dependancy, that's why I gave my
ack.
> Perhaps the only thing to note is that there is a runtime dependency
> from this patch on patches 1 and 2 in the series. Patch 1 is required to
> make blocking a mailbox possible in interrupt context, which we need in
> the TCU driver because the TTY and console paths do end up calling the
> mbox_send_message() from interrupt context. Patch 2 is only required at
> runtime if the two mailboxes are provided by different instances of the
> HSP block (which they are for TCU on Tegra194).
>
> Would you prefer for Jassi to pick up the TCU patch along with the
> mailbox core and driver changes so that we deal with the runtime
> dependencies that way?
>
> Alternatively, if Jassi is okay with the mailbox changes, I can pick up
> all of the v2 series of the patches into stable branches for v4.21, deal
> with the dependencies there and send out pull requests for everyone to
> merge into their subsystem trees.
I don't really care either way, whatever is easiest for you all is fine
with me.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 7/9] serial: Add Tegra Combined UART driver
From: Thierry Reding @ 2018-11-12 15:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jassi Brar, Jiri Slaby, Mikko Perttunen, Jon Hunter, Timo Alho,
Pekka Pessi, Mika Liljeberg, linux-tegra, linux-serial,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20181109170526.GB14151@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]
On Fri, Nov 09, 2018 at 09:05:26AM -0800, Greg Kroah-Hartman wrote:
> On Fri, Oct 26, 2018 at 01:16:36PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
> > multiplexing multiple "virtual UARTs" into a single hardware serial
> > port. The TCU is the primary serial port on Tegra194 devices.
> >
> > Add a TCU driver utilizing the mailbox framework, as the used mailboxes
> > are part of Tegra HSP blocks that are already controlled by the Tegra
> > HSP mailbox driver.
> >
> > Based on work by Mikko Perttunen <mperttunen@nvidia.com>.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks. I just sent out v2 addressing Pekka's comments on the series.
But I'm slightly confused now. Are you expecting anyone else to pick
this up? There are technically no build time dependencies between any
of these patches, so it should be fine to pick them all into the
corresponding subsystem trees.
Perhaps the only thing to note is that there is a runtime dependency
from this patch on patches 1 and 2 in the series. Patch 1 is required to
make blocking a mailbox possible in interrupt context, which we need in
the TCU driver because the TTY and console paths do end up calling the
mbox_send_message() from interrupt context. Patch 2 is only required at
runtime if the two mailboxes are provided by different instances of the
HSP block (which they are for TCU on Tegra194).
Would you prefer for Jassi to pick up the TCU patch along with the
mailbox core and driver changes so that we deal with the runtime
dependencies that way?
Alternatively, if Jassi is okay with the mailbox changes, I can pick up
all of the v2 series of the patches into stable branches for v4.21, deal
with the dependencies there and send out pull requests for everyone to
merge into their subsystem trees.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 10/10] arm64: defconfig: Enable Tegra TCU
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
The Tegra Combined UART is used on some Tegra194 devices as a way of
multiplexing output from multiple producers onto a single physical UART.
Enable this by default so that it can be used as the default console to
write kernel messages to.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c0a5275210c7..fdb9d60905ac 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -300,6 +300,7 @@ CONFIG_SERIAL_MESON_CONSOLE=y
CONFIG_SERIAL_SAMSUNG=y
CONFIG_SERIAL_SAMSUNG_CONSOLE=y
CONFIG_SERIAL_TEGRA=y
+CONFIG_SERIAL_TEGRA_TCU=y
CONFIG_SERIAL_SH_SCI=y
CONFIG_SERIAL_MSM=y
CONFIG_SERIAL_MSM_CONSOLE=y
--
2.19.1
^ permalink raw reply related
* [PATCH v2 09/10] arm64: tegra: Mark TCU as primary serial port on Tegra194 P2888
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Mikko Perttunen <mperttunen@nvidia.com>
The Tegra Combined UART is the proper primary serial port on P2888,
so use it.
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
index 57d3f00464ce..fcbe2a88f8db 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
@@ -10,7 +10,7 @@
aliases {
sdhci0 = "/cbb/sdhci@3460000";
sdhci1 = "/cbb/sdhci@3400000";
- serial0 = &uartb;
+ serial0 = &tcu;
i2c0 = "/bpmp/i2c";
i2c1 = "/cbb/i2c@3160000";
i2c2 = "/cbb/i2c@c240000";
--
2.19.1
^ permalink raw reply related
* [PATCH v2 08/10] arm64: tegra: Add nodes for TCU on Tegra194
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Mikko Perttunen <mperttunen@nvidia.com>
Add nodes required for communication through the Tegra Combined UART.
This includes the AON HSP instance, addition of shared interrupts
for the TOP0 HSP instance, and finally the TCU node itself. Also
mark the HSP instances as compatible to tegra194-hsp, as the hardware
is not identical but is compatible to tegra186-hsp.
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- encode direction of mailboxes in device tree mailbox specifier
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 38 ++++++++++++++++++++++--
1 file changed, 35 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index c2091bb16546..4451532a2b4c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -340,10 +340,35 @@
};
hsp_top0: hsp@3c00000 {
- compatible = "nvidia,tegra186-hsp";
+ compatible = "nvidia,tegra194-hsp", "nvidia,tegra186-hsp";
reg = <0x03c00000 0xa0000>;
- interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "doorbell";
+ interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "doorbell", "shared0", "shared1", "shared2",
+ "shared3", "shared4", "shared5", "shared6",
+ "shared7";
+ #mbox-cells = <2>;
+ };
+
+ hsp_aon: hsp@c150000 {
+ compatible = "nvidia,tegra194-hsp", "nvidia,tegra186-hsp";
+ reg = <0x0c150000 0xa0000>;
+ interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+ /*
+ * Shared interrupt 0 is routed only to AON/SPE, so
+ * we only have 4 shared interrupts for the CCPLEX.
+ */
+ interrupt-names = "shared1", "shared2", "shared3", "shared4";
#mbox-cells = <2>;
};
@@ -531,6 +556,13 @@
method = "smc";
};
+ tcu: tcu {
+ compatible = "nvidia,tegra194-tcu";
+ mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_SM TEGRA_HSP_SM_RX(0)>,
+ <&hsp_aon TEGRA_HSP_MBOX_TYPE_SM TEGRA_HSP_SM_TX(1)>;
+ mbox-names = "rx", "tx";
+ };
+
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 13
--
2.19.1
^ permalink raw reply related
* [PATCH v2 07/10] serial: Add Tegra Combined UART driver
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
multiplexing multiple "virtual UARTs" into a single hardware serial
port. The TCU is the primary serial port on Tegra194 devices.
Add a TCU driver utilizing the mailbox framework, as the used mailboxes
are part of Tegra HSP blocks that are already controlled by the Tegra
HSP mailbox driver.
Based on work by Mikko Perttunen <mperttunen@nvidia.com>.
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/tty/serial/Kconfig | 22 +++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/tegra-tcu.c | 299 +++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
4 files changed, 325 insertions(+)
create mode 100644 drivers/tty/serial/tegra-tcu.c
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 32886c304641..785306388aa4 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -323,6 +323,28 @@ config SERIAL_TEGRA
are enabled). This driver uses the APB DMA to achieve higher baudrate
and better performance.
+config SERIAL_TEGRA_TCU
+ tristate "NVIDIA Tegra Combined UART"
+ depends on ARCH_TEGRA && TEGRA_HSP_MBOX
+ select SERIAL_CORE
+ help
+ Support for the mailbox-based TCU (Tegra Combined UART) serial port.
+ TCU is a virtual serial port that allows multiplexing multiple data
+ streams into a single hardware serial port.
+
+config SERIAL_TEGRA_TCU_CONSOLE
+ bool "Support for console on a Tegra TCU serial port"
+ depends on SERIAL_TEGRA_TCU=y
+ select SERIAL_CORE_CONSOLE
+ default y
+ ---help---
+ If you say Y here, it will be possible to use a the Tegra TCU as the
+ system console (the system console is the device which receives all
+ kernel messages and warnings and which allows logins in single user
+ mode).
+
+ If unsure, say Y.
+
config SERIAL_MAX3100
tristate "MAX3100 support"
depends on SPI
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675612df..4ad82231ff8a 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_SERIAL_LANTIQ) += lantiq.o
obj-$(CONFIG_SERIAL_XILINX_PS_UART) += xilinx_uartps.o
obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o
obj-$(CONFIG_SERIAL_TEGRA) += serial-tegra.o
+obj-$(CONFIG_SERIAL_TEGRA_TCU) += tegra-tcu.o
obj-$(CONFIG_SERIAL_AR933X) += ar933x_uart.o
obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
obj-$(CONFIG_SERIAL_ARC) += arc_uart.o
diff --git a/drivers/tty/serial/tegra-tcu.c b/drivers/tty/serial/tegra-tcu.c
new file mode 100644
index 000000000000..1d360cd03b18
--- /dev/null
+++ b/drivers/tty/serial/tegra-tcu.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ */
+
+#include <linux/console.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+#define TCU_MBOX_BYTE(i, x) ((x) << (i * 8))
+#define TCU_MBOX_BYTE_V(x, i) (((x) >> (i * 8)) & 0xff)
+#define TCU_MBOX_NUM_BYTES(x) ((x) << 24)
+#define TCU_MBOX_NUM_BYTES_V(x) (((x) >> 24) & 0x3)
+
+struct tegra_tcu {
+ struct uart_driver driver;
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+ struct console console;
+#endif
+ struct uart_port port;
+
+ struct mbox_client tx_client, rx_client;
+ struct mbox_chan *tx, *rx;
+};
+
+static unsigned int tegra_tcu_uart_tx_empty(struct uart_port *port)
+{
+ return TIOCSER_TEMT;
+}
+
+static void tegra_tcu_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}
+
+static unsigned int tegra_tcu_uart_get_mctrl(struct uart_port *port)
+{
+ return 0;
+}
+
+static void tegra_tcu_uart_stop_tx(struct uart_port *port)
+{
+}
+
+static void tegra_tcu_write_one(struct tegra_tcu *tcu, u32 value,
+ unsigned int count)
+{
+ void *msg;
+
+ value |= TCU_MBOX_NUM_BYTES(count);
+ msg = (void *)(unsigned long)value;
+ mbox_send_message(tcu->tx, msg);
+}
+
+static void tegra_tcu_write(struct tegra_tcu *tcu, const char *s,
+ unsigned int count)
+{
+ unsigned int written = 0, i = 0;
+ bool insert_nl = false;
+ u32 value = 0;
+
+ while (i < count) {
+ if (insert_nl) {
+ value |= TCU_MBOX_BYTE(written++, '\n');
+ insert_nl = false;
+ i++;
+ } else if (s[i] == '\n') {
+ value |= TCU_MBOX_BYTE(written++, '\r');
+ insert_nl = true;
+ } else {
+ value |= TCU_MBOX_BYTE(written++, s[i++]);
+ }
+
+ if (written == 3) {
+ tegra_tcu_write_one(tcu, value, 3);
+ value = written = 0;
+ }
+ }
+
+ if (written)
+ tegra_tcu_write_one(tcu, value, written);
+}
+
+static void tegra_tcu_uart_start_tx(struct uart_port *port)
+{
+ struct tegra_tcu *tcu = port->private_data;
+ struct circ_buf *xmit = &port->state->xmit;
+ unsigned long count;
+
+ for (;;) {
+ count = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
+ if (!count)
+ break;
+
+ tegra_tcu_write(tcu, &xmit->buf[xmit->tail], count);
+ xmit->tail = (xmit->tail + count) & (UART_XMIT_SIZE - 1);
+ }
+
+ uart_write_wakeup(port);
+}
+
+static void tegra_tcu_uart_stop_rx(struct uart_port *port)
+{
+}
+
+static void tegra_tcu_uart_break_ctl(struct uart_port *port, int ctl)
+{
+}
+
+static int tegra_tcu_uart_startup(struct uart_port *port)
+{
+ return 0;
+}
+
+static void tegra_tcu_uart_shutdown(struct uart_port *port)
+{
+}
+
+static void tegra_tcu_uart_set_termios(struct uart_port *port,
+ struct ktermios *new,
+ struct ktermios *old)
+{
+}
+
+static const struct uart_ops tegra_tcu_uart_ops = {
+ .tx_empty = tegra_tcu_uart_tx_empty,
+ .set_mctrl = tegra_tcu_uart_set_mctrl,
+ .get_mctrl = tegra_tcu_uart_get_mctrl,
+ .stop_tx = tegra_tcu_uart_stop_tx,
+ .start_tx = tegra_tcu_uart_start_tx,
+ .stop_rx = tegra_tcu_uart_stop_rx,
+ .break_ctl = tegra_tcu_uart_break_ctl,
+ .startup = tegra_tcu_uart_startup,
+ .shutdown = tegra_tcu_uart_shutdown,
+ .set_termios = tegra_tcu_uart_set_termios,
+};
+
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+static void tegra_tcu_console_write(struct console *cons, const char *s,
+ unsigned int count)
+{
+ struct tegra_tcu *tcu = container_of(cons, struct tegra_tcu, console);
+
+ tegra_tcu_write(tcu, s, count);
+}
+
+static int tegra_tcu_console_setup(struct console *cons, char *options)
+{
+ return 0;
+}
+#endif
+
+static void tegra_tcu_receive(struct mbox_client *cl, void *msg)
+{
+ struct tegra_tcu *tcu = container_of(cl, struct tegra_tcu, rx_client);
+ struct tty_port *port = &tcu->port.state->port;
+ u32 value = (u32)(unsigned long)msg;
+ unsigned int num_bytes, i;
+
+ num_bytes = TCU_MBOX_NUM_BYTES_V(value);
+
+ for (i = 0; i < num_bytes; i++)
+ tty_insert_flip_char(port, TCU_MBOX_BYTE_V(value, i),
+ TTY_NORMAL);
+
+ tty_flip_buffer_push(port);
+}
+
+static int tegra_tcu_probe(struct platform_device *pdev)
+{
+ struct uart_port *port;
+ struct tegra_tcu *tcu;
+ int err;
+
+ tcu = devm_kzalloc(&pdev->dev, sizeof(*tcu), GFP_KERNEL);
+ if (!tcu)
+ 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");
+ if (IS_ERR(tcu->tx)) {
+ err = PTR_ERR(tcu->tx);
+ dev_err(&pdev->dev, "failed to get tx mailbox: %d\n", err);
+ return err;
+ }
+
+ tcu->rx = mbox_request_channel_byname(&tcu->rx_client, "rx");
+ if (IS_ERR(tcu->rx)) {
+ err = PTR_ERR(tcu->rx);
+ dev_err(&pdev->dev, "failed to get rx mailbox: %d\n", err);
+ goto free_tx;
+ }
+
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+ /* setup the console */
+ strcpy(tcu->console.name, "ttyTCU");
+ tcu->console.device = uart_console_device;
+ tcu->console.flags = CON_PRINTBUFFER | CON_ANYTIME;
+ tcu->console.index = -1;
+ tcu->console.write = tegra_tcu_console_write;
+ tcu->console.setup = tegra_tcu_console_setup;
+ tcu->console.data = &tcu->driver;
+#endif
+
+ /* setup the driver */
+ tcu->driver.owner = THIS_MODULE;
+ tcu->driver.driver_name = "tegra-tcu";
+ tcu->driver.dev_name = "ttyTCU";
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+ tcu->driver.cons = &tcu->console;
+#endif
+ tcu->driver.nr = 1;
+
+ err = uart_register_driver(&tcu->driver);
+ if (err) {
+ dev_err(&pdev->dev, "failed to register UART driver: %d\n",
+ err);
+ goto free_rx;
+ }
+
+ /* setup the port */
+ port = &tcu->port;
+ spin_lock_init(&port->lock);
+ port->dev = &pdev->dev;
+ port->type = PORT_TEGRA_TCU;
+ port->ops = &tegra_tcu_uart_ops;
+ port->fifosize = 1;
+ port->iotype = UPIO_MEM;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->private_data = tcu;
+
+ err = uart_add_one_port(&tcu->driver, port);
+ if (err) {
+ dev_err(&pdev->dev, "failed to add UART port: %d\n", err);
+ goto unregister_uart;
+ }
+
+ platform_set_drvdata(pdev, tcu);
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+ register_console(&tcu->console);
+#endif
+
+ return 0;
+
+unregister_uart:
+ uart_unregister_driver(&tcu->driver);
+free_rx:
+ mbox_free_channel(tcu->rx);
+free_tx:
+ mbox_free_channel(tcu->tx);
+
+ return err;
+}
+
+static int tegra_tcu_remove(struct platform_device *pdev)
+{
+ struct tegra_tcu *tcu = platform_get_drvdata(pdev);
+
+#if IS_ENABLED(CONFIG_SERIAL_TEGRA_TCU_CONSOLE)
+ unregister_console(&tcu->console);
+#endif
+ uart_remove_one_port(&tcu->driver, &tcu->port);
+ uart_unregister_driver(&tcu->driver);
+ mbox_free_channel(tcu->rx);
+ mbox_free_channel(tcu->tx);
+
+ return 0;
+}
+
+static const struct of_device_id tegra_tcu_match[] = {
+ { .compatible = "nvidia,tegra194-tcu" },
+ { }
+};
+
+static struct platform_driver tegra_tcu_driver = {
+ .driver = {
+ .name = "tegra-tcu",
+ .of_match_table = tegra_tcu_match,
+ },
+ .probe = tegra_tcu_probe,
+ .remove = tegra_tcu_remove,
+};
+module_platform_driver(tegra_tcu_driver);
+
+MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("NVIDIA Tegra Combined UART driver");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index dce5f9dae121..69883c32cb98 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -79,6 +79,9 @@
/* Nuvoton UART */
#define PORT_NPCM 40
+/* NVIDIA Tegra Combined UART */
+#define PORT_TEGRA_TCU 41
+
/* Intel EG20 */
#define PORT_PCH_8LINE 44
#define PORT_PCH_2LINE 45
--
2.19.1
^ permalink raw reply related
* [PATCH v2 06/10] dt-bindings: serial: Add bindings for nvidia, tegra194-tcu
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Mikko Perttunen <mperttunen@nvidia.com>
Add bindings for the Tegra Combined UART device used to talk to the
UART console on Tegra194 systems.
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
.../bindings/serial/nvidia,tegra194-tcu.txt | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
new file mode 100644
index 000000000000..085a8591accd
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
@@ -0,0 +1,35 @@
+NVIDIA Tegra Combined UART (TCU)
+
+The TCU is a system for sharing a hardware UART instance among multiple
+systems within the Tegra SoC. It is implemented through a mailbox-
+based protocol where each "virtual UART" has a pair of mailboxes, one
+for transmitting and one for receiving, that is used to communicate
+with the hardware implementing the TCU.
+
+Required properties:
+- name : Should be tcu
+- compatible
+ Array of strings
+ One of:
+ - "nvidia,tegra194-tcu"
+- mbox-names:
+ "rx" - Mailbox for receiving data from hardware UART
+ "tx" - Mailbox for transmitting data to hardware UART
+- mboxes: Mailboxes corresponding to the mbox-names.
+
+This node is a mailbox consumer. See the following files for details of
+the mailbox subsystem, and the specifiers implemented by the relevant
+provider(s):
+
+- .../mailbox/mailbox.txt
+- .../mailbox/nvidia,tegra186-hsp.txt
+
+Example bindings:
+-----------------
+
+tcu: tcu {
+ compatible = "nvidia,tegra194-tcu";
+ mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_SM 0>,
+ <&hsp_aon TEGRA_HSP_MBOX_TYPE_SM 1>;
+ mbox-names = "rx", "tx";
+};
--
2.19.1
^ permalink raw reply related
* [PATCH v2 05/10] mailbox: tegra-hsp: Add suspend/resume support
From: Thierry Reding @ 2018-11-12 15:18 UTC (permalink / raw)
To: Thierry Reding, Jassi Brar, Greg Kroah-Hartman
Cc: devicetree, Mika Liljeberg, Mikko Perttunen, Timo Alho,
linux-serial, Jiri Slaby, linux-tegra, Pekka Pessi, Jon Hunter,
linux-arm-kernel
In-Reply-To: <20181112151853.29289-1-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
Upon resuming from a system sleep state, the interrupts for all active
shared mailboxes need to be reenabled, otherwise they will not work.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/mailbox/tegra-hsp.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index 0100a974149b..1259abf3542f 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -18,6 +18,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <linux/slab.h>
#include <dt-bindings/mailbox/tegra186-hsp.h>
@@ -817,6 +818,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
return 0;
}
+static int tegra_hsp_resume(struct device *dev)
+{
+ struct tegra_hsp *hsp = dev_get_drvdata(dev);
+ unsigned int i;
+
+ for (i = 0; i < hsp->num_sm; i++) {
+ struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
+
+ if (mb->channel.chan->cl)
+ tegra_hsp_mailbox_startup(mb->channel.chan);
+ }
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
+
static const struct tegra_hsp_db_map tegra186_hsp_db_map[] = {
{ "ccplex", TEGRA_HSP_DB_MASTER_CCPLEX, HSP_DB_CCPLEX, },
{ "bpmp", TEGRA_HSP_DB_MASTER_BPMP, HSP_DB_BPMP, },
@@ -843,6 +861,7 @@ static struct platform_driver tegra_hsp_driver = {
.driver = {
.name = "tegra-hsp",
.of_match_table = tegra_hsp_match,
+ .pm = &tegra_hsp_pm_ops,
},
.probe = tegra_hsp_probe,
.remove = tegra_hsp_remove,
--
2.19.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox