> -----Original Message-----
> From: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> Sent: Monday, December 16, 2024 4:48 PM
> To: Michael Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd
> <sboyd@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Jassi Brar
> <jassisinghbrar@xxxxxxxxx>
> Cc: Palmer Dabbelt <palmer@xxxxxxxxxxx>; Paul Walmsley
> <paul.walmsley@xxxxxxxxxx>; Sunil V L <sunilvl@xxxxxxxxxxxxxxxx>; Rahul
> Pathak <rpathak@xxxxxxxxxxxxxxxx>; Leyfoon Tan
> <leyfoon.tan@xxxxxxxxxxxxxxxx>; Atish Patra <atishp@xxxxxxxxxxxxxx>;
> Andrew Jones <ajones@xxxxxxxxxxxxxxxx>; Anup Patel
> <anup@xxxxxxxxxxxxxx>; linux-clk@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-riscv@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> Subject: [RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY)
> based mailbox driver
>
> Add a mailbox controller driver for the new SBI message proxy extension
> which is part of the SBI v3.0 specification.
>
> Co-developed-by: Rahul Pathak <rpathak@xxxxxxxxxxxxxxxx>
> Signed-off-by: Rahul Pathak <rpathak@xxxxxxxxxxxxxxxx>
> Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> ---
> drivers/mailbox/Kconfig | 11 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/riscv-sbi-mpxy-mbox.c | 979
> ++++++++++++++++++++++++++
> 3 files changed, 992 insertions(+)
> create mode 100644 drivers/mailbox/riscv-sbi-mpxy-mbox.c
>
[...]
> sbi-mpxy-mbox.c
> new file mode 100644
> index 000000000000..0592df3028f9
> --- /dev/null
> +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> @@ -0,0 +1,979 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V SBI Message Proxy (MPXY) mailbox controller driver
> + *
> + * Copyright (C) 2024 Ventana Micro Systems Inc.
> + */
> +
> +#include <asm/sbi.h>
> +#include <linux/cpu.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/jump_label.h>
> +#include <linux/kernel.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/mailbox/riscv-rpmi-message.h>
> +#include <linux/mm.h>
> +#include <linux/msi.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
Sorting include header files based on alphanumeric.
> +#include <linux/percpu.h>
> +#include <linux/platform_device.h>
> +#include <linux/smp.h>
> +
> +/* ====== SBI MPXY extension data structures ====== */
> +
> +/* SBI MPXY MSI related channel attributes */ struct sbi_mpxy_msi_info
> +{
> + /* Lower 32-bits of the MSI target address */
> + u32 msi_addr_lo;
> + /* Upper 32-bits of the MSI target address */
> + u32 msi_addr_hi;
> + /* MSI data value */
> + u32 msi_data;
> +};
> +
> +/*
> + * SBI MPXY standard channel attributes.
> + *
> + * NOTE: The sequence of attribute fields are as-per the
> + * defined sequence in the attribute table in spec (or
> + * as-per the enum sbi_mpxy_attribute_id).
> + */
> +struct sbi_mpxy_channel_attrs {
> + /* Message protocol ID */
> + u32 msg_proto_id;
> + /* Message protocol Version */
Don't need capital letter for "version" .
> + u32 msg_proto_version;
> + /* Message protocol maximum message length */
> + u32 msg_max_len;
> + /* Message protocol message send timeout in microseconds */
> + u32 msg_send_timeout;
> + /* Message protocol message completion timeout in microseconds */
> + u32 msg_completion_timeout;
> + /* Bit array for channel capabilities */
> + u32 capability;
> + /* SSE Event Id */
Same for 'event'.
> + u32 sse_event_id;
> + /* MSI enable/disable control knob */
> + u32 msi_control;
> + /* Channel MSI info */
> + struct sbi_mpxy_msi_info msi_info;
> + /* Events State Control */
Same here
> + u32 events_state_ctrl;
> +};
> +
> +/*
[...]
> +
> +static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
> + void *tx, unsigned long tx_len,
> + void *rx, unsigned long max_rx_len,
> + unsigned long *rx_len)
> +{
> + struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
> + unsigned long rx_bytes;
> + struct sbiret sret;
> +
> + if (!mpxy->shmem_active)
> + return -ENODEV;
> + if (!tx && tx_len)
> + return -EINVAL;
> +
> + get_cpu();
> +
> + /* Message protocols allowed to have no data in messages */
> + if (tx_len)
> + memcpy(mpxy->shmem, tx, tx_len);
> +
> + sret = sbi_ecall(SBI_EXT_MPXY,
> SBI_EXT_MPXY_SEND_MSG_WITH_RESP,
> + channel_id, msg_id, tx_len, 0, 0, 0);
> + if (rx && !sret.error) {
> + rx_bytes = sret.value;
> + rx_bytes = min(max_rx_len, rx_bytes);
Caller should know if the rx_bytes is larger than max_rx_len?
> + memcpy(rx, mpxy->shmem, rx_bytes);
> + if (rx_len)
> + *rx_len = rx_bytes;
> + }
> +
> + put_cpu();
> + return sbi_err_map_linux_errno(sret.error);
> +}
> +
[...]
> +
> +static int mpxy_mbox_setup_msi(struct mbox_chan *chan,
> + struct mpxy_mbox_channel *mchan) {
> + struct device *dev = mchan->mbox->dev;
> + int rc;
> +
> + /* Do nothing if MSI not supported */
> + if (mchan->msi_irq == U32_MAX)
> + return 0;
> +
> + /* Request channel MSI handler */
> + rc = request_threaded_irq(mchan->msi_irq,
> + mpxy_mbox_irq_event,
> + mpxy_mbox_irq_thread,
> + 0, dev_name(dev), chan);
> + if (rc) {
> + dev_err(dev, "failed to request MPXY channel 0x%x IRQ\n",
> + mchan->channel_id);
> + return rc;
> + }
> +
> + /* Enable channel MSI control */
> + mchan->attrs.msi_control = 1;
> + rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_MSI_CONTROL,
> + 1, &mchan->attrs.msi_control);
> + if (rc) {
> + dev_err(dev, "enable MSI control failed for MPXY channel
> 0x%x\n",
> + mchan->channel_id);
> + free_irq(mchan->msi_irq, chan);
Set mchan->attrs.msi_control = 0 if failed?
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void mpxy_mbox_cleanup_msi(struct mbox_chan *chan,
> + struct mpxy_mbox_channel *mchan)
> +{
> + struct device *dev = mchan->mbox->dev;
> + int rc;
> +
> + /* Do nothing if MSI not supported */
> + if (mchan->msi_irq == U32_MAX)
Should check if(!mchan->attrs.msi_control) instead of mchan->msi_irq?
> + return;
> +
> + /* Disable channel MSI control */
> + mchan->attrs.msi_control = 0;
> + rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_MSI_CONTROL,
> + 1, &mchan->attrs.msi_control);
> + if (rc) {
> + dev_err(dev, "disable MSI control failed for MPXY channel
> 0x%x\n",
> + mchan->channel_id);
> + }
> +
> + /* Free channel MSI handler */
> + free_irq(mchan->msi_irq, chan);
> +}
> +
> +static int mpxy_mbox_setup_events(struct mpxy_mbox_channel *mchan) {
> + struct device *dev = mchan->mbox->dev;
> + int rc;
> +
> + /* Do nothing if events state not supported */
> + if (!mchan->have_events_state)
> + return 0;
> +
> + /* Enable channel events state */
> + mchan->attrs.events_state_ctrl = 1;
> + rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> + 1, &mchan->attrs.events_state_ctrl);
> + if (rc) {
> + dev_err(dev, "enable events state failed for MPXY channel
> 0x%x\n",
> + mchan->channel_id);
Should set mchan->attrs.events_state_ctrl = 0; ?
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void mpxy_mbox_cleanup_events(struct mpxy_mbox_channel
> *mchan) {
> + struct device *dev = mchan->mbox->dev;
> + int rc;
> +
> + /* Do nothing if events state not supported */
> + if (!mchan->have_events_state)
Check also if (!mchan->attrs.events_state_ctrl)?
> + return;
> +
> + /* Disable channel events state */
> + mchan->attrs.events_state_ctrl = 0;
> + rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> + 1, &mchan->attrs.events_state_ctrl);
> + if (rc) {
> + dev_err(dev, "disbable events state failed for MPXY channel
Typo ' disbable'.
> 0x%x\n",
> + mchan->channel_id);
> + }
> +}
> +
[...]
> +
> +static int mpxy_mbox_probe(struct platform_device *pdev) {
> + struct device *dev = &pdev->dev;
> + struct mpxy_mbox_channel *mchan;
> + struct mpxy_mbox *mbox;
> + int i, msi_idx, rc;
> + u32 *channel_ids;
> +
> + /*
> + * Initialize MPXY shared memory only once. This also ensures
> + * that SBI MPXY mailbox is probed only once.
> + */
> + if (mpxy_shmem_init_done) {
> + dev_err(dev, "SBI MPXY mailbox already initialized\n");
> + return -EALREADY;
> + }
> +
> + /* Probe for SBI MPXY extension */
> + if (sbi_spec_version < sbi_mk_version(1, 0) ||
> + sbi_probe_extension(SBI_EXT_MPXY) <= 0) {
> + dev_info(dev, "SBI MPXY extension not available\n");
> + return -ENODEV;
> + }
> +
> + /* Setup cpuhp notifier for per-CPU MPXY shared memory */
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/sbi-mpxy-
> shmem",
> + mpxy_setup_shmem, mpxy_cleanup_shmem);
> +
> + /* Mark as MPXY shared memory initialization done */
> + mpxy_shmem_init_done = true;
> +
> + /* Allocate mailbox instance */
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> + mbox->dev = dev;
> + platform_set_drvdata(pdev, mbox);
> +
> + /* Find-out of number of channels */
> + rc = mpxy_get_channel_count(&mbox->channel_count);
> + if (rc) {
> + dev_err(dev, "failed to get number of MPXY channels\n");
Suggest print 'rc' value when error. Same for other error messages below.
> + return rc;
> + }
> + if (!mbox->channel_count) {
> + dev_err(dev, "no MPXY channels available\n");
> + return -ENODEV;
> + }
> +
> + /* Allocate and fetch all channel IDs */
> + channel_ids = devm_kcalloc(dev, mbox->channel_count,
> + sizeof(*channel_ids), GFP_KERNEL);
> + if (!channel_ids)
> + return -ENOMEM;
> + rc = mpxy_get_channel_ids(mbox->channel_count, channel_ids);
> + if (rc) {
> + dev_err(dev, "failed to get number of MPXY channels\n");
> + return rc;
> + }
> +
[...]
Regards
Ley Foon
