This series enables CPUFreq support on the X1E SoC using the SCMI perf
protocol. This was originally part of the RFC: firmware: arm_scmi:
Qualcomm Vendor Protocol [1]. I've split it up so that this part can
land earlier.
V3:
* Fix Maintainer info in cpucp mbox bindings. [Bjorn]
* Fix copyright info in cpucp driver. [Bjorn]
* Drop unused APSS_CPUCP_TX_MBOX_IDR, value init and drv_data. [Bjorn/Dmitry]
* Convert to lower case hex. [Bjorn]
* Convert irq and dev to local variables. [Bjorn]
* Replace for and if with for_each_set_bit. [Bjorn]
* Document the need for spinlock. [Bjorn]
* Add space after " for aesthetics. [Bjorn]
* Fix err in calc and add fixes tag. [Bjorn]
* Include io.h and re-order platform_device.h
* Use GENMASK_ULL to generate APSS_CPUCP_RX_MBOX_CMD_MASK.
V2:
* Fix series version number [Rob]
* Pickup Rbs from Dimitry and Rob.
* Use power-domain instead of clocks. [Sudeep/Ulf]
* Rename sram sub-nodes according to schema. [Dmitry]
* Use BIT() instead of manual shift. [Dmitry]
* Define RX_MBOX_CMD to account for chan calculation. [Dmitry]
* Clear the bit instead of the entire status within the spinlock. [Dmitry]
* Use dev_err_probe instead. [Dmitry]
* Drop superfluous error message while handling errors from get_irq. [Dmitry]
* Use devm_mbox_controller_register and drop remove path. [Dmitry]
* Define TX_MBOX_CMD to account for chan calculation.
* Use cpucp->dev in probe path for conformity.
RFC V1:
* Use x1e80100 as the fallback for future SoCs using the cpucp-mbox
controller. [Krzysztoff/Konrad/Rob]
* Use chan->lock and chan->cl to detect if the channel is no longer
Available. [Dmitry]
* Use BIT() instead of using manual shifts. [Dmitry]
* Don't use integer as a pointer value. [Dmitry]
* Allow it to default to of_mbox_index_xlate. [Dmitry]
* Use devm_of_iomap. [Dmitry]
* Use module_platform_driver instead of module init/exit. [Dmitry]
* Get channel number using mailbox core (like other drivers) and
further simplify the driver by dropping setup_mbox func.
[1]: https://lore.kernel.org/lkml/[email protected]/#r
Other relevant Links:
https://lore.kernel.org/lkml/[email protected]/
Sibi Sankar (5):
dt-bindings: mailbox: qcom: Add CPUCP mailbox controller bindings
mailbox: Add support for QTI CPUCP mailbox controller
arm64: dts: qcom: x1e80100: Resize GIC Redistributor register region
arm64: dts: qcom: x1e80100: Add cpucp mailbox and sram nodes
arm64: dts: qcom: x1e80100: Enable cpufreq
.../bindings/mailbox/qcom,cpucp-mbox.yaml | 49 +++++
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 91 ++++++---
drivers/mailbox/Kconfig | 8 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/qcom-cpucp-mbox.c | 179 ++++++++++++++++++
5 files changed, 304 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
create mode 100644 drivers/mailbox/qcom-cpucp-mbox.c
--
2.34.1
Add the cpucp mailbox and sram nodes required by SCMI perf protocol
on X1E80100 SoCs.
Signed-off-by: Sibi Sankar <[email protected]>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index c48b3fdc550b..6dcd851f31b2 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -4972,6 +4972,13 @@ gic_its: msi-controller@17040000 {
};
};
+ cpucp_mbox: mailbox@17430000 {
+ compatible = "qcom,x1e80100-cpucp-mbox";
+ reg = <0 0x17430000 0 0x10000>, <0 0x18830000 0 0x10000>;
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+ #mbox-cells = <1>;
+ };
+
apps_rsc: rsc@17500000 {
compatible = "qcom,rpmh-rsc";
reg = <0 0x17500000 0 0x10000>,
@@ -5155,6 +5162,25 @@ frame@1780d000 {
};
};
+ sram: sram@18b4e000 {
+ compatible = "mmio-sram";
+ reg = <0x0 0x18b4e000 0x0 0x400>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x0 0x18b4e000 0x400>;
+
+ cpu_scp_lpri0: scp-sram-section@0 {
+ compatible = "arm,scmi-shmem";
+ reg = <0x0 0x200>;
+ };
+
+ cpu_scp_lpri1: scp-sram-section@200 {
+ compatible = "arm,scmi-shmem";
+ reg = <0x200 0x200>;
+ };
+ };
+
system-cache-controller@25000000 {
compatible = "qcom,x1e80100-llcc";
reg = <0 0x25000000 0 0x200000>,
--
2.34.1
Enable cpufreq on X1E80100 SoCs through the SCMI perf protocol node.
Signed-off-by: Sibi Sankar <[email protected]>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 63 ++++++++++++++++----------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 6dcd851f31b2..96d1c5bab13f 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -69,8 +69,8 @@ CPU0: cpu@0 {
reg = <0x0 0x0>;
enable-method = "psci";
next-level-cache = <&L2_0>;
- power-domains = <&CPU_PD0>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD0>, <&scmi_dvfs 0>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
L2_0: l2-cache {
@@ -86,8 +86,8 @@ CPU1: cpu@100 {
reg = <0x0 0x100>;
enable-method = "psci";
next-level-cache = <&L2_0>;
- power-domains = <&CPU_PD1>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD1>, <&scmi_dvfs 0>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -97,8 +97,8 @@ CPU2: cpu@200 {
reg = <0x0 0x200>;
enable-method = "psci";
next-level-cache = <&L2_0>;
- power-domains = <&CPU_PD2>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD2>, <&scmi_dvfs 0>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -108,8 +108,8 @@ CPU3: cpu@300 {
reg = <0x0 0x300>;
enable-method = "psci";
next-level-cache = <&L2_0>;
- power-domains = <&CPU_PD3>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD3>, <&scmi_dvfs 0>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -119,8 +119,8 @@ CPU4: cpu@10000 {
reg = <0x0 0x10000>;
enable-method = "psci";
next-level-cache = <&L2_1>;
- power-domains = <&CPU_PD4>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD4>, <&scmi_dvfs 1>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
L2_1: l2-cache {
@@ -136,8 +136,8 @@ CPU5: cpu@10100 {
reg = <0x0 0x10100>;
enable-method = "psci";
next-level-cache = <&L2_1>;
- power-domains = <&CPU_PD5>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD5>, <&scmi_dvfs 1>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -147,8 +147,8 @@ CPU6: cpu@10200 {
reg = <0x0 0x10200>;
enable-method = "psci";
next-level-cache = <&L2_1>;
- power-domains = <&CPU_PD6>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD6>, <&scmi_dvfs 1>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -158,8 +158,8 @@ CPU7: cpu@10300 {
reg = <0x0 0x10300>;
enable-method = "psci";
next-level-cache = <&L2_1>;
- power-domains = <&CPU_PD7>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD7>, <&scmi_dvfs 1>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -169,8 +169,8 @@ CPU8: cpu@20000 {
reg = <0x0 0x20000>;
enable-method = "psci";
next-level-cache = <&L2_2>;
- power-domains = <&CPU_PD8>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD8>, <&scmi_dvfs 2>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
L2_2: l2-cache {
@@ -186,8 +186,8 @@ CPU9: cpu@20100 {
reg = <0x0 0x20100>;
enable-method = "psci";
next-level-cache = <&L2_2>;
- power-domains = <&CPU_PD9>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD9>, <&scmi_dvfs 2>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -197,8 +197,8 @@ CPU10: cpu@20200 {
reg = <0x0 0x20200>;
enable-method = "psci";
next-level-cache = <&L2_2>;
- power-domains = <&CPU_PD10>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD10>, <&scmi_dvfs 2>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -208,8 +208,8 @@ CPU11: cpu@20300 {
reg = <0x0 0x20300>;
enable-method = "psci";
next-level-cache = <&L2_2>;
- power-domains = <&CPU_PD11>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD11>, <&scmi_dvfs 2>;
+ power-domain-names = "psci", "perf";
cpu-idle-states = <&CLUSTER_C4>;
};
@@ -309,6 +309,21 @@ scm: scm {
interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
};
+
+ scmi {
+ compatible = "arm,scmi";
+ mboxes = <&cpucp_mbox 0>, <&cpucp_mbox 2>;
+ mbox-names = "tx", "rx";
+ shmem = <&cpu_scp_lpri0>, <&cpu_scp_lpri1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ scmi_dvfs: protocol@13 {
+ reg = <0x13>;
+ #power-domain-cells = <1>;
+ };
+ };
};
clk_virt: interconnect-0 {
--
2.34.1
Add devicetree binding for CPUSS Control Processor (CPUCP) mailbox
controller.
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
V3:
* Fix Maintainer info in cpucp mbox bindings. [Bjorn]
.../bindings/mailbox/qcom,cpucp-mbox.yaml | 49 +++++++++++++++++++
1 file changed, 49 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..f7342d04beec
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,cpucp-mbox.yaml
@@ -0,0 +1,49 @@
+# 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:
+ - const: qcom,x1e80100-cpucp-mbox
+
+ reg:
+ items:
+ - description: CPUCP rx register region
+ - description: CPUCP tx register region
+
+ interrupts:
+ maxItems: 1
+
+ "#mbox-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mailbox@17430000 {
+ compatible = "qcom,x1e80100-cpucp-mbox";
+ reg = <0x17430000 0x10000>, <0x18830000 0x10000>;
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+ #mbox-cells = <1>;
+ };
--
2.34.1
Add support for CPUSS Control Processor (CPUCP) mailbox controller,
this driver enables communication between AP and CPUCP by acting as
a doorbell between them.
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
V3:
* Fix copyright info in cpucp driver. [Bjorn]
* Drop unused APSS_CPUCP_TX_MBOX_IDR, value init and drv_data. [Bjorn/Dmitry]
* Convert to lower case hex. [Bjorn]
* Convert irq and dev to local variables. [Bjorn]
* Replace for and if with for_each_set_bit. [Bjorn]
* Document the need for spinlock. [Bjorn]
* Add space after " for aesthetics. [Bjorn]
* Include io.h and re-order platform_device.h
* Use GENMASK_ULL to generate APSS_CPUCP_RX_MBOX_CMD_MASK.
* Pick Rb.
drivers/mailbox/Kconfig | 8 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/qcom-cpucp-mbox.c | 179 ++++++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
create mode 100644 drivers/mailbox/qcom-cpucp-mbox.c
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 42940108a187..23741a6f054e 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -273,6 +273,14 @@ 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. 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 18793e6caa2f..53b512800bde 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -59,4 +59,6 @@ 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
diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
new file mode 100644
index 000000000000..78d0872a0e33
--- /dev/null
+++ b/drivers/mailbox/qcom-cpucp-mbox.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define APSS_CPUCP_IPC_CHAN_SUPPORTED 3
+#define APSS_CPUCP_MBOX_CMD_OFF 0x4
+
+/* Tx Registers */
+#define APSS_CPUCP_TX_MBOX_CMD(i) (0x100 + ((i) * 8))
+
+/* Rx Registers */
+#define APSS_CPUCP_RX_MBOX_CMD(i) (0x100 + ((i) * 8))
+#define APSS_CPUCP_RX_MBOX_MAP 0x4000
+#define APSS_CPUCP_RX_MBOX_STAT 0x4400
+#define APSS_CPUCP_RX_MBOX_CLEAR 0x4800
+#define APSS_CPUCP_RX_MBOX_EN 0x4c00
+#define APSS_CPUCP_RX_MBOX_CMD_MASK GENMASK_ULL(63, 0)
+
+/**
+ * struct qcom_cpucp_mbox - Holder for the mailbox driver
+ * @chans: The mailbox channel
+ * @mbox: The mailbox controller
+ * @tx_base: Base address of the CPUCP tx registers
+ * @rx_base: Base address of the CPUCP rx registers
+ */
+struct qcom_cpucp_mbox {
+ struct mbox_chan chans[APSS_CPUCP_IPC_CHAN_SUPPORTED];
+ struct mbox_controller mbox;
+ void __iomem *tx_base;
+ void __iomem *rx_base;
+};
+
+static inline int channel_number(struct mbox_chan *chan)
+{
+ return chan - chan->mbox->chans;
+}
+
+static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
+{
+ struct qcom_cpucp_mbox *cpucp = data;
+ struct mbox_chan *chan;
+ unsigned long flags;
+ u64 status;
+ u32 val;
+ int i;
+
+ status = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_STAT);
+
+ for_each_set_bit(i, (unsigned long *)&status, APSS_CPUCP_IPC_CHAN_SUPPORTED) {
+ val = readl(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i) + APSS_CPUCP_MBOX_CMD_OFF);
+ chan = &cpucp->chans[i];
+ /* Provide mutual exclusion with changes to chan->cl */
+ spin_lock_irqsave(&chan->lock, flags);
+ if (chan->cl)
+ mbox_chan_received_data(chan, &val);
+ writeq(BIT(i), cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
+ spin_unlock_irqrestore(&chan->lock, flags);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int qcom_cpucp_mbox_startup(struct mbox_chan *chan)
+{
+ struct qcom_cpucp_mbox *cpucp = container_of(chan->mbox, struct qcom_cpucp_mbox, mbox);
+ unsigned long chan_id = channel_number(chan);
+ u64 val;
+
+ val = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
+ val |= BIT(chan_id);
+ writeq(val, cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
+
+ return 0;
+}
+
+static void qcom_cpucp_mbox_shutdown(struct mbox_chan *chan)
+{
+ struct qcom_cpucp_mbox *cpucp = container_of(chan->mbox, struct qcom_cpucp_mbox, mbox);
+ unsigned long chan_id = channel_number(chan);
+ u64 val;
+
+ val = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
+ val &= ~BIT(chan_id);
+ writeq(val, cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
+}
+
+static int qcom_cpucp_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+ struct qcom_cpucp_mbox *cpucp = container_of(chan->mbox, struct qcom_cpucp_mbox, mbox);
+ unsigned long chan_id = channel_number(chan);
+ u32 *val = data;
+
+ writel(*val, cpucp->tx_base + APSS_CPUCP_TX_MBOX_CMD(chan_id) + APSS_CPUCP_MBOX_CMD_OFF);
+
+ return 0;
+}
+
+static const struct mbox_chan_ops qcom_cpucp_mbox_chan_ops = {
+ .startup = qcom_cpucp_mbox_startup,
+ .send_data = qcom_cpucp_mbox_send_data,
+ .shutdown = qcom_cpucp_mbox_shutdown
+};
+
+static int qcom_cpucp_mbox_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct qcom_cpucp_mbox *cpucp;
+ struct mbox_controller *mbox;
+ int irq, ret;
+
+ cpucp = devm_kzalloc(dev, sizeof(*cpucp), GFP_KERNEL);
+ if (!cpucp)
+ return -ENOMEM;
+
+ cpucp->rx_base = devm_of_iomap(dev, dev->of_node, 0, NULL);
+ if (IS_ERR(cpucp->rx_base))
+ return PTR_ERR(cpucp->rx_base);
+
+ cpucp->tx_base = devm_of_iomap(dev, dev->of_node, 1, NULL);
+ if (IS_ERR(cpucp->tx_base))
+ return PTR_ERR(cpucp->tx_base);
+
+ writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
+ writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
+ writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_irq(dev, irq, qcom_cpucp_mbox_irq_fn,
+ IRQF_TRIGGER_HIGH, "apss_cpucp_mbox", cpucp);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to register irq: %d\n", irq);
+
+ writeq(APSS_CPUCP_RX_MBOX_CMD_MASK, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
+
+ mbox = &cpucp->mbox;
+ mbox->dev = dev;
+ mbox->num_chans = APSS_CPUCP_IPC_CHAN_SUPPORTED;
+ mbox->chans = cpucp->chans;
+ mbox->ops = &qcom_cpucp_mbox_chan_ops;
+ mbox->txdone_irq = false;
+ mbox->txdone_poll = false;
+
+ ret = devm_mbox_controller_register(dev, mbox);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to create mailbox\n");
+
+ return 0;
+}
+
+static const struct of_device_id qcom_cpucp_mbox_of_match[] = {
+ { .compatible = "qcom,x1e80100-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_DESCRIPTION("QTI CPUCP MBOX Driver");
+MODULE_LICENSE("GPL");
--
2.34.1
Resize the GICR register region as it currently seeps into the CPU Control
Processor mailbox RX region.
Fixes: af16b00578a7 ("arm64: dts: qcom: Add base X1E80100 dtsi and the QCP dts")
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
V3:
* Fix err in calc and add fixes tag. [Bjorn]
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 5f90a0b3c016..c48b3fdc550b 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -4947,7 +4947,7 @@ apps_smmu: iommu@15000000 {
intc: interrupt-controller@17000000 {
compatible = "arm,gic-v3";
reg = <0 0x17000000 0 0x10000>, /* GICD */
- <0 0x17080000 0 0x480000>; /* GICR * 12 */
+ <0 0x17080000 0 0x300000>; /* GICR * 12 */
interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
--
2.34.1
On 4/22/24 18:40, 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.
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
[...]
> +
> +static int qcom_cpucp_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct qcom_cpucp_mbox *cpucp = container_of(chan->mbox, struct qcom_cpucp_mbox, mbox);
> + unsigned long chan_id = channel_number(chan);
> + u32 *val = data;
> +
> + writel(*val, cpucp->tx_base + APSS_CPUCP_TX_MBOX_CMD(chan_id) + APSS_CPUCP_MBOX_CMD_OFF);
Just checking in, is *this access only* supposed to be 32b instead of 64 like others?
[...]
> +
> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
If these writes are here to prevent a possible interrupt storm type tragedy,
you need to read back these registers to ensure the writes have left the CPU
complex and reached the observer at the other end of the bus (not to be
confused with barriers which only ensure that such accesses are ordered
*when still possibly within the CPU complex*).
Moreover, if the order of them arriving (en/clear/mask) doesn't matter, you
can add _relaxed for a possible nanosecond-order perf gain
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_irq(dev, irq, qcom_cpucp_mbox_irq_fn,
> + IRQF_TRIGGER_HIGH, "apss_cpucp_mbox", cpucp);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to register irq: %d\n", irq);
> +
> + writeq(APSS_CPUCP_RX_MBOX_CMD_MASK, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
Similarly here, unless read back, we may potentially miss some interrupts if
e.g. a channel is opened and that write "is decided" (by the silicon) to leave
the internal buffer first
> +
> + mbox = &cpucp->mbox;
> + mbox->dev = dev;
> + mbox->num_chans = APSS_CPUCP_IPC_CHAN_SUPPORTED;
> + mbox->chans = cpucp->chans;
> + mbox->ops = &qcom_cpucp_mbox_chan_ops;
> + mbox->txdone_irq = false;
> + mbox->txdone_poll = false;
"false" == 0 is the default value (as you're using k*z*alloc)
> +
> + ret = devm_mbox_controller_register(dev, mbox);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to create mailbox\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_cpucp_mbox_of_match[] = {
> + { .compatible = "qcom,x1e80100-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);
That's turbo late. Go core_initcall.
Konrad
On 4/23/24 04:47, Konrad Dybcio wrote:
>
>
> On 4/22/24 18:40, 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.
>>
>> Reviewed-by: Dmitry Baryshkov <[email protected]>
>> Signed-off-by: Sibi Sankar <[email protected]>
>> ---
>
> [...]
>
>> +
>> +static int qcom_cpucp_mbox_send_data(struct mbox_chan *chan, void *data)
>> +{
>> + struct qcom_cpucp_mbox *cpucp = container_of(chan->mbox, struct
>> qcom_cpucp_mbox, mbox);
>> + unsigned long chan_id = channel_number(chan);
>> + u32 *val = data;
>> +
>> + writel(*val, cpucp->tx_base + APSS_CPUCP_TX_MBOX_CMD(chan_id) +
>> APSS_CPUCP_MBOX_CMD_OFF);
>
Hey Konrad,
Thanks for taking time to review the series.
> Just checking in, is *this access only* supposed to be 32b instead of 64
> like others?
yeah, the readl and writely in the driver were used intentionally.
>
> [...]
>
>> +
>> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_EN);
>> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
>> + writeq(0, cpucp->rx_base + APSS_CPUCP_RX_MBOX_MAP);
>
> If these writes are here to prevent a possible interrupt storm type
> tragedy,
> you need to read back these registers to ensure the writes have left the
> CPU
> complex and reached the observer at the other end of the bus (not to be
> confused with barriers which only ensure that such accesses are ordered
> *when still possibly within the CPU complex*).
I couldn't find anything alluding to ^^. This sequence was just
meant to reset the mailbox. Looks like we do need to preserve the
ordering so relaxed read/writes aren't an option.
-Sibi
>
> Moreover, if the order of them arriving (en/clear/mask) doesn't matter, you
> can add _relaxed for a possible nanosecond-order perf gain
>
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0)
>> + return irq;
>> +
>> + ret = devm_request_irq(dev, irq, qcom_cpucp_mbox_irq_fn,
>> + IRQF_TRIGGER_HIGH, "apss_cpucp_mbox", cpucp);
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret, "Failed to register irq:
>> %d\n", irq);
>> +
>> + writeq(APSS_CPUCP_RX_MBOX_CMD_MASK, cpucp->rx_base +
>> APSS_CPUCP_RX_MBOX_MAP);
>
> Similarly here, unless read back, we may potentially miss some
> interrupts if
> e.g. a channel is opened and that write "is decided" (by the silicon) to
> leave
> the internal buffer first
At this point in time we don't expect any interrupts. They are expected
only after channel activation. Also there were no recommendations for
reading it back here as well.
-Sibi
>
>
>> +
>> + mbox = &cpucp->mbox;
>> + mbox->dev = dev;
>> + mbox->num_chans = APSS_CPUCP_IPC_CHAN_SUPPORTED;
>> + mbox->chans = cpucp->chans;
>> + mbox->ops = &qcom_cpucp_mbox_chan_ops;
>> + mbox->txdone_irq = false;
>> + mbox->txdone_poll = false;
>
> "false" == 0 is the default value (as you're using k*z*alloc)
>
>
>> +
>> + ret = devm_mbox_controller_register(dev, mbox);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to create mailbox\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id qcom_cpucp_mbox_of_match[] = {
>> + { .compatible = "qcom,x1e80100-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);
>
> That's turbo late. Go core_initcall.
>
> Konrad
On Mon, Apr 22, 2024 at 11:41 AM Sibi Sankar <[email protected]> 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.
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
Do you want to add an entry in the MAINTAINERS ?
> diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
.....
> +static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
> +{
> + struct qcom_cpucp_mbox *cpucp = data;
> + struct mbox_chan *chan;
> + unsigned long flags;
> + u64 status;
> + u32 val;
> + int i;
> +
The variables flags, val and chan are better inside the for loop below.
> + status = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_STAT);
> +
> + for_each_set_bit(i, (unsigned long *)&status, APSS_CPUCP_IPC_CHAN_SUPPORTED) {
> + val = readl(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i) + APSS_CPUCP_MBOX_CMD_OFF);
> + chan = &cpucp->chans[i];
> + /* Provide mutual exclusion with changes to chan->cl */
> + spin_lock_irqsave(&chan->lock, flags);
> + if (chan->cl)
> + mbox_chan_received_data(chan, &val);
> + writeq(BIT(i), cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
> + spin_unlock_irqrestore(&chan->lock, flags);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
Thanks
Jassi
On Mon, Apr 22, 2024 at 10:10:32PM +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.
>
Hi Sibi,
one small reflection about locking on the RX path down below...
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
>
[snip]
> +struct qcom_cpucp_mbox {
> + struct mbox_chan chans[APSS_CPUCP_IPC_CHAN_SUPPORTED];
> + struct mbox_controller mbox;
> + void __iomem *tx_base;
> + void __iomem *rx_base;
> +};
> +
> +static inline int channel_number(struct mbox_chan *chan)
> +{
> + return chan - chan->mbox->chans;
> +}
> +
> +static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
> +{
> + struct qcom_cpucp_mbox *cpucp = data;
> + struct mbox_chan *chan;
> + unsigned long flags;
> + u64 status;
> + u32 val;
> + int i;
> +
> + status = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_STAT);
> +
> + for_each_set_bit(i, (unsigned long *)&status, APSS_CPUCP_IPC_CHAN_SUPPORTED) {
> + val = readl(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i) + APSS_CPUCP_MBOX_CMD_OFF);
> + chan = &cpucp->chans[i];
> + /* Provide mutual exclusion with changes to chan->cl */
> + spin_lock_irqsave(&chan->lock, flags);
> + if (chan->cl)
So the spinlock here is needed to properly check for races on chan->cl
being NULLified by concurrent calls to mbox_channel_free()...the end
result, though, is that you disable IRQs here on each single data
processed on the RX path, while calling mbox_chan_received_data(), in order
to avoid the remote (but real) possibility that the mbox users could free
the channel while some traffic is still in-flight and processed by this ISR.
Note that, though, that mbox_channel_free() calls straight away at start
your controller provided qcom_cpucp_mbox_shutdown() method, where you disable
the IRQ at the HW level in the chip: this means that the only race which could
then happen between the call to .shutdown and chan->cl = NULL, would happen in
any already executing qcom_cpucp_mbox_irq_fn() ISR...
So, I was thinking, what if you add a
sincronize_irq(cpucp->irq);
in your shutdown right after having disabled the HW IRQs.
This would mean waiting for the termination of any IRQ handlers pending on your
cpucp->irq (field that does not exist as of now :D), right after having
disabled such irq and so just before NULLifying chan->cl...in this way you
should be able to safely drop this spinlock call from the host RX path,
because once you chan->cl = NULL is executed, the IRQs are disabled and
any ongoing ISR would have been terminated.
syncronize_irq() is blocking of course, potentially, but the shutdown
method in mbox_chan_ops is allowed to be blocking looking at the comments.
..not sure if all of this is worth to avoid this small section of code to be
run with IRQs disabled....note though that the mbox_chan_received_data() calls
straight away into the client provided cl->callback....so the real lenght of this
code path is uncertain ....
..just an idea to reason about...
Thanks,
Cristian