| Message ID | 1663135386-26270-3-git-send-email-quic_sibis@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add support for CPUCP mailbox controller | expand |
Hi Sibi, > + > +MODULE_AUTHOR("Gaurav Kohli <gkohli@codeaurora.org>"); > +MODULE_AUTHOR("Sibi Sankar <quic_sibis@qti.qualcomm.com>"); Email address should be quic_sibi@quicinc.com. codeaurora.org is any way deprecated, so we should we keep it ? I know giving the credit is important, but above address will not be reachable anyways. ---Trilok Soni
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@codeaurora.org> > [sibis: moved to mailbox and misc. cleanups] > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.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@codeaurora.org>"); > +MODULE_AUTHOR("Sibi Sankar <quic_sibis@qti.qualcomm.com>"); > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver"); > +MODULE_LICENSE("GPL"); > -- > 2.7.4 >
Hey Trilok, Thanks for taking time to review the patch. On 9/14/22 11:08 PM, Trilok Soni wrote: > Hi Sibi, > >> + >> +MODULE_AUTHOR("Gaurav Kohli <gkohli@codeaurora.org>"); >> +MODULE_AUTHOR("Sibi Sankar <quic_sibis@qti.qualcomm.com>"); > > Email address should be quic_sibi@quicinc.com. > > codeaurora.org is any way deprecated, so we should we keep it ? I know > giving the credit is important, but above address will not be reachable > anyways. Ack my bad. > > ---Trilok Soni
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@codeaurora.org> > [sibis: moved to mailbox and misc. cleanups] > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- [...] > + > + ret = devm_request_irq(&pdev->dev, cpucp->irq, qcom_cpucp_mbox_irq_fn, > + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, "qcom_cpucp_mbox", cpucp); 1. Do you plan to use this mailbox during noirq phase of system suspend ? If not why do you have IRQF_NO_SUSPEND ? If yes, how will it be used ? 2. Why are you setting IRQF_TRIGGER_HIGH here ? Won't platform_get_irq() take care of that ? Otherwise how would you use this driver on the platform that need say IRQF_TRIGGER_LOW ?
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. + 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); + 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); + if (ret < 0) { + dev_err(&pdev->dev, "Failed to register the irq: %d\n", ret); + return ret; + } + + platform_set_drvdata(pdev, cpucp); + + 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@codeaurora.org>"); +MODULE_AUTHOR("Sibi Sankar <quic_sibis@qti.qualcomm.com>"); +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver"); +MODULE_LICENSE("GPL");