2022-09-14 06:06:46

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH 0/2] Add support for CPUCP mailbox controller

Add support for CPUSS Control Processor (CPUCP) mailbox controller
which enables communication between AP and CPUCP by acting as a
doorbell between them.

Sibi Sankar (2):
dt-bindings: mailbox: Add dt binding for QTI CPUCP mailbox controller
mailbox: Add support for QTI CPUCP mailbox controller

.../bindings/mailbox/qcom,cpucp-mbox.yaml | 51 ++++++
drivers/mailbox/Kconfig | 9 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/qcom-cpucp-mbox.c | 176 +++++++++++++++++++++
4 files changed, 238 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
create mode 100644 drivers/mailbox/qcom-cpucp-mbox.c

--
2.7.4


2022-09-14 06:47:03

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH 2/2] mailbox: Add support for QTI CPUCP mailbox controller

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 <[email protected]>
[sibis: moved to mailbox and misc. cleanups]
Signed-off-by: Sibi Sankar <[email protected]>
---
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.
+
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 <[email protected]>");
+MODULE_AUTHOR("Sibi Sankar <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver");
+MODULE_LICENSE("GPL");
--
2.7.4

2022-09-14 07:36:54

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: mailbox: Add dt binding for QTI CPUCP mailbox controller

Add devicetree binding for CPUSS Control Processor (CPUCP) mailbox
controller.

Signed-off-by: Sibi Sankar <[email protected]>
---
.../bindings/mailbox/qcom,cpucp-mbox.yaml | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml b/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
new file mode 100644
index 000000000000..1f7e1204cda0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/qcom,cpucp-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. CPUCP Mailbox Controller
+
+maintainers:
+ - Sibi Sankar <[email protected]>
+
+description:
+ The CPUSS Control Processor (CPUCP) mailbox controller enables communication
+ between AP and CPUCP by acting as a doorbell between them.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - qcom,sc7280-cpucp-mbox
+ - const: qcom,cpucp-mbox
+
+ reg:
+ items:
+ - description: CPUCP tx register region
+ - description: CPUCP rx register region
+
+ interrupts:
+ maxItems: 1
+
+ "#mbox-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mailbox@17400000 {
+ compatible = "qcom,sc7280-cpucp-mbox", "qcom,cpucp-mbox";
+ reg = <0x0 0x17c00000 0x0 0x10>, <0x0 0x18590300 0x0 0x700>;
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ #mbox-cells = <0>;
+ };
--
2.7.4

2022-09-14 11:38:58

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mailbox: Add dt binding for QTI CPUCP mailbox controller

On Wed, 14 Sep 2022 11:33:05 +0530, Sibi Sankar wrote:
> Add devicetree binding for CPUSS Control Processor (CPUCP) mailbox
> controller.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> .../bindings/mailbox/qcom,cpucp-mbox.yaml | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.example.dtb: mailbox@17400000: reg: [[0, 398458880], [0, 16], [0, 408486656], [0, 1792]] is too long
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-09-14 18:17:28

by Bjorn Andersson

[permalink] [raw]
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 <[email protected]>
> [sibis: moved to mailbox and misc. cleanups]
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> 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 <[email protected]>");
> +MODULE_AUTHOR("Sibi Sankar <[email protected]>");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>

2022-09-14 18:29:39

by Trilok Soni

[permalink] [raw]
Subject: Re: [PATCH 2/2] mailbox: Add support for QTI CPUCP mailbox controller

Hi Sibi,

> +
> +MODULE_AUTHOR("Gaurav Kohli <[email protected]>");
> +MODULE_AUTHOR("Sibi Sankar <[email protected]>");

Email address should be [email protected].

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

2022-09-15 01:49:27

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 2/2] mailbox: Add support for QTI CPUCP mailbox controller

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 <[email protected]>");
>> +MODULE_AUTHOR("Sibi Sankar <[email protected]>");
>
> Email address should be [email protected].
>
> 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

2022-09-15 01:49:50

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mailbox: Add dt binding for QTI CPUCP mailbox controller



On 9/14/22 4:38 PM, Rob Herring wrote:
> On Wed, 14 Sep 2022 11:33:05 +0530, Sibi Sankar wrote:
>> Add devicetree binding for CPUSS Control Processor (CPUCP) mailbox
>> controller.
>>
>> Signed-off-by: Sibi Sankar <[email protected]>
>> ---
>> .../bindings/mailbox/qcom,cpucp-mbox.yaml | 51 ++++++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.example.dtb: mailbox@17400000: reg: [[0, 398458880], [0, 16], [0, 408486656], [0, 1792]] is too long
> From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
>
> doc reference errors (make refcheckdocs):

yup noticed that I hadn't included these fixes when I sent the patches
out. Will fix it in the next re-spin.

>
> See https://patchwork.ozlabs.org/patch/
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

2022-11-03 11:27:37

by Sudeep Holla

[permalink] [raw]
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 <[email protected]>
> [sibis: moved to mailbox and misc. cleanups]
> Signed-off-by: Sibi Sankar <[email protected]>
> ---

[...]

> +
> + 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 ?

--
Regards,
Sudeep

2022-12-13 14:30:06

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 2/2] mailbox: Add support for QTI CPUCP mailbox controller

Additional patches got tagged along. Please ignore.


On 12/13/22 19:34, 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 <[email protected]>
> [sibis: moved to mailbox and misc. cleanups]
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> 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.
> +
> 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 <[email protected]>");
> +MODULE_AUTHOR("Sibi Sankar <[email protected]>");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUSS Control Processor Mailbox driver");
> +MODULE_LICENSE("GPL");

2022-12-13 14:46:28

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 0/2] Add support for CPUCP mailbox controller

Additional patches got tagged along. Please ignore.

On 12/13/22 19:34, Sibi Sankar wrote:
> Add support for CPUSS Control Processor (CPUCP) mailbox controller
> which enables communication between AP and CPUCP by acting as a
> doorbell between them.
>
> Sibi Sankar (2):
> dt-bindings: mailbox: Add dt binding for QTI CPUCP mailbox controller
> mailbox: Add support for QTI CPUCP mailbox controller
>
> .../bindings/mailbox/qcom,cpucp-mbox.yaml | 51 ++++++
> drivers/mailbox/Kconfig | 9 ++
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/qcom-cpucp-mbox.c | 176 +++++++++++++++++++++
> 4 files changed, 238 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
> create mode 100644 drivers/mailbox/qcom-cpucp-mbox.c
>