Re: [PATCH V3 2/5] mailbox: Add support for QTI CPUCP mailbox controller

From: Dmitry Baryshkov
Date: Wed Apr 17 2024 - 14:28:15 EST


On Wed, 17 Apr 2024 at 16:29, Sibi Sankar <quic_sibis@xxxxxxxxxxx> 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: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
> ---
>
> v2:
> * 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.
>
> drivers/mailbox/Kconfig | 8 ++
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/qcom-cpucp-mbox.c | 188 ++++++++++++++++++++++++++++++
> 3 files changed, 198 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..059eb25f217c
> --- /dev/null
> +++ b/drivers/mailbox/qcom-cpucp-mbox.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/platform_device.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +
> +#define APSS_CPUCP_IPC_CHAN_SUPPORTED 3
> +#define APSS_CPUCP_MBOX_CMD_OFF 0x4
> +
> +/* Tx Registers */
> +#define APSS_CPUCP_TX_MBOX_IDR 0

I don't see _IDR defines being used.

Other than that:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>


> +#define APSS_CPUCP_TX_MBOX_CMD(i) (0x100 + ((i) * 8))
> +
> +/* Rx Registers */
> +#define APSS_CPUCP_RX_MBOX_IDR 0
> +#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 0xFFFFFFFFFFFFFFFF


--
With best wishes
Dmitry