| Message ID | 20190131003933.11436-6-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Qualcomm AOSS QMP driver and modem dts | expand |
Hi, On Wed, Jan 30, 2019 at 4:40 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > +++ b/drivers/soc/qcom/aoss-qmp.c > @@ -0,0 +1,317 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, Linaro Ltd > + */ > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/mailbox_client.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/soc/qcom/aoss-qmp.h> > + > +#define QMP_DESC_MAGIC 0x0 > +#define QMP_DESC_VERSION 0x4 > +#define QMP_DESC_FEATURES 0x8 > + > +#define QMP_DESC_UCORE_LINK_STATE 0xc > +#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10 > +#define QMP_DESC_UCORE_CH_STATE 0x14 > +#define QMP_DESC_UCORE_CH_STATE_ACK 0x18 > +#define QMP_DESC_UCORE_MBOX_SIZE 0x1c > +#define QMP_DESC_UCORE_MBOX_OFFSET 0x20 > + > +#define QMP_DESC_MCORE_LINK_STATE 0x24 > +#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28 > +#define QMP_DESC_MCORE_CH_STATE 0x2c > +#define QMP_DESC_MCORE_CH_STATE_ACK 0x30 > +#define QMP_DESC_MCORE_MBOX_SIZE 0x34 > +#define QMP_DESC_MCORE_MBOX_OFFSET 0x38 I sure wish something in this file told me what a mcore and a ucore were. The only thing I can think of is that an "m" core is two "u" cores flipped upside down and placed really close to each other. ...if we had 6 upside down "u" cores we'd have an "mmm" core. Mmm, core. > +static int qmp_open(struct qmp *qmp) > +{ > + int ret; > + u32 val; > + > + ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ); I'm a totally noob here, but I'm curious: what kicks this event? Do we just assume that an IRQ is pending already when the probe() function is called? Maybe you could add a comment? ...or maybe you never actually get an IRQ here and just rely on the magic value being right at boot in which case we should just check qmp_magic_valid() ...or maybe you never actually get an IRQ here and this is equivalent to msleep(1000) followed by a check of qmp_magic_valid()? > + if (!ret) { > + dev_err(qmp->dev, "QMP magic doesn't match\n"); > + return -ETIMEDOUT; > + } > + > + val = readl(qmp->msgram + QMP_DESC_VERSION); > + if (val != QMP_VERSION) { > + dev_err(qmp->dev, "unsupported QMP version %d\n", val); > + return -EINVAL; > + } > + > + qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET); > + qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE); > + if (!qmp->size) { > + dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size); nitty nit: Can you do "%#zx" to avoid the need for the 0x? > + return -EINVAL; > + } > + > + /* Ack remote core's link state */ > + val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE); > + writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK); > + > + /* Set local core's link state to up */ > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); > + > + qmp_kick(qmp); > + > + ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "ucore didn't ack link\n"); > + goto timeout_close_link; > + } > + > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE); > + > + ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ); Again maybe a noob question, but what kicks the interrupt here? Is the other side looping waiting to see us write "QMP_STATE_UP" into "QMP_DESC_MCORE_CH_STATE" and then it sends us another interrupt? ...or are we just getting lucky that the condition is right to begin with? > + if (!ret) { > + dev_err(qmp->dev, "ucore didn't open channel\n"); > + goto timeout_close_channel; > + } > + > + /* Ack remote core's channel state */ > + val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE); > + writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK); nit: the readl() is silly here. Just before this you called qmp_ucore_channel_up() and that confirmed that the value you're getting here is exactly equal to "QMP_STATE_UP". Just write that. > +static int qmp_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct qmp *qmp; > + int irq; > + int ret; > + > + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); > + if (!qmp) > + return -ENOMEM; > + > + qmp->dev = &pdev->dev; > + init_waitqueue_head(&qmp->event); > + mutex_init(&qmp->tx_lock); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(qmp->msgram)) > + return PTR_ERR(qmp->msgram); > + > + qmp->mbox_client.dev = &pdev->dev; > + qmp->mbox_client.knows_txdone = true; > + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); nit: your code would be simplified a bit if you created devm_mbox_request_channel() in a prior patch. > + if (IS_ERR(qmp->mbox_chan)) { > + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); > + return PTR_ERR(qmp->mbox_chan); > + } > + > + irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, > + "aoss-qmp", qmp); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request interrupt\n"); > + mbox_free_channel(qmp->mbox_chan); > + return ret; > + } > + > + ret = qmp_open(qmp); > + if (ret < 0) { > + mbox_free_channel(qmp->mbox_chan); > + return ret; > + } > + > + platform_set_drvdata(pdev, qmp); > + > + if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) { > + qmp->pd_pdev = platform_device_register_data(&pdev->dev, > + "aoss_qmp_pd", > + PLATFORM_DEVID_NONE, > + NULL, 0); > + if (IS_ERR(qmp->pd_pdev)) > + dev_err(&pdev->dev, "failed to register AOSS PD\n"); nit: I'd prefer dev_warn() for serious but non-fatal errors. This appears to be non-fatal since it doesn't cause you to return an error. ...ideally the error message should indicate that the error is being ignored. I'm also not 100% sure why the "aoss_qmp_pd" needs to be broken up as a separate driver. I guess there is expectation that there will be more sub-drivers that use qmp_send() and that stuffing them all in the same driver would be too much? It sure seems like your life would be simplified if they were just one driver though unless you think someone would want to enable "AOSS_QMP" without enabling "AOSS_QMP_PD". > +static int qmp_remove(struct platform_device *pdev) > +{ > + struct qmp *qmp = platform_get_drvdata(pdev); > + > + platform_device_unregister(qmp->pd_pdev); Presumably the above should be prefixed with: if (!IS_ERR(qmp->pd_pdev)) ...since it appears that the probe will return with no error if you fail to register the pd_pdev and thus you need to handle it being an error in remove. > + mbox_free_channel(qmp->mbox_chan); > + qmp_close(qmp); nit: I always expect that remove should be in the opposite order of probe. That means qmp_close() should be before mbox_free_channel().
On Fri 01 Feb 15:36 PST 2019, Doug Anderson wrote: > Hi, > > On Wed, Jan 30, 2019 at 4:40 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > +++ b/drivers/soc/qcom/aoss-qmp.c > > @@ -0,0 +1,317 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018, Linaro Ltd > > + */ > > +#include <linux/interrupt.h> > > +#include <linux/io.h> > > +#include <linux/mailbox_client.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/soc/qcom/aoss-qmp.h> > > + > > +#define QMP_DESC_MAGIC 0x0 > > +#define QMP_DESC_VERSION 0x4 > > +#define QMP_DESC_FEATURES 0x8 > > + > > +#define QMP_DESC_UCORE_LINK_STATE 0xc > > +#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10 > > +#define QMP_DESC_UCORE_CH_STATE 0x14 > > +#define QMP_DESC_UCORE_CH_STATE_ACK 0x18 > > +#define QMP_DESC_UCORE_MBOX_SIZE 0x1c > > +#define QMP_DESC_UCORE_MBOX_OFFSET 0x20 > > + > > +#define QMP_DESC_MCORE_LINK_STATE 0x24 > > +#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28 > > +#define QMP_DESC_MCORE_CH_STATE 0x2c > > +#define QMP_DESC_MCORE_CH_STATE_ACK 0x30 > > +#define QMP_DESC_MCORE_MBOX_SIZE 0x34 > > +#define QMP_DESC_MCORE_MBOX_OFFSET 0x38 > > I sure wish something in this file told me what a mcore and a ucore > were. The only thing I can think of is that an "m" core is two "u" > cores flipped upside down and placed really close to each other. > ...if we had 6 upside down "u" cores we'd have an "mmm" core. Mmm, > core. > I had to look at the code again to figure out which side was which, so I'll add a comment here to indicate which is which. > > > +static int qmp_open(struct qmp *qmp) > > +{ > > + int ret; > > + u32 val; > > + > > + ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ); > > I'm a totally noob here, but I'm curious: what kicks this event? Do > we just assume that an IRQ is pending already when the probe() > function is called? Maybe you could add a comment? > > ...or maybe you never actually get an IRQ here and just rely on the > magic value being right at boot in which case we should just check > qmp_magic_valid() > > ...or maybe you never actually get an IRQ here and this is equivalent > to msleep(1000) followed by a check of qmp_magic_valid()? > This must be me misinterpreting the downstream driver, the magic is already in place when we enter here. > > > + if (!ret) { > > + dev_err(qmp->dev, "QMP magic doesn't match\n"); > > + return -ETIMEDOUT; > > + } > > + > > + val = readl(qmp->msgram + QMP_DESC_VERSION); > > + if (val != QMP_VERSION) { > > + dev_err(qmp->dev, "unsupported QMP version %d\n", val); > > + return -EINVAL; > > + } > > + > > + qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET); > > + qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE); > > + if (!qmp->size) { > > + dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size); > > nitty nit: Can you do "%#zx" to avoid the need for the 0x? > Didn't know I could do that, but that said this is conditional on !qmp->size, so I'm dropping the 0x0 from the error... > > > + return -EINVAL; > > + } > > + > > + /* Ack remote core's link state */ > > + val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE); > > + writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK); > > + > > + /* Set local core's link state to up */ > > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); > > + > > + qmp_kick(qmp); > > + > > + ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ); > > + if (!ret) { > > + dev_err(qmp->dev, "ucore didn't ack link\n"); > > + goto timeout_close_link; > > + } > > + > > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE); > > + > > + ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ); > > Again maybe a noob question, but what kicks the interrupt here? Is > the other side looping waiting to see us write "QMP_STATE_UP" into > "QMP_DESC_MCORE_CH_STATE" and then it sends us another interrupt? > ...or are we just getting lucky that the condition is right to begin > with? > I guess it does, but I think there is a kick inbetween here in the downstream driver, I'll add one for good measure. > > > + if (!ret) { > > + dev_err(qmp->dev, "ucore didn't open channel\n"); > > + goto timeout_close_channel; > > + } > > + > > + /* Ack remote core's channel state */ > > + val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE); > > + writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK); > > nit: the readl() is silly here. Just before this you called > qmp_ucore_channel_up() and that confirmed that the value you're > getting here is exactly equal to "QMP_STATE_UP". Just write that. > Right > > > +static int qmp_probe(struct platform_device *pdev) > > +{ > > + struct resource *res; > > + struct qmp *qmp; > > + int irq; > > + int ret; > > + > > + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); > > + if (!qmp) > > + return -ENOMEM; > > + > > + qmp->dev = &pdev->dev; > > + init_waitqueue_head(&qmp->event); > > + mutex_init(&qmp->tx_lock); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(qmp->msgram)) > > + return PTR_ERR(qmp->msgram); > > + > > + qmp->mbox_client.dev = &pdev->dev; > > + qmp->mbox_client.knows_txdone = true; > > + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); > > nit: your code would be simplified a bit if you created > devm_mbox_request_channel() in a prior patch. > > > > + if (IS_ERR(qmp->mbox_chan)) { > > + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); > > + return PTR_ERR(qmp->mbox_chan); > > + } > > + > > + irq = platform_get_irq(pdev, 0); > > + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, > > + "aoss-qmp", qmp); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "failed to request interrupt\n"); > > + mbox_free_channel(qmp->mbox_chan); > > + return ret; > > + } > > + > > + ret = qmp_open(qmp); > > + if (ret < 0) { > > + mbox_free_channel(qmp->mbox_chan); > > + return ret; > > + } > > + > > + platform_set_drvdata(pdev, qmp); > > + > > + if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) { > > + qmp->pd_pdev = platform_device_register_data(&pdev->dev, > > + "aoss_qmp_pd", > > + PLATFORM_DEVID_NONE, > > + NULL, 0); > > + if (IS_ERR(qmp->pd_pdev)) > > + dev_err(&pdev->dev, "failed to register AOSS PD\n"); > > nit: I'd prefer dev_warn() for serious but non-fatal errors. This > appears to be non-fatal since it doesn't cause you to return an error. > > ...ideally the error message should indicate that the error is being ignored. > I think it makes more sense to keep this as dev_err() and actually fail probe on this. Will update. > > I'm also not 100% sure why the "aoss_qmp_pd" needs to be broken up as > a separate driver. I guess there is expectation that there will be > more sub-drivers that use qmp_send() and that stuffing them all in the > same driver would be too much? It sure seems like your life would be > simplified if they were just one driver though unless you think > someone would want to enable "AOSS_QMP" without enabling > "AOSS_QMP_PD". > I think splitting them in different files makes a lot of sense, whether they should be separate drivers or just linked to one chunk that's food for thought. > > > +static int qmp_remove(struct platform_device *pdev) > > +{ > > + struct qmp *qmp = platform_get_drvdata(pdev); > > + > > + platform_device_unregister(qmp->pd_pdev); > > Presumably the above should be prefixed with: > > if (!IS_ERR(qmp->pd_pdev)) > > ...since it appears that the probe will return with no error if you > fail to register the pd_pdev and thus you need to handle it being an > error in remove. > Let's just make sure this doesn't happen. > > > + mbox_free_channel(qmp->mbox_chan); > > + qmp_close(qmp); > > nit: I always expect that remove should be in the opposite order of > probe. That means qmp_close() should be before mbox_free_channel(). You're right. Thanks, Bjorn
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 1ee298f6bf17..28ab19bf8c98 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -3,6 +3,15 @@ # menu "Qualcomm SoC drivers" +config QCOM_AOSS_QMP + tristate "Qualcomm AOSS Messaging Driver" + depends on ARCH_QCOM || COMPILE_TEST + depends on MAILBOX + help + This driver provides the means for communicating with the + micro-controller in the AOSS, using QMP, to control certain resource + that are not exposed through RPMh. + config QCOM_COMMAND_DB bool "Qualcomm Command DB" depends on ARCH_QCOM || COMPILE_TEST diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index ffe519b0cb66..2c04d27fbf9e 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS_rpmh-rsc.o := -I$(src) +obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c new file mode 100644 index 000000000000..86ee622cdadf --- /dev/null +++ b/drivers/soc/qcom/aoss-qmp.c @@ -0,0 +1,317 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, Linaro Ltd + */ +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/mailbox_client.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/soc/qcom/aoss-qmp.h> + +#define QMP_DESC_MAGIC 0x0 +#define QMP_DESC_VERSION 0x4 +#define QMP_DESC_FEATURES 0x8 + +#define QMP_DESC_UCORE_LINK_STATE 0xc +#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10 +#define QMP_DESC_UCORE_CH_STATE 0x14 +#define QMP_DESC_UCORE_CH_STATE_ACK 0x18 +#define QMP_DESC_UCORE_MBOX_SIZE 0x1c +#define QMP_DESC_UCORE_MBOX_OFFSET 0x20 + +#define QMP_DESC_MCORE_LINK_STATE 0x24 +#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28 +#define QMP_DESC_MCORE_CH_STATE 0x2c +#define QMP_DESC_MCORE_CH_STATE_ACK 0x30 +#define QMP_DESC_MCORE_MBOX_SIZE 0x34 +#define QMP_DESC_MCORE_MBOX_OFFSET 0x38 + +#define QMP_STATE_UP 0x0000ffff +#define QMP_STATE_DOWN 0xffff0000 + +#define QMP_MAGIC 0x4d41494c +#define QMP_VERSION 1 + +/** + * struct qmp - driver state for QMP implementation + * @msgram: iomem referencing the message RAM used for communication + * @dev: reference to QMP device + * @mbox_client: mailbox client used to ring the doorbell on transmit + * @mbox_chan: mailbox channel used to ring the doorbell on transmit + * @offset: offset within @msgram where messages should be written + * @size: maximum size of the messages to be transmitted + * @event: wait_queue for synchronization with the IRQ + * @tx_lock: provides syncrhonization between multiple callers of qmp_send() + * @pd_pdev: platform device for the power-domain child device + */ +struct qmp { + void __iomem *msgram; + struct device *dev; + + struct mbox_client mbox_client; + struct mbox_chan *mbox_chan; + + size_t offset; + size_t size; + + wait_queue_head_t event; + + struct mutex tx_lock; + + struct platform_device *pd_pdev; +}; + +static void qmp_kick(struct qmp *qmp) +{ + mbox_send_message(qmp->mbox_chan, NULL); + mbox_client_txdone(qmp->mbox_chan, 0); +} + +static bool qmp_magic_valid(struct qmp *qmp) +{ + return readl(qmp->msgram + QMP_DESC_MAGIC) == QMP_MAGIC; +} + +static bool qmp_link_acked(struct qmp *qmp) +{ + return readl(qmp->msgram + QMP_DESC_MCORE_LINK_STATE_ACK) == QMP_STATE_UP; +} + +static bool qmp_mcore_channel_acked(struct qmp *qmp) +{ + return readl(qmp->msgram + QMP_DESC_MCORE_CH_STATE_ACK) == QMP_STATE_UP; +} + +static bool qmp_ucore_channel_up(struct qmp *qmp) +{ + return readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE) == QMP_STATE_UP; +} + +static int qmp_open(struct qmp *qmp) +{ + int ret; + u32 val; + + ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ); + if (!ret) { + dev_err(qmp->dev, "QMP magic doesn't match\n"); + return -ETIMEDOUT; + } + + val = readl(qmp->msgram + QMP_DESC_VERSION); + if (val != QMP_VERSION) { + dev_err(qmp->dev, "unsupported QMP version %d\n", val); + return -EINVAL; + } + + qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET); + qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE); + if (!qmp->size) { + dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size); + return -EINVAL; + } + + /* Ack remote core's link state */ + val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE); + writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK); + + /* Set local core's link state to up */ + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); + + qmp_kick(qmp); + + ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ); + if (!ret) { + dev_err(qmp->dev, "ucore didn't ack link\n"); + goto timeout_close_link; + } + + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE); + + ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ); + if (!ret) { + dev_err(qmp->dev, "ucore didn't open channel\n"); + goto timeout_close_channel; + } + + /* Ack remote core's channel state */ + val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE); + writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK); + + qmp_kick(qmp); + + ret = wait_event_timeout(qmp->event, qmp_mcore_channel_acked(qmp), HZ); + if (!ret) { + dev_err(qmp->dev, "ucore didn't ack channel\n"); + goto timeout_close_channel; + } + + return 0; + +timeout_close_channel: + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE); + +timeout_close_link: + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); + qmp_kick(qmp); + + return -ETIMEDOUT; +} + +static void qmp_close(struct qmp *qmp) +{ + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE); + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); + qmp_kick(qmp); +} + +static irqreturn_t qmp_intr(int irq, void *data) +{ + struct qmp *qmp = data; + + wake_up_interruptible_all(&qmp->event); + + return IRQ_HANDLED; +} + +static bool qmp_message_empty(struct qmp *qmp) +{ + return readl(qmp->msgram + qmp->offset) == 0; +} + +/** + * qmp_send() - send a message to the AOSS + * @qmp: qmp context + * @data: message to be sent + * @len: length of the message + * + * Transmit @data to AOSS and wait for the AOSS to acknowledge the message. + * @len must be a multiple of 4 and not longer than the mailbox size. Access is + * synchronized by this implementation. + * + * Return: 0 on success, negative errno on failure + */ +int qmp_send(struct qmp *qmp, const void *data, size_t len) +{ + int ret; + + if (WARN_ON(len + sizeof(u32) > qmp->size)) + return -EINVAL; + + if (WARN_ON(len % sizeof(u32))) + return -EINVAL; + + mutex_lock(&qmp->tx_lock); + + /* The message RAM only implements 32-bit accesses */ + __iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32), + data, len / sizeof(u32)); + writel(len, qmp->msgram + qmp->offset); + qmp_kick(qmp); + + ret = wait_event_interruptible_timeout(qmp->event, + qmp_message_empty(qmp), HZ); + if (!ret) { + dev_err(qmp->dev, "ucore did not ack channel\n"); + ret = -ETIMEDOUT; + + /* Clear message from buffer */ + writel(0, qmp->msgram + qmp->offset); + } else { + ret = 0; + } + + mutex_unlock(&qmp->tx_lock); + + return ret; +} +EXPORT_SYMBOL(qmp_send); + +static int qmp_probe(struct platform_device *pdev) +{ + struct resource *res; + struct qmp *qmp; + int irq; + int ret; + + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); + if (!qmp) + return -ENOMEM; + + qmp->dev = &pdev->dev; + init_waitqueue_head(&qmp->event); + mutex_init(&qmp->tx_lock); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(qmp->msgram)) + return PTR_ERR(qmp->msgram); + + qmp->mbox_client.dev = &pdev->dev; + qmp->mbox_client.knows_txdone = true; + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); + if (IS_ERR(qmp->mbox_chan)) { + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); + return PTR_ERR(qmp->mbox_chan); + } + + irq = platform_get_irq(pdev, 0); + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, + "aoss-qmp", qmp); + if (ret < 0) { + dev_err(&pdev->dev, "failed to request interrupt\n"); + mbox_free_channel(qmp->mbox_chan); + return ret; + } + + ret = qmp_open(qmp); + if (ret < 0) { + mbox_free_channel(qmp->mbox_chan); + return ret; + } + + platform_set_drvdata(pdev, qmp); + + if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) { + qmp->pd_pdev = platform_device_register_data(&pdev->dev, + "aoss_qmp_pd", + PLATFORM_DEVID_NONE, + NULL, 0); + if (IS_ERR(qmp->pd_pdev)) + dev_err(&pdev->dev, "failed to register AOSS PD\n"); + } + + return 0; +} + +static int qmp_remove(struct platform_device *pdev) +{ + struct qmp *qmp = platform_get_drvdata(pdev); + + platform_device_unregister(qmp->pd_pdev); + + mbox_free_channel(qmp->mbox_chan); + qmp_close(qmp); + + return 0; +} + +static const struct of_device_id qmp_dt_match[] = { + { .compatible = "qcom,sdm845-aoss-qmp", }, + {} +}; +MODULE_DEVICE_TABLE(of, qmp_dt_match); + +static struct platform_driver qmp_driver = { + .driver = { + .name = "aoss_qmp", + .of_match_table = qmp_dt_match, + }, + .probe = qmp_probe, + .remove = qmp_remove, +}; +module_platform_driver(qmp_driver); + +MODULE_DESCRIPTION("Qualcomm AOSS QMP driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h new file mode 100644 index 000000000000..a2ac891d7fd4 --- /dev/null +++ b/include/linux/soc/qcom/aoss-qmp.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2018, Linaro Ltd + */ +#ifndef __AOP_QMP_H__ +#define __AOP_QMP_H__ + +#include <linux/types.h> + +struct qmp; + +int qmp_send(struct qmp *qmp, const void *data, size_t len); + +#endif