| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 14 Sep 2022 12:52:52 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Sibi Sankar <quic_sibis@...cinc.com>
Cc: krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org,
jassisinghbrar@...il.com, manivannan.sadhasivam@...aro.org,
agross@...nel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
konrad.dybcio@...ainline.org, Gaurav Kohli <gkohli@...eaurora.org>
Subject: Re: [PATCH 2/2] mailbox: Add support for QTI CPUCP mailbox controller
On Wed, Sep 14, 2022 at 11:33:06AM +0530, Sibi Sankar wrote:
> Add support for CPUSS Control Processor (CPUCP) mailbox controller,
> this driver enables communication between AP and CPUCP by acting as
> a doorbell between them.
>
> Signed-off-by: Gaurav Kohli <gkohli@...eaurora.org>
> [sibis: moved to mailbox and misc. cleanups]
> Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
> ---
> drivers/mailbox/Kconfig | 9 ++
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/qcom-cpucp-mbox.c | 176 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 187 insertions(+)
> create mode 100644 drivers/mailbox/qcom-cpucp-mbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index 05d6fae800e3..7766e0ad2f12 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -284,6 +284,15 @@ config SPRD_MBOX
> to send message between application processors and MCU. Say Y here if
> you want to build the Spreatrum mailbox controller driver.
>
> +config QCOM_CPUCP_MBOX
> + tristate "Qualcomm Technologies, Inc. CPUCP mailbox driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + help
> + Qualcomm Technologies, Inc. CPUSS Control Processor (CPUCP) mailbox
> + controller driver enables communication between AP and CPUCP by
> + acting as a doorbell between them. Say Y here if you want to build
> + this driver.
What will consume this interface?
Given that there's a single channel, is there any benefit of separating
the interrupt handling out of the single client driver?
> +
> config QCOM_IPCC
> tristate "Qualcomm Technologies, Inc. IPCC driver"
> depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index fc9376117111..195b7e40541f 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -59,6 +59,8 @@ obj-$(CONFIG_SUN6I_MSGBOX) += sun6i-msgbox.o
>
> obj-$(CONFIG_SPRD_MBOX) += sprd-mailbox.o
>
> +obj-$(CONFIG_QCOM_CPUCP_MBOX) += qcom-cpucp-mbox.o
> +
> obj-$(CONFIG_QCOM_IPCC) += qcom-ipcc.o
>
> obj-$(CONFIG_APPLE_MAILBOX) += apple-mailbox.o
> diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
> new file mode 100644
> index 000000000000..063bb2d80f3e
> --- /dev/null
> +++ b/drivers/mailbox/qcom-cpucp-mbox.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +/* CPUCP Register offsets */
> +#define CPUCP_INTR_CLEAR_REG 0x8
> +#define CPUCP_INTR_STATUS_REG 0xC
> +#define CPUCP_SEND_IRQ_REG 0xC
> +
> +#define CPUCP_IRQ_CLEAR BIT(3)
> +#define CPUCP_IRQ_STATUS_ASSERTED BIT(3)
> +#define CPUCP_SEND_IRQ BIT(28)
> +
> +/**
> + * struct qcom_cpucp_mbox - Holder for the mailbox driver
> + * @mbox: The mailbox controller
> + * @chan: The mailbox channel
> + * @tx_base: Base address of the CPUCP tx registers
> + * @rx_base: Base address of the CPUCP rx registers
> + * @dev: Device associated with this instance
> + * @lock: Lock protecting private
> + * @irq: CPUCP to AP irq
> + */
> +struct qcom_cpucp_mbox {
> + struct mbox_controller mbox;
> + struct mbox_chan chan;
> + void __iomem *tx_base;
> + void __iomem *rx_base;
> + struct device *dev;
> + int irq;
> +
> + /* control access to the chan private data */
> + spinlock_t lock;
> +};
> +
> +static inline struct qcom_cpucp_mbox *to_qcom_cpucp_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct qcom_cpucp_mbox, mbox);
> +}
> +
> +static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
> +{
> + struct qcom_cpucp_mbox *cpucp = data;
> + unsigned long flags;
> + u32 val;
> +
> + val = readl(cpucp->rx_base + CPUCP_INTR_STATUS_REG);
> + if (val & CPUCP_IRQ_STATUS_ASSERTED) {
> + writel(CPUCP_IRQ_CLEAR, cpucp->rx_base + CPUCP_INTR_CLEAR_REG);
> +
> + spin_lock_irqsave(&cpucp->lock, flags);
> + if (cpucp->chan.con_priv)
> + mbox_chan_received_data(&cpucp->chan, NULL);
Afaict this will deliver a data-less message to the mailbox receiver to
communicate an incoming interrupt.
I would prefer that you represent this as an irq_chip, like we've done
for similar designs previously.
> + spin_unlock_irqrestore(&cpucp->lock, flags);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int qcom_cpucp_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct qcom_cpucp_mbox *cpucp = to_qcom_cpucp_mbox(chan->mbox);
> +
> + writel(CPUCP_SEND_IRQ, cpucp->tx_base + CPUCP_SEND_IRQ_REG);
> +
> + return 0;
> +}
> +
> +static void qcom_cpucp_mbox_shutdown(struct mbox_chan *chan)
> +{
> + struct qcom_cpucp_mbox *cpucp = to_qcom_cpucp_mbox(chan->mbox);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&cpucp->lock, flags);
> + chan->con_priv = NULL;
> + spin_unlock_irqrestore(&cpucp->lock, flags);
> +}
> +
> +static const struct mbox_chan_ops cpucp_mbox_chan_ops = {
> + .send_data = qcom_cpucp_mbox_send_data,
> + .shutdown = qcom_cpucp_mbox_shutdown
> +};
> +
> +static struct mbox_chan *qcom_cpucp_mbox_xlate(struct mbox_controller *mbox,
> + const struct of_phandle_args *ph)
> +{
> + struct qcom_cpucp_mbox *cpucp = to_qcom_cpucp_mbox(mbox);
> +
> + if (ph->args_count != 0)
> + return ERR_PTR(-EINVAL);
> +
> + if (mbox->chans[0].con_priv)
> + return ERR_PTR(-EBUSY);
> +
> + mbox->chans[0].con_priv = cpucp;
> +
> + return &mbox->chans[0];
> +}
> +
> +static int qcom_cpucp_mbox_probe(struct platform_device *pdev)
> +{
> + struct qcom_cpucp_mbox *cpucp;
> + struct mbox_controller *mbox;
> + int ret;
> +
> + cpucp = devm_kzalloc(&pdev->dev, sizeof(*cpucp), GFP_KERNEL);
> + if (!cpucp)
> + return -ENOMEM;
> +
> + spin_lock_init(&cpucp->lock);
> + cpucp->dev = &pdev->dev;
> +
> + cpucp->tx_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(cpucp->tx_base))
> + return PTR_ERR(cpucp->tx_base);
> +
> + cpucp->rx_base = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(cpucp->rx_base))
> + return PTR_ERR(cpucp->rx_base);
> +
> + cpucp->irq = platform_get_irq(pdev, 0);
> + if (cpucp->irq < 0)
> + return cpucp->irq;
> +
> + mbox = &cpucp->mbox;
> + mbox->dev = cpucp->dev;
> + mbox->num_chans = 1;
> + mbox->chans = &cpucp->chan;
> + mbox->ops = &cpucp_mbox_chan_ops;
> + mbox->of_xlate = qcom_cpucp_mbox_xlate;
> + mbox->txdone_irq = false;
> + mbox->txdone_poll = false;
> +
> + ret = devm_mbox_controller_register(&pdev->dev, mbox);
> + if (ret)
> + return ret;
> +
> + ret = devm_request_irq(&pdev->dev, cpucp->irq, qcom_cpucp_mbox_irq_fn,
> + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, "qcom_cpucp_mbox", cpucp);
Please rely on trigger from Devicetree.
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register the irq: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, cpucp);
drvdata seems to be unused.
Regards,
Bjorn
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_cpucp_mbox_of_match[] = {
> + { .compatible = "qcom,cpucp-mbox"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_cpucp_mbox_of_match);
> +
> +static struct platform_driver qcom_cpucp_mbox_driver = {
> + .probe = qcom_cpucp_mbox_probe,
> + .driver = {
> + .name = "qcom_cpucp_mbox",
> + .of_match_table = qcom_cpucp_mbox_of_match,
> + },
> +};
> +module_platform_driver(qcom_cpucp_mbox_driver);
> +
> +MODULE_AUTHOR("Gaurav Kohli <gkohli@...eaurora.org>");
> +MODULE_AUTHOR("Sibi Sankar <quic_sibis@....qualcomm.com>");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
Powered by blists - more mailing lists