* [PATCH v3 0/5] Add support for the PolarFire SoC system controller
@ 2020-12-23 16:32 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:32 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: lewis.hanly, cyril.jean, daire.mcnamara, atish.patra, anup.patel,
david.abdurachmanov, j.neuschaefer, Conor Dooley
From: Conor Dooley <conor.dooley@microchip.com>
This patch series adds support for the system controller on
the PolarFire SoC, using the mailbox framework. A Microchip directory
in the SoC subsystem has been created to hold the mailbox client
driver and will be used for future service drivers.
This directory is included by the kconfig option:
CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
Patra's PolarFire SoC support patches which introduce that option.
It further depends on the MAINTAINERS entry created in the same series.
Changes from v2:
* Further reworked dt bindings to satisfy errors and feedback
(hopefully phandle array is the correct type for the mboxes)
* Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
* Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
* Further simplified driver code from feedback
Changes from v1:
* Squashed header into first patch
* Fixed DT binding warnings & small fixes
* Cleaned up drivers from feedback
Conor Dooley (5):
mbox: add polarfire soc system controller mailbox
dt-bindings: add bindings for polarfire soc mailbox
soc: add polarfire soc system controller
dt-bindings: add bindings for polarfire soc system controller
MAINTAINERS: add maintainers for polarfire soc mailbox driver
.../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
.../microchip,mpfs_sys_controller.yaml | 34 +++
MAINTAINERS | 1 +
drivers/mailbox/Kconfig | 12 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/microchip/Kconfig | 10 +
drivers/soc/microchip/Makefile | 1 +
drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
include/soc/microchip/mpfs.h | 51 ++++
12 files changed, 572 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
create mode 100644 drivers/mailbox/mailbox-mpfs.c
create mode 100644 drivers/soc/microchip/Kconfig
create mode 100644 drivers/soc/microchip/Makefile
create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
create mode 100644 include/soc/microchip/mpfs.h
--
2.17.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 0/5] Add support for the PolarFire SoC system controller
@ 2020-12-23 16:32 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:32 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: cyril.jean, david.abdurachmanov, daire.mcnamara, anup.patel,
j.neuschaefer, atish.patra, Conor Dooley, lewis.hanly
From: Conor Dooley <conor.dooley@microchip.com>
This patch series adds support for the system controller on
the PolarFire SoC, using the mailbox framework. A Microchip directory
in the SoC subsystem has been created to hold the mailbox client
driver and will be used for future service drivers.
This directory is included by the kconfig option:
CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
Patra's PolarFire SoC support patches which introduce that option.
It further depends on the MAINTAINERS entry created in the same series.
Changes from v2:
* Further reworked dt bindings to satisfy errors and feedback
(hopefully phandle array is the correct type for the mboxes)
* Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
* Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
* Further simplified driver code from feedback
Changes from v1:
* Squashed header into first patch
* Fixed DT binding warnings & small fixes
* Cleaned up drivers from feedback
Conor Dooley (5):
mbox: add polarfire soc system controller mailbox
dt-bindings: add bindings for polarfire soc mailbox
soc: add polarfire soc system controller
dt-bindings: add bindings for polarfire soc system controller
MAINTAINERS: add maintainers for polarfire soc mailbox driver
.../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
.../microchip,mpfs_sys_controller.yaml | 34 +++
MAINTAINERS | 1 +
drivers/mailbox/Kconfig | 12 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/microchip/Kconfig | 10 +
drivers/soc/microchip/Makefile | 1 +
drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
include/soc/microchip/mpfs.h | 51 ++++
12 files changed, 572 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
create mode 100644 drivers/mailbox/mailbox-mpfs.c
create mode 100644 drivers/soc/microchip/Kconfig
create mode 100644 drivers/soc/microchip/Makefile
create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
create mode 100644 include/soc/microchip/mpfs.h
--
2.17.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
@ 2020-12-23 16:32 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:32 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: lewis.hanly, cyril.jean, daire.mcnamara, atish.patra, anup.patel,
david.abdurachmanov, j.neuschaefer, Conor Dooley
From: Conor Dooley <conor.dooley@microchip.com>
This driver adds support for the single mailbox channel of the MSS
system controller on the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/mailbox/Kconfig | 12 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-mpfs.c | 285 +++++++++++++++++++++++++++++++++
include/soc/microchip/mpfs.h | 51 ++++++
4 files changed, 350 insertions(+)
create mode 100644 drivers/mailbox/mailbox-mpfs.c
create mode 100644 include/soc/microchip/mpfs.h
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 05b1009e2820..9aa18631c50b 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -153,6 +153,18 @@ config MAILBOX_TEST
Test client to help with testing new Controller driver
implementations.
+config POLARFIRE_SOC_MAILBOX
+ tristate "PolarFire SoC (MPFS) Mailbox"
+ depends on HAS_IOMEM
+ depends on SOC_MICROCHIP_POLARFIRE || COMPILE_TEST
+ help
+ This driver adds support for the PolarFire SoC (MPFS) mailbox controller.
+
+ To compile this driver as a module, choose M here. the
+ module will be called mailbox-mpfs.
+
+ If unsure, say N.
+
config QCOM_APCS_IPC
tristate "Qualcomm APCS IPC driver"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2e06e02b2e03..77b79a0c384c 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -39,6 +39,8 @@ obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
obj-$(CONFIG_BCM_FLEXRM_MBOX) += bcm-flexrm-mailbox.o
+obj-$(CONFIG_POLARFIRE_SOC_MAILBOX) += mailbox-mpfs.o
+
obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o
obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o
diff --git a/drivers/mailbox/mailbox-mpfs.c b/drivers/mailbox/mailbox-mpfs.c
new file mode 100644
index 000000000000..b31317a8f44e
--- /dev/null
+++ b/drivers/mailbox/mailbox-mpfs.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip PolarFire SoC (MPFS) system controller/mailbox controller driver
+ *
+ * Copyright (c) 2020 Microchip Corporation. All rights reserved.
+ *
+ * Author: Conor Dooley <conor.dooley@microchip.com>
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/mailbox_controller.h>
+#include <soc/microchip/mpfs.h>
+
+#define SERVICES_CR_OFFSET 0x50u
+#define SERVICES_SR_OFFSET 0x54u
+#define MAILBOX_REG_OFFSET 0x800u
+#define MSS_SYS_BUSY -EBUSY
+#define MSS_SYS_PARAM_ERR -EINVAL
+#define MSS_SYS_MAILBOX_DATA_OFFSET 0u
+#define SCB_MASK_WIDTH 16u
+
+/* SCBCTRL service control register */
+
+#define SCB_CTRL_REQ (0)
+#define SCB_CTRL_REQ_MASK BIT(SCB_CTRL_REQ)
+
+#define SCB_CTRL_BUSY (1)
+#define SCB_CTRL_BUSY_MASK BIT(SCB_CTRL_BUSY)
+
+#define SCB_CTRL_ABORT (2)
+#define SCB_CTRL_ABORT_MASK BIT(SCB_CTRL_ABORT)
+
+#define SCB_CTRL_NOTIFY (3)
+#define SCB_CTRL_NOTIFY_MASK BIT(SCB_CTRL_NOTIFY)
+
+#define SCB_CTRL_POS (16)
+#define SCB_CTRL_MASK GENMASK(SCB_CTRL_POS + SCB_MASK_WIDTH, SCB_CTRL_POS)
+
+/* SCBCTRL service status registers */
+
+#define SCB_STATUS_REQ (0)
+#define SCB_STATUS_REQ_MASK BIT(SCB_STATUS_REQ)
+
+#define SCB_STATUS_BUSY (1)
+#define SCB_STATUS_BUSY_MASK BIT(SCB_STATUS_BUSY)
+
+#define SCB_STATUS_ABORT (2)
+#define SCB_STATUS_ABORT_MASK BIT(SCB_STATUS_ABORT)
+
+#define SCB_STATUS_NOTIFY (3)
+#define SCB_STATUS_NOTIFY_MASK BIT(SCB_STATUS_NOTIFY)
+
+#define SCB_STATUS_POS (16)
+#define SCB_STATUS_MASK GENMASK(SCB_STATUS_POS + SCB_MASK_WIDTH, SCB_STATUS_POS)
+
+struct mpfs_mbox {
+ struct mbox_controller controller;
+ struct device *dev;
+ int irq;
+ void __iomem *mailbox_base;
+ void __iomem *int_reg;
+ struct mbox_chan *chan;
+ u16 response_size;
+ u16 response_offset;
+};
+
+static bool mpfs_mbox_busy(struct mpfs_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
+
+ return status & SCB_STATUS_BUSY_MASK;
+}
+
+static struct mpfs_mbox *mbox_chan_to_mpfs_mbox(struct mbox_chan *chan)
+{
+ if (!chan)
+ return NULL;
+
+ return (struct mpfs_mbox *)chan->con_priv;
+}
+
+static int mpfs_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+ u32 index;
+ u32 *word_buf;
+ u8 *byte_buf;
+ u8 byte_off;
+ u8 extra_bits;
+ u8 i;
+ u32 mailbox_val = 0u;
+ u16 mbox_offset;
+ u16 mbox_options_select;
+ u32 mbox_tx_trigger;
+ struct mpfs_mss_msg *msg = data;
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ mbox_offset = msg->mailbox_offset;
+ mbox->response_size = msg->response_size;
+ mbox->response_offset = msg->response_offset;
+
+ if (mpfs_mbox_busy(mbox))
+ return MSS_SYS_BUSY;
+
+ mbox_options_select = ((mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu));
+ mbox_tx_trigger = (((mbox_options_select << SCB_CTRL_POS) &
+ SCB_CTRL_MASK) | SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK);
+
+ /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
+
+ writel_relaxed(0, mbox->int_reg);
+
+ if (msg->cmd_data_size) {
+ byte_buf = msg->cmd_data;
+
+ for (index = 0; index < (msg->cmd_data_size / 4); index++)
+ writel_relaxed(word_buf[index],
+ mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
+ extra_bits = msg->cmd_data_size & 3;
+ if (extra_bits) {
+ byte_off = ALIGN_DOWN(msg->cmd_data_size, 4);
+ byte_buf = msg->cmd_data + byte_off;
+
+ mailbox_val = readl_relaxed(mbox->mailbox_base +
+ MAILBOX_REG_OFFSET + index);
+
+ for (i = 0u; i < extra_bits; i++) {
+ mailbox_val &= ~(0xffu << (i * 8u));
+ mailbox_val |= (byte_buf[i] << (i * 8u));
+ }
+
+ writel_relaxed(mailbox_val,
+ mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
+ }
+ }
+
+ writel_relaxed(mbox_tx_trigger, mbox->mailbox_base + SERVICES_CR_OFFSET);
+
+ return 0;
+}
+
+static inline bool mpfs_mbox_pending(struct mpfs_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
+
+ return !(status & SCB_STATUS_BUSY_MASK);
+}
+
+static void mpfs_mbox_rx_data(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+ u32 *data;
+ u32 i;
+ u32 response_limit;
+
+ data = devm_kzalloc(mbox->dev, sizeof(*data) * mbox->response_size, GFP_ATOMIC);
+ if (!data)
+ dev_err(mbox->dev, "failed to assign memory for response\n", -ENOMEM);
+
+ response_limit = mbox->response_size + mbox->response_offset;
+ if (mpfs_mbox_pending(mbox) && mbox->response_size > 0U) {
+ for (i = mbox->response_offset; i < response_limit; i++) {
+ data[i - mbox->response_offset] =
+ readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET + i * 0x4);
+ }
+ }
+
+ mbox_chan_received_data(chan, (void *)data);
+ devm_kfree(mbox->dev, data);
+}
+
+static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)data;
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ writel_relaxed(0, mbox->int_reg);
+
+ mpfs_mbox_rx_data(chan);
+
+ mbox_chan_txdone(chan, 0);
+ return IRQ_HANDLED;
+}
+
+static int mpfs_mbox_startup(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+ int ret = 0;
+
+ if (!mbox)
+ return -EINVAL;
+ ret = devm_request_irq(mbox->dev, mbox->irq, mpfs_mbox_inbox_isr, 0, "mpfs-mailbox", chan);
+ if (ret)
+ dev_err(mbox->dev, "failed to register mailbox interrupt:%d\n", ret);
+
+ return ret;
+}
+
+static void mpfs_mbox_shutdown(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ devm_free_irq(mbox->dev, mbox->irq, chan);
+}
+
+static const struct mbox_chan_ops mpfs_mbox_ops = {
+ .send_data = mpfs_mbox_send_data,
+ .startup = mpfs_mbox_startup,
+ .shutdown = mpfs_mbox_shutdown,
+};
+
+static int mpfs_mbox_probe(struct platform_device *pdev)
+{
+ struct mpfs_mbox *mbox;
+ struct resource *regs;
+ struct mbox_chan *chans;
+ int ret;
+
+ mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ chans = devm_kzalloc(&pdev->dev, sizeof(*chans), GFP_KERNEL);
+ if (!chans)
+ return -ENOMEM;
+
+ mbox->mailbox_base = devm_platform_get_and_ioremap_resource(pdev, 0, ®s);
+ if (IS_ERR(mbox->mailbox_base))
+ return PTR_ERR(mbox->mailbox_base);
+
+ mbox->int_reg = devm_platform_get_and_ioremap_resource(pdev, 1, ®s);
+ if (IS_ERR(mbox->int_reg))
+ return PTR_ERR(mbox->int_reg);
+
+ mbox->irq = platform_get_irq(pdev, 0);
+ if (mbox->irq < 0)
+ return mbox->irq;
+
+ mbox->dev = &pdev->dev;
+
+ chans[0].con_priv = mbox;
+ mbox->controller.dev = mbox->dev;
+ mbox->controller.num_chans = 1;
+ mbox->controller.chans = chans;
+ mbox->controller.ops = &mpfs_mbox_ops;
+ mbox->controller.txdone_irq = true;
+
+ ret = devm_mbox_controller_register(&pdev->dev, &mbox->controller);
+ if (ret) {
+ dev_err(&pdev->dev, "Registering MPFS mailbox controller failed\n");
+ return ret;
+ }
+ dev_info(&pdev->dev, "Registered MPFS mailbox controller driver\n");
+
+ return 0;
+}
+
+static const struct of_device_id mpfs_mbox_of_match[] = {
+ {.compatible = "microchip,polarfire-soc-mailbox", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);
+
+static struct platform_driver mpfs_mbox_driver = {
+ .driver = {
+ .name = "mpfs-mailbox",
+ .of_match_table = mpfs_mbox_of_match,
+ },
+ .probe = mpfs_mbox_probe,
+};
+module_platform_driver(mpfs_mbox_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
+MODULE_DESCRIPTION("MPFS mailbox controller driver");
diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
new file mode 100644
index 000000000000..4a9d335154a1
--- /dev/null
+++ b/include/soc/microchip/mpfs.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * Microchip PolarFire SoC (MPFS) mailbox
+ *
+ * Copyright (c) 2020 Microchip Corporation. All rights reserved.
+ *
+ * Author: Conor Dooley <conor.dooley@microchip.com>
+ *
+ */
+
+#ifndef __SOC_MPFS_H__
+#define __SOC_MPFS_H__
+
+#include <linux/types.h>
+#include <linux/of_device.h>
+
+struct mpfs_sys_controller;
+
+struct mpfs_mss_msg {
+ u8 cmd_opcode;
+ u16 cmd_data_size;
+ u16 response_size;
+ u8 *cmd_data;
+ u16 mailbox_offset;
+ u16 response_offset;
+};
+
+#if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL)
+
+int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg,
+ void *response, u16 response_size_bytes);
+
+struct mpfs_sys_controller *mpfs_sys_controller_get(struct device_node *mailbox_node);
+
+#else
+
+static int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg,
+ void *response, u16 response_size_bytes)
+{
+ return -ENOSYS;
+}
+
+struct mpfs_sys_controller *mpfs_sys_controller_get(struct device_node *mailbox_node)
+{
+ return NULL;
+}
+
+#endif /* if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL) */
+
+#endif /* __SOC_MPFS_H__ */
--
2.17.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
@ 2020-12-23 16:32 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:32 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: cyril.jean, david.abdurachmanov, daire.mcnamara, anup.patel,
j.neuschaefer, atish.patra, Conor Dooley, lewis.hanly
From: Conor Dooley <conor.dooley@microchip.com>
This driver adds support for the single mailbox channel of the MSS
system controller on the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/mailbox/Kconfig | 12 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-mpfs.c | 285 +++++++++++++++++++++++++++++++++
include/soc/microchip/mpfs.h | 51 ++++++
4 files changed, 350 insertions(+)
create mode 100644 drivers/mailbox/mailbox-mpfs.c
create mode 100644 include/soc/microchip/mpfs.h
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 05b1009e2820..9aa18631c50b 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -153,6 +153,18 @@ config MAILBOX_TEST
Test client to help with testing new Controller driver
implementations.
+config POLARFIRE_SOC_MAILBOX
+ tristate "PolarFire SoC (MPFS) Mailbox"
+ depends on HAS_IOMEM
+ depends on SOC_MICROCHIP_POLARFIRE || COMPILE_TEST
+ help
+ This driver adds support for the PolarFire SoC (MPFS) mailbox controller.
+
+ To compile this driver as a module, choose M here. the
+ module will be called mailbox-mpfs.
+
+ If unsure, say N.
+
config QCOM_APCS_IPC
tristate "Qualcomm APCS IPC driver"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2e06e02b2e03..77b79a0c384c 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -39,6 +39,8 @@ obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
obj-$(CONFIG_BCM_FLEXRM_MBOX) += bcm-flexrm-mailbox.o
+obj-$(CONFIG_POLARFIRE_SOC_MAILBOX) += mailbox-mpfs.o
+
obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o
obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o
diff --git a/drivers/mailbox/mailbox-mpfs.c b/drivers/mailbox/mailbox-mpfs.c
new file mode 100644
index 000000000000..b31317a8f44e
--- /dev/null
+++ b/drivers/mailbox/mailbox-mpfs.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip PolarFire SoC (MPFS) system controller/mailbox controller driver
+ *
+ * Copyright (c) 2020 Microchip Corporation. All rights reserved.
+ *
+ * Author: Conor Dooley <conor.dooley@microchip.com>
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/mailbox_controller.h>
+#include <soc/microchip/mpfs.h>
+
+#define SERVICES_CR_OFFSET 0x50u
+#define SERVICES_SR_OFFSET 0x54u
+#define MAILBOX_REG_OFFSET 0x800u
+#define MSS_SYS_BUSY -EBUSY
+#define MSS_SYS_PARAM_ERR -EINVAL
+#define MSS_SYS_MAILBOX_DATA_OFFSET 0u
+#define SCB_MASK_WIDTH 16u
+
+/* SCBCTRL service control register */
+
+#define SCB_CTRL_REQ (0)
+#define SCB_CTRL_REQ_MASK BIT(SCB_CTRL_REQ)
+
+#define SCB_CTRL_BUSY (1)
+#define SCB_CTRL_BUSY_MASK BIT(SCB_CTRL_BUSY)
+
+#define SCB_CTRL_ABORT (2)
+#define SCB_CTRL_ABORT_MASK BIT(SCB_CTRL_ABORT)
+
+#define SCB_CTRL_NOTIFY (3)
+#define SCB_CTRL_NOTIFY_MASK BIT(SCB_CTRL_NOTIFY)
+
+#define SCB_CTRL_POS (16)
+#define SCB_CTRL_MASK GENMASK(SCB_CTRL_POS + SCB_MASK_WIDTH, SCB_CTRL_POS)
+
+/* SCBCTRL service status registers */
+
+#define SCB_STATUS_REQ (0)
+#define SCB_STATUS_REQ_MASK BIT(SCB_STATUS_REQ)
+
+#define SCB_STATUS_BUSY (1)
+#define SCB_STATUS_BUSY_MASK BIT(SCB_STATUS_BUSY)
+
+#define SCB_STATUS_ABORT (2)
+#define SCB_STATUS_ABORT_MASK BIT(SCB_STATUS_ABORT)
+
+#define SCB_STATUS_NOTIFY (3)
+#define SCB_STATUS_NOTIFY_MASK BIT(SCB_STATUS_NOTIFY)
+
+#define SCB_STATUS_POS (16)
+#define SCB_STATUS_MASK GENMASK(SCB_STATUS_POS + SCB_MASK_WIDTH, SCB_STATUS_POS)
+
+struct mpfs_mbox {
+ struct mbox_controller controller;
+ struct device *dev;
+ int irq;
+ void __iomem *mailbox_base;
+ void __iomem *int_reg;
+ struct mbox_chan *chan;
+ u16 response_size;
+ u16 response_offset;
+};
+
+static bool mpfs_mbox_busy(struct mpfs_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
+
+ return status & SCB_STATUS_BUSY_MASK;
+}
+
+static struct mpfs_mbox *mbox_chan_to_mpfs_mbox(struct mbox_chan *chan)
+{
+ if (!chan)
+ return NULL;
+
+ return (struct mpfs_mbox *)chan->con_priv;
+}
+
+static int mpfs_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+ u32 index;
+ u32 *word_buf;
+ u8 *byte_buf;
+ u8 byte_off;
+ u8 extra_bits;
+ u8 i;
+ u32 mailbox_val = 0u;
+ u16 mbox_offset;
+ u16 mbox_options_select;
+ u32 mbox_tx_trigger;
+ struct mpfs_mss_msg *msg = data;
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ mbox_offset = msg->mailbox_offset;
+ mbox->response_size = msg->response_size;
+ mbox->response_offset = msg->response_offset;
+
+ if (mpfs_mbox_busy(mbox))
+ return MSS_SYS_BUSY;
+
+ mbox_options_select = ((mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu));
+ mbox_tx_trigger = (((mbox_options_select << SCB_CTRL_POS) &
+ SCB_CTRL_MASK) | SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK);
+
+ /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
+
+ writel_relaxed(0, mbox->int_reg);
+
+ if (msg->cmd_data_size) {
+ byte_buf = msg->cmd_data;
+
+ for (index = 0; index < (msg->cmd_data_size / 4); index++)
+ writel_relaxed(word_buf[index],
+ mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
+ extra_bits = msg->cmd_data_size & 3;
+ if (extra_bits) {
+ byte_off = ALIGN_DOWN(msg->cmd_data_size, 4);
+ byte_buf = msg->cmd_data + byte_off;
+
+ mailbox_val = readl_relaxed(mbox->mailbox_base +
+ MAILBOX_REG_OFFSET + index);
+
+ for (i = 0u; i < extra_bits; i++) {
+ mailbox_val &= ~(0xffu << (i * 8u));
+ mailbox_val |= (byte_buf[i] << (i * 8u));
+ }
+
+ writel_relaxed(mailbox_val,
+ mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
+ }
+ }
+
+ writel_relaxed(mbox_tx_trigger, mbox->mailbox_base + SERVICES_CR_OFFSET);
+
+ return 0;
+}
+
+static inline bool mpfs_mbox_pending(struct mpfs_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
+
+ return !(status & SCB_STATUS_BUSY_MASK);
+}
+
+static void mpfs_mbox_rx_data(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+ u32 *data;
+ u32 i;
+ u32 response_limit;
+
+ data = devm_kzalloc(mbox->dev, sizeof(*data) * mbox->response_size, GFP_ATOMIC);
+ if (!data)
+ dev_err(mbox->dev, "failed to assign memory for response\n", -ENOMEM);
+
+ response_limit = mbox->response_size + mbox->response_offset;
+ if (mpfs_mbox_pending(mbox) && mbox->response_size > 0U) {
+ for (i = mbox->response_offset; i < response_limit; i++) {
+ data[i - mbox->response_offset] =
+ readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET + i * 0x4);
+ }
+ }
+
+ mbox_chan_received_data(chan, (void *)data);
+ devm_kfree(mbox->dev, data);
+}
+
+static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)data;
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ writel_relaxed(0, mbox->int_reg);
+
+ mpfs_mbox_rx_data(chan);
+
+ mbox_chan_txdone(chan, 0);
+ return IRQ_HANDLED;
+}
+
+static int mpfs_mbox_startup(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+ int ret = 0;
+
+ if (!mbox)
+ return -EINVAL;
+ ret = devm_request_irq(mbox->dev, mbox->irq, mpfs_mbox_inbox_isr, 0, "mpfs-mailbox", chan);
+ if (ret)
+ dev_err(mbox->dev, "failed to register mailbox interrupt:%d\n", ret);
+
+ return ret;
+}
+
+static void mpfs_mbox_shutdown(struct mbox_chan *chan)
+{
+ struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
+
+ devm_free_irq(mbox->dev, mbox->irq, chan);
+}
+
+static const struct mbox_chan_ops mpfs_mbox_ops = {
+ .send_data = mpfs_mbox_send_data,
+ .startup = mpfs_mbox_startup,
+ .shutdown = mpfs_mbox_shutdown,
+};
+
+static int mpfs_mbox_probe(struct platform_device *pdev)
+{
+ struct mpfs_mbox *mbox;
+ struct resource *regs;
+ struct mbox_chan *chans;
+ int ret;
+
+ mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ chans = devm_kzalloc(&pdev->dev, sizeof(*chans), GFP_KERNEL);
+ if (!chans)
+ return -ENOMEM;
+
+ mbox->mailbox_base = devm_platform_get_and_ioremap_resource(pdev, 0, ®s);
+ if (IS_ERR(mbox->mailbox_base))
+ return PTR_ERR(mbox->mailbox_base);
+
+ mbox->int_reg = devm_platform_get_and_ioremap_resource(pdev, 1, ®s);
+ if (IS_ERR(mbox->int_reg))
+ return PTR_ERR(mbox->int_reg);
+
+ mbox->irq = platform_get_irq(pdev, 0);
+ if (mbox->irq < 0)
+ return mbox->irq;
+
+ mbox->dev = &pdev->dev;
+
+ chans[0].con_priv = mbox;
+ mbox->controller.dev = mbox->dev;
+ mbox->controller.num_chans = 1;
+ mbox->controller.chans = chans;
+ mbox->controller.ops = &mpfs_mbox_ops;
+ mbox->controller.txdone_irq = true;
+
+ ret = devm_mbox_controller_register(&pdev->dev, &mbox->controller);
+ if (ret) {
+ dev_err(&pdev->dev, "Registering MPFS mailbox controller failed\n");
+ return ret;
+ }
+ dev_info(&pdev->dev, "Registered MPFS mailbox controller driver\n");
+
+ return 0;
+}
+
+static const struct of_device_id mpfs_mbox_of_match[] = {
+ {.compatible = "microchip,polarfire-soc-mailbox", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);
+
+static struct platform_driver mpfs_mbox_driver = {
+ .driver = {
+ .name = "mpfs-mailbox",
+ .of_match_table = mpfs_mbox_of_match,
+ },
+ .probe = mpfs_mbox_probe,
+};
+module_platform_driver(mpfs_mbox_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
+MODULE_DESCRIPTION("MPFS mailbox controller driver");
diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
new file mode 100644
index 000000000000..4a9d335154a1
--- /dev/null
+++ b/include/soc/microchip/mpfs.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * Microchip PolarFire SoC (MPFS) mailbox
+ *
+ * Copyright (c) 2020 Microchip Corporation. All rights reserved.
+ *
+ * Author: Conor Dooley <conor.dooley@microchip.com>
+ *
+ */
+
+#ifndef __SOC_MPFS_H__
+#define __SOC_MPFS_H__
+
+#include <linux/types.h>
+#include <linux/of_device.h>
+
+struct mpfs_sys_controller;
+
+struct mpfs_mss_msg {
+ u8 cmd_opcode;
+ u16 cmd_data_size;
+ u16 response_size;
+ u8 *cmd_data;
+ u16 mailbox_offset;
+ u16 response_offset;
+};
+
+#if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL)
+
+int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg,
+ void *response, u16 response_size_bytes);
+
+struct mpfs_sys_controller *mpfs_sys_controller_get(struct device_node *mailbox_node);
+
+#else
+
+static int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg,
+ void *response, u16 response_size_bytes)
+{
+ return -ENOSYS;
+}
+
+struct mpfs_sys_controller *mpfs_sys_controller_get(struct device_node *mailbox_node)
+{
+ return NULL;
+}
+
+#endif /* if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL) */
+
+#endif /* __SOC_MPFS_H__ */
--
2.17.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
@ 2020-12-23 16:33 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:33 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: lewis.hanly, cyril.jean, daire.mcnamara, atish.patra, anup.patel,
david.abdurachmanov, j.neuschaefer, Conor Dooley
From: Conor Dooley <conor.dooley@microchip.com>
Add device tree bindings for the MSS system controller mailbox on
the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../mailbox/microchip,mailbox-mpfs.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
diff --git a/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml b/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
new file mode 100644
index 000000000000..c5b126c6fb66
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/microchip,mailbox-mpfs.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip PolarFire SoC (MPFS) mss mailbox controller
+
+maintainers:
+ - Conor Dooley <conor.dooley@microchip.com>
+
+properties:
+ compatible:
+ const: microchip,polarfire-soc-mailbox
+
+ reg:
+ items:
+ - description: mailbox data registers
+ - description: mailbox int registers
+
+ interrupts:
+ maxItems: 1
+
+ "#mbox-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ mbox: mailbox@37020000 {
+ compatible = "microchip,polarfire-soc-mailbox";
+ reg = <0x0 0x37020000 0x0 0x1000>, <0x0 0x2000318c 0x0 0x40>;
+ interrupt-parent = <&L1>;
+ interrupts = <96>;
+ #mbox-cells = <1>;
+ };
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
@ 2020-12-23 16:33 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:33 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: cyril.jean, david.abdurachmanov, daire.mcnamara, anup.patel,
j.neuschaefer, atish.patra, Conor Dooley, lewis.hanly
From: Conor Dooley <conor.dooley@microchip.com>
Add device tree bindings for the MSS system controller mailbox on
the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../mailbox/microchip,mailbox-mpfs.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
diff --git a/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml b/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
new file mode 100644
index 000000000000..c5b126c6fb66
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/microchip,mailbox-mpfs.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip PolarFire SoC (MPFS) mss mailbox controller
+
+maintainers:
+ - Conor Dooley <conor.dooley@microchip.com>
+
+properties:
+ compatible:
+ const: microchip,polarfire-soc-mailbox
+
+ reg:
+ items:
+ - description: mailbox data registers
+ - description: mailbox int registers
+
+ interrupts:
+ maxItems: 1
+
+ "#mbox-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ mbox: mailbox@37020000 {
+ compatible = "microchip,polarfire-soc-mailbox";
+ reg = <0x0 0x37020000 0x0 0x1000>, <0x0 0x2000318c 0x0 0x40>;
+ interrupt-parent = <&L1>;
+ interrupts = <96>;
+ #mbox-cells = <1>;
+ };
+ };
--
2.17.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
@ 2020-12-23 16:33 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:33 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: lewis.hanly, cyril.jean, daire.mcnamara, atish.patra, anup.patel,
david.abdurachmanov, j.neuschaefer, Conor Dooley
From: Conor Dooley <conor.dooley@microchip.com>
Add device tree bindings for the MSS system controller on
the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
new file mode 100644
index 000000000000..60fde1925910
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip PolarFire SoC (MPFS) system controller
+
+maintainers:
+ - Conor Dooley <conor.dooley@microchip.com>
+
+properties:
+ compatible:
+ const: microchip,polarfire-soc-sys-controller
+
+ mboxes:
+ $ref: '/schemas/types.yaml#/definitions/phandle-array'
+ description: |
+ phandle and index of the mailbox controller device node.
+ See: ../mailbox/mailbox.txt for more information.
+ maxItems: 1
+
+required:
+ - compatible
+ - "mboxes"
+
+additionalProperties: false
+
+examples:
+ - |
+ syscontroller: syscontroller {
+ compatible = "microchip,polarfire-soc-sys-controller";
+ mboxes = <&mbox 0>;
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
@ 2020-12-23 16:33 ` conor.dooley
0 siblings, 0 replies; 26+ messages in thread
From: conor.dooley @ 2020-12-23 16:33 UTC (permalink / raw)
To: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv
Cc: cyril.jean, david.abdurachmanov, daire.mcnamara, anup.patel,
j.neuschaefer, atish.patra, Conor Dooley, lewis.hanly
From: Conor Dooley <conor.dooley@microchip.com>
Add device tree bindings for the MSS system controller on
the Microchip PolarFire SoC.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
new file mode 100644
index 000000000000..60fde1925910
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip PolarFire SoC (MPFS) system controller
+
+maintainers:
+ - Conor Dooley <conor.dooley@microchip.com>
+
+properties:
+ compatible:
+ const: microchip,polarfire-soc-sys-controller
+
+ mboxes:
+ $ref: '/schemas/types.yaml#/definitions/phandle-array'
+ description: |
+ phandle and index of the mailbox controller device node.
+ See: ../mailbox/mailbox.txt for more information.
+ maxItems: 1
+
+required:
+ - compatible
+ - "mboxes"
+
+additionalProperties: false
+
+examples:
+ - |
+ syscontroller: syscontroller {
+ compatible = "microchip,polarfire-soc-sys-controller";
+ mboxes = <&mbox 0>;
+ };
--
2.17.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
2020-12-23 16:33 ` conor.dooley
@ 2020-12-31 18:34 ` Rob Herring
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-12-31 18:34 UTC (permalink / raw)
To: conor.dooley
Cc: damien.lemoal, devicetree, daire.mcnamara, paul.walmsley,
linux-riscv, david.abdurachmanov, aou, atish.patra, lewis.hanly,
j.neuschaefer, cyril.jean, anup.patel, robh+dt, jassisinghbrar,
palmer
On Wed, 23 Dec 2020 16:33:02 +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller mailbox on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
@ 2020-12-31 18:34 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-12-31 18:34 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, damien.lemoal, aou, lewis.hanly, david.abdurachmanov,
daire.mcnamara, anup.patel, j.neuschaefer, atish.patra, robh+dt,
palmer, paul.walmsley, cyril.jean, linux-riscv, jassisinghbrar
On Wed, 23 Dec 2020 16:33:02 +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller mailbox on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
2020-12-23 16:33 ` conor.dooley
@ 2021-01-02 11:30 ` Jonathan Neuschäfer
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-02 11:30 UTC (permalink / raw)
To: conor.dooley
Cc: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv, lewis.hanly, cyril.jean,
daire.mcnamara, atish.patra, anup.patel, david.abdurachmanov,
j.neuschaefer
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]
On Wed, Dec 23, 2020 at 04:33:02PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller mailbox on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
[...]
> + reg:
> + items:
> + - description: mailbox data registers
> + - description: mailbox int registers
Does "int registers" mean "interrupt registers", i.e. do they control
some aspect of interrupt delivery?
IMHO, it would be good to spell it out, because "int" could mean
different things.
Best regards, happy new year,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox
@ 2021-01-02 11:30 ` Jonathan Neuschäfer
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-02 11:30 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, damien.lemoal, aou, lewis.hanly, david.abdurachmanov,
daire.mcnamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, robh+dt, palmer, paul.walmsley, cyril.jean,
linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 685 bytes --]
On Wed, Dec 23, 2020 at 04:33:02PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller mailbox on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
[...]
> + reg:
> + items:
> + - description: mailbox data registers
> + - description: mailbox int registers
Does "int registers" mean "interrupt registers", i.e. do they control
some aspect of interrupt delivery?
IMHO, it would be good to spell it out, because "int" could mean
different things.
Best regards, happy new year,
Jonathan Neuschäfer
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
2020-12-23 16:32 ` conor.dooley
@ 2021-01-02 13:01 ` Jonathan Neuschäfer
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-02 13:01 UTC (permalink / raw)
To: conor.dooley
Cc: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv, lewis.hanly, cyril.jean,
daire.mcnamara, atish.patra, anup.patel, david.abdurachmanov,
j.neuschaefer
[-- Attachment #1: Type: text/plain, Size: 8544 bytes --]
Hello,
I've added review comments below. Some of them might be more detailed
than necessary, and reflect my opinion rather than something that must
be fixed. Anyway, I hope my comments make sense.
On Wed, Dec 23, 2020 at 04:32:55PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> This driver adds support for the single mailbox channel of the MSS
> system controller on the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
[...]
> +#define SERVICES_CR_OFFSET 0x50u
> +#define SERVICES_SR_OFFSET 0x54u
> +#define MAILBOX_REG_OFFSET 0x800u
> +#define MSS_SYS_BUSY -EBUSY
> +#define MSS_SYS_PARAM_ERR -EINVAL
Defining your own aliases for -Esomething is rather uncommon.
> +#define MSS_SYS_MAILBOX_DATA_OFFSET 0u
> +#define SCB_MASK_WIDTH 16u
> +
> +/* SCBCTRL service control register */
> +
> +#define SCB_CTRL_REQ (0)
> +#define SCB_CTRL_REQ_MASK BIT(SCB_CTRL_REQ)
> +
> +#define SCB_CTRL_BUSY (1)
> +#define SCB_CTRL_BUSY_MASK BIT(SCB_CTRL_BUSY)
> +
> +#define SCB_CTRL_ABORT (2)
> +#define SCB_CTRL_ABORT_MASK BIT(SCB_CTRL_ABORT)
> +
> +#define SCB_CTRL_NOTIFY (3)
> +#define SCB_CTRL_NOTIFY_MASK BIT(SCB_CTRL_NOTIFY)
> +
> +#define SCB_CTRL_POS (16)
> +#define SCB_CTRL_MASK GENMASK(SCB_CTRL_POS + SCB_MASK_WIDTH, SCB_CTRL_POS)
> +
> +/* SCBCTRL service status registers */
registers -> register ?
It seems to be only one status register.
> +
> +#define SCB_STATUS_REQ (0)
> +#define SCB_STATUS_REQ_MASK BIT(SCB_STATUS_REQ)
> +
> +#define SCB_STATUS_BUSY (1)
> +#define SCB_STATUS_BUSY_MASK BIT(SCB_STATUS_BUSY)
> +
> +#define SCB_STATUS_ABORT (2)
> +#define SCB_STATUS_ABORT_MASK BIT(SCB_STATUS_ABORT)
> +
> +#define SCB_STATUS_NOTIFY (3)
> +#define SCB_STATUS_NOTIFY_MASK BIT(SCB_STATUS_NOTIFY)
> +
> +#define SCB_STATUS_POS (16)
> +#define SCB_STATUS_MASK GENMASK(SCB_STATUS_POS + SCB_MASK_WIDTH, SCB_STATUS_POS)
> +
> +struct mpfs_mbox {
> + struct mbox_controller controller;
> + struct device *dev;
> + int irq;
> + void __iomem *mailbox_base;
> + void __iomem *int_reg;
> + struct mbox_chan *chan;
> + u16 response_size;
> + u16 response_offset;
> +};
> +
> +static bool mpfs_mbox_busy(struct mpfs_mbox *mbox)
> +{
> + u32 status;
> +
> + status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
> +
> + return status & SCB_STATUS_BUSY_MASK;
> +}
> +
> +static struct mpfs_mbox *mbox_chan_to_mpfs_mbox(struct mbox_chan *chan)
> +{
> + if (!chan)
> + return NULL;
> +
> + return (struct mpfs_mbox *)chan->con_priv;
> +}
> +
> +static int mpfs_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> + u32 index;
> + u32 *word_buf;
> + u8 *byte_buf;
> + u8 byte_off;
> + u8 extra_bits;
> + u8 i;
> + u32 mailbox_val = 0u;
> + u16 mbox_offset;
> + u16 mbox_options_select;
> + u32 mbox_tx_trigger;
the mbox_ prefix seems unnecessary because the whole driver deals with
a mailbox.
Some of the variables are only used in if/for scopes below. I'd move
their declaration down into these scopes, to make the outer scope of the
function a little easier to understand.
> + struct mpfs_mss_msg *msg = data;
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> +
> + mbox_offset = msg->mailbox_offset;
mbox_offset is only used once. The indirection of storing the value in
another variable makes the code slightly slower to read, IMO.
> + mbox->response_size = msg->response_size;
> + mbox->response_offset = msg->response_offset;
> +
> + if (mpfs_mbox_busy(mbox))
> + return MSS_SYS_BUSY;
> +
> + mbox_options_select = ((mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu));
> + mbox_tx_trigger = (((mbox_options_select << SCB_CTRL_POS) &
> + SCB_CTRL_MASK) | SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK);
Slightly reformatted, you could save a few parentheses:
mbox_options_select = (mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu);
mbox_tx_trigger = (mbox_options_select << SCB_CTRL_POS) & SCB_CTRL_MASK;
mbox_tx_trigger |= SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK;
> + /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
What is the "code for MSS_SYS_PARAM_ERR" semantically? Input validation?
> + writel_relaxed(0, mbox->int_reg);
What does a write to mbox->int_reg do? Does it acknowledge an interrupt?
It would be interesting to have an explaining comment here.
> +
> + if (msg->cmd_data_size) {
> + byte_buf = msg->cmd_data;
> +
> + for (index = 0; index < (msg->cmd_data_size / 4); index++)
> + writel_relaxed(word_buf[index],
> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
word_buf is uninitialized.
In mpfs_mbox_rx_data, you access the registers at mbox->mailbox_base +
MAILBOX_REG_OFFSET in steps of four bytes, here you increment the offset
in steps of one byte, because the index isn't scaled. This seems wrong.
> + extra_bits = msg->cmd_data_size & 3;
> + if (extra_bits) {
> + byte_off = ALIGN_DOWN(msg->cmd_data_size, 4);
> + byte_buf = msg->cmd_data + byte_off;
> +
> + mailbox_val = readl_relaxed(mbox->mailbox_base +
> + MAILBOX_REG_OFFSET + index);
> +
> + for (i = 0u; i < extra_bits; i++) {
> + mailbox_val &= ~(0xffu << (i * 8u));
> + mailbox_val |= (byte_buf[i] << (i * 8u));
> + }
> +
> + writel_relaxed(mailbox_val,
> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
> + }
> + }
> +
I'd move the calculation of mbox_tx_trigger down here, right before its
use, because it's not necessary for the code above (if I'm reading it
correctly).
> + writel_relaxed(mbox_tx_trigger, mbox->mailbox_base + SERVICES_CR_OFFSET);
> +
> + return 0;
> +}
> +
> +static inline bool mpfs_mbox_pending(struct mpfs_mbox *mbox)
> +{
> + u32 status;
> +
> + status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
> +
> + return !(status & SCB_STATUS_BUSY_MASK);
> +}
> +
> +static void mpfs_mbox_rx_data(struct mbox_chan *chan)
> +{
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> + u32 *data;
> + u32 i;
> + u32 response_limit;
> +
> + data = devm_kzalloc(mbox->dev, sizeof(*data) * mbox->response_size, GFP_ATOMIC);
The use of devm_ seems bogus here, because the allocated data does not
have the same lifetime as the device; it is allocated and freed for
every RX event.
However, please use kcalloc instead of kzalloc, so you don't have to do
the multiplication manually. kcalloc checks for overflows.
> + if (!data)
> + dev_err(mbox->dev, "failed to assign memory for response\n", -ENOMEM);
There is no format specifier for parameter -ENOMEM.
In the !data case, the code will still run into the loop below, and
probably cause a crash.
> +
> + response_limit = mbox->response_size + mbox->response_offset;
> + if (mpfs_mbox_pending(mbox) && mbox->response_size > 0U) {
> + for (i = mbox->response_offset; i < response_limit; i++) {
> + data[i - mbox->response_offset] =
> + readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET + i * 0x4);
> + }
> + }
It seems slightly easier to me to let the loop run from zero to
mbox->response_size-1. Most importantly, it becomes easy to see that
data is not accessed out of bounds. (But it's your choice)
if (mpfs_mbox_pending(mbox)) {
for (i = 0; i < mbox->response_size; i++) {
data[i] = readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET +
(i + mbox->response_offset) * 0x4);
}
}
> +
> + mbox_chan_received_data(chan, (void *)data);
> + devm_kfree(mbox->dev, data);
> +}
> +
> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)data;
This cast and the one at the end of mpfs_mbox_rx_data are somewhat
uncessary, because C allows implicit conversion of void pointers to and
from other pointer types.
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> +
> + writel_relaxed(0, mbox->int_reg);
> +
> + mpfs_mbox_rx_data(chan);
> +
> + mbox_chan_txdone(chan, 0);
> + return IRQ_HANDLED;
> +}
[...]
> +++ b/include/soc/microchip/mpfs.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + * Microchip PolarFire SoC (MPFS) mailbox
This is mpfs.h, but the comment implies it's only for mailbox-related
code, not for all of the Microchip PolarFire SoC. Is this intentional?
Best regards,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
@ 2021-01-02 13:01 ` Jonathan Neuschäfer
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-02 13:01 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, damien.lemoal, aou, lewis.hanly, david.abdurachmanov,
daire.mcnamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, robh+dt, palmer, paul.walmsley, cyril.jean,
linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 8544 bytes --]
Hello,
I've added review comments below. Some of them might be more detailed
than necessary, and reflect my opinion rather than something that must
be fixed. Anyway, I hope my comments make sense.
On Wed, Dec 23, 2020 at 04:32:55PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> This driver adds support for the single mailbox channel of the MSS
> system controller on the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
[...]
> +#define SERVICES_CR_OFFSET 0x50u
> +#define SERVICES_SR_OFFSET 0x54u
> +#define MAILBOX_REG_OFFSET 0x800u
> +#define MSS_SYS_BUSY -EBUSY
> +#define MSS_SYS_PARAM_ERR -EINVAL
Defining your own aliases for -Esomething is rather uncommon.
> +#define MSS_SYS_MAILBOX_DATA_OFFSET 0u
> +#define SCB_MASK_WIDTH 16u
> +
> +/* SCBCTRL service control register */
> +
> +#define SCB_CTRL_REQ (0)
> +#define SCB_CTRL_REQ_MASK BIT(SCB_CTRL_REQ)
> +
> +#define SCB_CTRL_BUSY (1)
> +#define SCB_CTRL_BUSY_MASK BIT(SCB_CTRL_BUSY)
> +
> +#define SCB_CTRL_ABORT (2)
> +#define SCB_CTRL_ABORT_MASK BIT(SCB_CTRL_ABORT)
> +
> +#define SCB_CTRL_NOTIFY (3)
> +#define SCB_CTRL_NOTIFY_MASK BIT(SCB_CTRL_NOTIFY)
> +
> +#define SCB_CTRL_POS (16)
> +#define SCB_CTRL_MASK GENMASK(SCB_CTRL_POS + SCB_MASK_WIDTH, SCB_CTRL_POS)
> +
> +/* SCBCTRL service status registers */
registers -> register ?
It seems to be only one status register.
> +
> +#define SCB_STATUS_REQ (0)
> +#define SCB_STATUS_REQ_MASK BIT(SCB_STATUS_REQ)
> +
> +#define SCB_STATUS_BUSY (1)
> +#define SCB_STATUS_BUSY_MASK BIT(SCB_STATUS_BUSY)
> +
> +#define SCB_STATUS_ABORT (2)
> +#define SCB_STATUS_ABORT_MASK BIT(SCB_STATUS_ABORT)
> +
> +#define SCB_STATUS_NOTIFY (3)
> +#define SCB_STATUS_NOTIFY_MASK BIT(SCB_STATUS_NOTIFY)
> +
> +#define SCB_STATUS_POS (16)
> +#define SCB_STATUS_MASK GENMASK(SCB_STATUS_POS + SCB_MASK_WIDTH, SCB_STATUS_POS)
> +
> +struct mpfs_mbox {
> + struct mbox_controller controller;
> + struct device *dev;
> + int irq;
> + void __iomem *mailbox_base;
> + void __iomem *int_reg;
> + struct mbox_chan *chan;
> + u16 response_size;
> + u16 response_offset;
> +};
> +
> +static bool mpfs_mbox_busy(struct mpfs_mbox *mbox)
> +{
> + u32 status;
> +
> + status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
> +
> + return status & SCB_STATUS_BUSY_MASK;
> +}
> +
> +static struct mpfs_mbox *mbox_chan_to_mpfs_mbox(struct mbox_chan *chan)
> +{
> + if (!chan)
> + return NULL;
> +
> + return (struct mpfs_mbox *)chan->con_priv;
> +}
> +
> +static int mpfs_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> + u32 index;
> + u32 *word_buf;
> + u8 *byte_buf;
> + u8 byte_off;
> + u8 extra_bits;
> + u8 i;
> + u32 mailbox_val = 0u;
> + u16 mbox_offset;
> + u16 mbox_options_select;
> + u32 mbox_tx_trigger;
the mbox_ prefix seems unnecessary because the whole driver deals with
a mailbox.
Some of the variables are only used in if/for scopes below. I'd move
their declaration down into these scopes, to make the outer scope of the
function a little easier to understand.
> + struct mpfs_mss_msg *msg = data;
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> +
> + mbox_offset = msg->mailbox_offset;
mbox_offset is only used once. The indirection of storing the value in
another variable makes the code slightly slower to read, IMO.
> + mbox->response_size = msg->response_size;
> + mbox->response_offset = msg->response_offset;
> +
> + if (mpfs_mbox_busy(mbox))
> + return MSS_SYS_BUSY;
> +
> + mbox_options_select = ((mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu));
> + mbox_tx_trigger = (((mbox_options_select << SCB_CTRL_POS) &
> + SCB_CTRL_MASK) | SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK);
Slightly reformatted, you could save a few parentheses:
mbox_options_select = (mbox_offset << 7u) | (msg->cmd_opcode & 0x7fu);
mbox_tx_trigger = (mbox_options_select << SCB_CTRL_POS) & SCB_CTRL_MASK;
mbox_tx_trigger |= SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK;
> + /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
What is the "code for MSS_SYS_PARAM_ERR" semantically? Input validation?
> + writel_relaxed(0, mbox->int_reg);
What does a write to mbox->int_reg do? Does it acknowledge an interrupt?
It would be interesting to have an explaining comment here.
> +
> + if (msg->cmd_data_size) {
> + byte_buf = msg->cmd_data;
> +
> + for (index = 0; index < (msg->cmd_data_size / 4); index++)
> + writel_relaxed(word_buf[index],
> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
word_buf is uninitialized.
In mpfs_mbox_rx_data, you access the registers at mbox->mailbox_base +
MAILBOX_REG_OFFSET in steps of four bytes, here you increment the offset
in steps of one byte, because the index isn't scaled. This seems wrong.
> + extra_bits = msg->cmd_data_size & 3;
> + if (extra_bits) {
> + byte_off = ALIGN_DOWN(msg->cmd_data_size, 4);
> + byte_buf = msg->cmd_data + byte_off;
> +
> + mailbox_val = readl_relaxed(mbox->mailbox_base +
> + MAILBOX_REG_OFFSET + index);
> +
> + for (i = 0u; i < extra_bits; i++) {
> + mailbox_val &= ~(0xffu << (i * 8u));
> + mailbox_val |= (byte_buf[i] << (i * 8u));
> + }
> +
> + writel_relaxed(mailbox_val,
> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
> + }
> + }
> +
I'd move the calculation of mbox_tx_trigger down here, right before its
use, because it's not necessary for the code above (if I'm reading it
correctly).
> + writel_relaxed(mbox_tx_trigger, mbox->mailbox_base + SERVICES_CR_OFFSET);
> +
> + return 0;
> +}
> +
> +static inline bool mpfs_mbox_pending(struct mpfs_mbox *mbox)
> +{
> + u32 status;
> +
> + status = readl_relaxed(mbox->mailbox_base + SERVICES_SR_OFFSET);
> +
> + return !(status & SCB_STATUS_BUSY_MASK);
> +}
> +
> +static void mpfs_mbox_rx_data(struct mbox_chan *chan)
> +{
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> + u32 *data;
> + u32 i;
> + u32 response_limit;
> +
> + data = devm_kzalloc(mbox->dev, sizeof(*data) * mbox->response_size, GFP_ATOMIC);
The use of devm_ seems bogus here, because the allocated data does not
have the same lifetime as the device; it is allocated and freed for
every RX event.
However, please use kcalloc instead of kzalloc, so you don't have to do
the multiplication manually. kcalloc checks for overflows.
> + if (!data)
> + dev_err(mbox->dev, "failed to assign memory for response\n", -ENOMEM);
There is no format specifier for parameter -ENOMEM.
In the !data case, the code will still run into the loop below, and
probably cause a crash.
> +
> + response_limit = mbox->response_size + mbox->response_offset;
> + if (mpfs_mbox_pending(mbox) && mbox->response_size > 0U) {
> + for (i = mbox->response_offset; i < response_limit; i++) {
> + data[i - mbox->response_offset] =
> + readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET + i * 0x4);
> + }
> + }
It seems slightly easier to me to let the loop run from zero to
mbox->response_size-1. Most importantly, it becomes easy to see that
data is not accessed out of bounds. (But it's your choice)
if (mpfs_mbox_pending(mbox)) {
for (i = 0; i < mbox->response_size; i++) {
data[i] = readl_relaxed(mbox->mailbox_base + MAILBOX_REG_OFFSET +
(i + mbox->response_offset) * 0x4);
}
}
> +
> + mbox_chan_received_data(chan, (void *)data);
> + devm_kfree(mbox->dev, data);
> +}
> +
> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)data;
This cast and the one at the end of mpfs_mbox_rx_data are somewhat
uncessary, because C allows implicit conversion of void pointers to and
from other pointer types.
> + struct mpfs_mbox *mbox = mbox_chan_to_mpfs_mbox(chan);
> +
> + writel_relaxed(0, mbox->int_reg);
> +
> + mpfs_mbox_rx_data(chan);
> +
> + mbox_chan_txdone(chan, 0);
> + return IRQ_HANDLED;
> +}
[...]
> +++ b/include/soc/microchip/mpfs.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + * Microchip PolarFire SoC (MPFS) mailbox
This is mpfs.h, but the comment implies it's only for mailbox-related
code, not for all of the Microchip PolarFire SoC. Is this intentional?
Best regards,
Jonathan Neuschäfer
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
2020-12-23 16:33 ` conor.dooley
@ 2021-01-05 21:25 ` Jonathan Neuschäfer
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-05 21:25 UTC (permalink / raw)
To: conor.dooley
Cc: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv, lewis.hanly, cyril.jean,
daire.mcnamara, atish.patra, anup.patel, david.abdurachmanov,
j.neuschaefer
[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]
Hi,
On Wed, Dec 23, 2020 at 04:33:16PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller on
> the Microchip PolarFire SoC.
"MSS" doesn't appear below, so perhaps an expansion in parentheses would
be nice, e.g.: "MSS (microprocessor subsystem)"
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> new file mode 100644
> index 000000000000..60fde1925910
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip PolarFire SoC (MPFS) system controller
> +
> +maintainers:
> + - Conor Dooley <conor.dooley@microchip.com>
> +
> +properties:
> + compatible:
> + const: microchip,polarfire-soc-sys-controller
The file name and compatible string are inconsistent in phrasing
(mpfs- vs. polarfire-soc-). If possible, please align them.
> +
> + mboxes:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + phandle and index of the mailbox controller device node.
> + See: ../mailbox/mailbox.txt for more information.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - "mboxes"
Quotation marks are unnecessary here, because there are no special
characters in "mboxes".
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + syscontroller: syscontroller {
> + compatible = "microchip,polarfire-soc-sys-controller";
> + mboxes = <&mbox 0>;
> + };
> --
Best regards,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
@ 2021-01-05 21:25 ` Jonathan Neuschäfer
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-05 21:25 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, damien.lemoal, aou, lewis.hanly, david.abdurachmanov,
daire.mcnamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, robh+dt, palmer, paul.walmsley, cyril.jean,
linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 2285 bytes --]
Hi,
On Wed, Dec 23, 2020 at 04:33:16PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller on
> the Microchip PolarFire SoC.
"MSS" doesn't appear below, so perhaps an expansion in parentheses would
be nice, e.g.: "MSS (microprocessor subsystem)"
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> new file mode 100644
> index 000000000000..60fde1925910
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip PolarFire SoC (MPFS) system controller
> +
> +maintainers:
> + - Conor Dooley <conor.dooley@microchip.com>
> +
> +properties:
> + compatible:
> + const: microchip,polarfire-soc-sys-controller
The file name and compatible string are inconsistent in phrasing
(mpfs- vs. polarfire-soc-). If possible, please align them.
> +
> + mboxes:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + phandle and index of the mailbox controller device node.
> + See: ../mailbox/mailbox.txt for more information.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - "mboxes"
Quotation marks are unnecessary here, because there are no special
characters in "mboxes".
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + syscontroller: syscontroller {
> + compatible = "microchip,polarfire-soc-sys-controller";
> + mboxes = <&mbox 0>;
> + };
> --
Best regards,
Jonathan Neuschäfer
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/5] Add support for the PolarFire SoC system controller
2020-12-23 16:32 ` conor.dooley
@ 2021-01-06 19:25 ` Atish Patra
-1 siblings, 0 replies; 26+ messages in thread
From: Atish Patra @ 2021-01-06 19:25 UTC (permalink / raw)
To: conor.dooley
Cc: Rob Herring, Damien Le Moal, jassisinghbrar, Albert Ou,
Paul Walmsley, Palmer Dabbelt, devicetree, linux-riscv,
cyril.jean, David Abdurachmanov, Daire McNamara, Anup Patel,
j.neuschaefer, Atish Patra, lewis.hanly
On Wed, Dec 23, 2020 at 8:33 AM <conor.dooley@microchip.com> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> This patch series adds support for the system controller on
> the PolarFire SoC, using the mailbox framework. A Microchip directory
> in the SoC subsystem has been created to hold the mailbox client
> driver and will be used for future service drivers.
>
> This directory is included by the kconfig option:
> CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
> Patra's PolarFire SoC support patches which introduce that option.
>
What version of clock driver are you using? I am waiting for an
updated clock patch to post the next
version of the basic soc support series.
> It further depends on the MAINTAINERS entry created in the same series.
>
> Changes from v2:
> * Further reworked dt bindings to satisfy errors and feedback
> (hopefully phandle array is the correct type for the mboxes)
> * Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
> * Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
> * Further simplified driver code from feedback
>
> Changes from v1:
> * Squashed header into first patch
> * Fixed DT binding warnings & small fixes
> * Cleaned up drivers from feedback
>
> Conor Dooley (5):
> mbox: add polarfire soc system controller mailbox
> dt-bindings: add bindings for polarfire soc mailbox
> soc: add polarfire soc system controller
> dt-bindings: add bindings for polarfire soc system controller
> MAINTAINERS: add maintainers for polarfire soc mailbox driver
>
> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
> .../microchip,mpfs_sys_controller.yaml | 34 +++
> MAINTAINERS | 1 +
> drivers/mailbox/Kconfig | 12 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/microchip/Kconfig | 10 +
> drivers/soc/microchip/Makefile | 1 +
> drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
> include/soc/microchip/mpfs.h | 51 ++++
> 12 files changed, 572 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> create mode 100644 drivers/mailbox/mailbox-mpfs.c
> create mode 100644 drivers/soc/microchip/Kconfig
> create mode 100644 drivers/soc/microchip/Makefile
> create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
> create mode 100644 include/soc/microchip/mpfs.h
>
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/5] Add support for the PolarFire SoC system controller
@ 2021-01-06 19:25 ` Atish Patra
0 siblings, 0 replies; 26+ messages in thread
From: Atish Patra @ 2021-01-06 19:25 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, Damien Le Moal, Albert Ou, cyril.jean,
David Abdurachmanov, Daire McNamara, jassisinghbrar,
j.neuschaefer, Atish Patra, Anup Patel, Rob Herring,
Palmer Dabbelt, Paul Walmsley, lewis.hanly, linux-riscv
On Wed, Dec 23, 2020 at 8:33 AM <conor.dooley@microchip.com> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> This patch series adds support for the system controller on
> the PolarFire SoC, using the mailbox framework. A Microchip directory
> in the SoC subsystem has been created to hold the mailbox client
> driver and will be used for future service drivers.
>
> This directory is included by the kconfig option:
> CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
> Patra's PolarFire SoC support patches which introduce that option.
>
What version of clock driver are you using? I am waiting for an
updated clock patch to post the next
version of the basic soc support series.
> It further depends on the MAINTAINERS entry created in the same series.
>
> Changes from v2:
> * Further reworked dt bindings to satisfy errors and feedback
> (hopefully phandle array is the correct type for the mboxes)
> * Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
> * Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
> * Further simplified driver code from feedback
>
> Changes from v1:
> * Squashed header into first patch
> * Fixed DT binding warnings & small fixes
> * Cleaned up drivers from feedback
>
> Conor Dooley (5):
> mbox: add polarfire soc system controller mailbox
> dt-bindings: add bindings for polarfire soc mailbox
> soc: add polarfire soc system controller
> dt-bindings: add bindings for polarfire soc system controller
> MAINTAINERS: add maintainers for polarfire soc mailbox driver
>
> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
> .../microchip,mpfs_sys_controller.yaml | 34 +++
> MAINTAINERS | 1 +
> drivers/mailbox/Kconfig | 12 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/microchip/Kconfig | 10 +
> drivers/soc/microchip/Makefile | 1 +
> drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
> include/soc/microchip/mpfs.h | 51 ++++
> 12 files changed, 572 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> create mode 100644 drivers/mailbox/mailbox-mpfs.c
> create mode 100644 drivers/soc/microchip/Kconfig
> create mode 100644 drivers/soc/microchip/Makefile
> create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
> create mode 100644 include/soc/microchip/mpfs.h
>
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/5] Add support for the PolarFire SoC system controller
2021-01-06 19:25 ` Atish Patra
@ 2021-01-07 11:49 ` Cyril.Jean
-1 siblings, 0 replies; 26+ messages in thread
From: Cyril.Jean @ 2021-01-07 11:49 UTC (permalink / raw)
To: atishp, Conor.Dooley
Cc: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv, david.abdurachmanov,
Daire.McNamara, anup.patel, j.neuschaefer, atish.patra,
Lewis.Hanly
On 1/6/21 7:25 PM, Atish Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Wed, Dec 23, 2020 at 8:33 AM <conor.dooley@microchip.com> wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> This patch series adds support for the system controller on
>> the PolarFire SoC, using the mailbox framework. A Microchip directory
>> in the SoC subsystem has been created to hold the mailbox client
>> driver and will be used for future service drivers.
>>
>> This directory is included by the kconfig option:
>> CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
>> Patra's PolarFire SoC support patches which introduce that option.
>>
> What version of clock driver are you using? I am waiting for an
> updated clock patch to post the next
> version of the basic soc support series.
v2 clock driver should be available shortly. It should only require two
changes in the basic soc support series (pfsoc -> mpfs naming changes).
>> It further depends on the MAINTAINERS entry created in the same series.
>>
>> Changes from v2:
>> * Further reworked dt bindings to satisfy errors and feedback
>> (hopefully phandle array is the correct type for the mboxes)
>> * Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
>> * Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
>> * Further simplified driver code from feedback
>>
>> Changes from v1:
>> * Squashed header into first patch
>> * Fixed DT binding warnings & small fixes
>> * Cleaned up drivers from feedback
>>
>> Conor Dooley (5):
>> mbox: add polarfire soc system controller mailbox
>> dt-bindings: add bindings for polarfire soc mailbox
>> soc: add polarfire soc system controller
>> dt-bindings: add bindings for polarfire soc system controller
>> MAINTAINERS: add maintainers for polarfire soc mailbox driver
>>
>> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
>> .../microchip,mpfs_sys_controller.yaml | 34 +++
>> MAINTAINERS | 1 +
>> drivers/mailbox/Kconfig | 12 +
>> drivers/mailbox/Makefile | 2 +
>> drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
>> drivers/soc/Kconfig | 1 +
>> drivers/soc/Makefile | 1 +
>> drivers/soc/microchip/Kconfig | 10 +
>> drivers/soc/microchip/Makefile | 1 +
>> drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
>> include/soc/microchip/mpfs.h | 51 ++++
>> 12 files changed, 572 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
>> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>> create mode 100644 drivers/mailbox/mailbox-mpfs.c
>> create mode 100644 drivers/soc/microchip/Kconfig
>> create mode 100644 drivers/soc/microchip/Makefile
>> create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
>> create mode 100644 include/soc/microchip/mpfs.h
>>
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
> --
> Regards,
> Atish
Regards,
Cyril
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/5] Add support for the PolarFire SoC system controller
@ 2021-01-07 11:49 ` Cyril.Jean
0 siblings, 0 replies; 26+ messages in thread
From: Cyril.Jean @ 2021-01-07 11:49 UTC (permalink / raw)
To: atishp, Conor.Dooley
Cc: devicetree, damien.lemoal, aou, Lewis.Hanly, david.abdurachmanov,
Daire.McNamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, robh+dt, palmer, paul.walmsley, linux-riscv
On 1/6/21 7:25 PM, Atish Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Wed, Dec 23, 2020 at 8:33 AM <conor.dooley@microchip.com> wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> This patch series adds support for the system controller on
>> the PolarFire SoC, using the mailbox framework. A Microchip directory
>> in the SoC subsystem has been created to hold the mailbox client
>> driver and will be used for future service drivers.
>>
>> This directory is included by the kconfig option:
>> CONFIG_SOC_MICROCHIP_POLARFIRE, so this patch series depends on Atish
>> Patra's PolarFire SoC support patches which introduce that option.
>>
> What version of clock driver are you using? I am waiting for an
> updated clock patch to post the next
> version of the basic soc support series.
v2 clock driver should be available shortly. It should only require two
changes in the basic soc support series (pfsoc -> mpfs naming changes).
>> It further depends on the MAINTAINERS entry created in the same series.
>>
>> Changes from v2:
>> * Further reworked dt bindings to satisfy errors and feedback
>> (hopefully phandle array is the correct type for the mboxes)
>> * Full maintainers entry moved to Atish's PFSoC support series, this series now only adds mailbox driver
>> * Converted config options from MPFS to POLARFIRE_SOC so they are more recognisable
>> * Further simplified driver code from feedback
>>
>> Changes from v1:
>> * Squashed header into first patch
>> * Fixed DT binding warnings & small fixes
>> * Cleaned up drivers from feedback
>>
>> Conor Dooley (5):
>> mbox: add polarfire soc system controller mailbox
>> dt-bindings: add bindings for polarfire soc mailbox
>> soc: add polarfire soc system controller
>> dt-bindings: add bindings for polarfire soc system controller
>> MAINTAINERS: add maintainers for polarfire soc mailbox driver
>>
>> .../mailbox/microchip,mailbox-mpfs.yaml | 47 +++
>> .../microchip,mpfs_sys_controller.yaml | 34 +++
>> MAINTAINERS | 1 +
>> drivers/mailbox/Kconfig | 12 +
>> drivers/mailbox/Makefile | 2 +
>> drivers/mailbox/mailbox-mpfs.c | 285 ++++++++++++++++++
>> drivers/soc/Kconfig | 1 +
>> drivers/soc/Makefile | 1 +
>> drivers/soc/microchip/Kconfig | 10 +
>> drivers/soc/microchip/Makefile | 1 +
>> drivers/soc/microchip/mpfs-sys-controller.c | 127 ++++++++
>> include/soc/microchip/mpfs.h | 51 ++++
>> 12 files changed, 572 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,mailbox-mpfs.yaml
>> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>> create mode 100644 drivers/mailbox/mailbox-mpfs.c
>> create mode 100644 drivers/soc/microchip/Kconfig
>> create mode 100644 drivers/soc/microchip/Makefile
>> create mode 100644 drivers/soc/microchip/mpfs-sys-controller.c
>> create mode 100644 include/soc/microchip/mpfs.h
>>
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
> --
> Regards,
> Atish
Regards,
Cyril
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
2020-12-23 16:33 ` conor.dooley
@ 2021-01-08 2:31 ` Rob Herring
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2021-01-08 2:31 UTC (permalink / raw)
To: conor.dooley
Cc: damien.lemoal, jassisinghbrar, aou, paul.walmsley, palmer,
devicetree, linux-riscv, lewis.hanly, cyril.jean, daire.mcnamara,
atish.patra, anup.patel, david.abdurachmanov, j.neuschaefer
On Wed, Dec 23, 2020 at 04:33:16PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> new file mode 100644
> index 000000000000..60fde1925910
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip PolarFire SoC (MPFS) system controller
> +
> +maintainers:
> + - Conor Dooley <conor.dooley@microchip.com>
'description'?
Perhaps including some info about what this block does and how it is
accessed.
> +
> +properties:
> + compatible:
> + const: microchip,polarfire-soc-sys-controller
> +
> + mboxes:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + phandle and index of the mailbox controller device node.
> + See: ../mailbox/mailbox.txt for more information.
All this is defined already in the common binding schema, drop.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - "mboxes"
Don't need quotes.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + syscontroller: syscontroller {
> + compatible = "microchip,polarfire-soc-sys-controller";
> + mboxes = <&mbox 0>;
> + };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller
@ 2021-01-08 2:31 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2021-01-08 2:31 UTC (permalink / raw)
To: conor.dooley
Cc: devicetree, damien.lemoal, aou, lewis.hanly, david.abdurachmanov,
daire.mcnamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, palmer, paul.walmsley, cyril.jean, linux-riscv
On Wed, Dec 23, 2020 at 04:33:16PM +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the MSS system controller on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../microchip,mpfs_sys_controller.yaml | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> new file mode 100644
> index 000000000000..60fde1925910
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs_sys_controller.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs_sys_controller.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip PolarFire SoC (MPFS) system controller
> +
> +maintainers:
> + - Conor Dooley <conor.dooley@microchip.com>
'description'?
Perhaps including some info about what this block does and how it is
accessed.
> +
> +properties:
> + compatible:
> + const: microchip,polarfire-soc-sys-controller
> +
> + mboxes:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + phandle and index of the mailbox controller device node.
> + See: ../mailbox/mailbox.txt for more information.
All this is defined already in the common binding schema, drop.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - "mboxes"
Don't need quotes.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + syscontroller: syscontroller {
> + compatible = "microchip,polarfire-soc-sys-controller";
> + mboxes = <&mbox 0>;
> + };
> --
> 2.17.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
2021-01-02 13:01 ` Jonathan Neuschäfer
@ 2021-01-21 12:46 ` Conor.Dooley
-1 siblings, 0 replies; 26+ messages in thread
From: Conor.Dooley @ 2021-01-21 12:46 UTC (permalink / raw)
To: j.neuschaefer
Cc: robh+dt, damien.lemoal, jassisinghbrar, aou, paul.walmsley,
palmer, devicetree, linux-riscv, Lewis.Hanly, Cyril.Jean,
Daire.McNamara, atish.patra, anup.patel, david.abdurachmanov
On 02/01/2021 13:01, Jonathan Neuschäfer wrote
>Hello,
>
>I've added review comments below. Some of them might be more detailed
>than necessary, and reflect my opinion rather than something that must
>be fixed. Anyway, I hope my comments make sense.
>
the more detailed feedback the better in my book, if i dont mention it youll probably see in changed in the next day or two
...
...
>
>
>> + /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
>
>What is the "code for MSS_SYS_PARAM_ERR" semantically? Input validation?
>
>> + writel_relaxed(0, mbox->int_reg);
>
>What does a write to mbox->int_reg do? Does it acknowledge an interrupt?
>It would be interesting to have an explaining comment here.
both of these are hang overs from the bare metal driver and will be dropped
>> +
>> + if (msg->cmd_data_size) {
>> + byte_buf = msg->cmd_data;
>> +
>> + for (index = 0; index < (msg->cmd_data_size / 4); index++)
>> + writel_relaxed(word_buf[index],
>> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
>
>word_buf is uninitialized.
>
>In mpfs_mbox_rx_data, you access the registers at mbox->mailbox_base +
>MAILBOX_REG_OFFSET in steps of four bytes, here you increment the offset
>in steps of one byte, because the index isn't scaled. This seems wrong.
>
Thanks for catching this bug, both are related and were introduced during refactoring.
The only dependent drivers implemented so far don't use the full sending functionality
so it went unnoticed.
...
>
>
>> +
>> + mbox_chan_received_data(chan, (void *)data);
>> + devm_kfree(mbox->dev, data);
>> +}
>> +
>> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
>> +{
>> + struct mbox_chan *chan = (struct mbox_chan *)data;
>
>This cast and the one at the end of mpfs_mbox_rx_data are somewhat
>uncessary, because C allows implicit conversion of void pointers to and
>from other pointer types.
>
true, i had put them in thinking it made it more clear, but on reflection it doesnt.
...
>> +++ b/include/soc/microchip/mpfs.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + *
>> + * Microchip PolarFire SoC (MPFS) mailbox
>
>This is mpfs.h, but the comment implies it's only for mailbox-related
>code, not for all of the Microchip PolarFire SoC. Is this intentional?
>
yeah, while thats correct for now it wont remain that way for long. ill drop the mailbox reference
>
>Best regards,
>Jonathan Neuschäfer
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
@ 2021-01-21 12:46 ` Conor.Dooley
0 siblings, 0 replies; 26+ messages in thread
From: Conor.Dooley @ 2021-01-21 12:46 UTC (permalink / raw)
To: j.neuschaefer
Cc: devicetree, damien.lemoal, aou, Lewis.Hanly, david.abdurachmanov,
Daire.McNamara, jassisinghbrar, atish.patra, anup.patel, robh+dt,
palmer, paul.walmsley, Cyril.Jean, linux-riscv
On 02/01/2021 13:01, Jonathan Neuschäfer wrote
>Hello,
>
>I've added review comments below. Some of them might be more detailed
>than necessary, and reflect my opinion rather than something that must
>be fixed. Anyway, I hope my comments make sense.
>
the more detailed feedback the better in my book, if i dont mention it youll probably see in changed in the next day or two
...
...
>
>
>> + /* Code for MSS_SYS_PARAM_ERR is not implemented with this version of driver. */
>
>What is the "code for MSS_SYS_PARAM_ERR" semantically? Input validation?
>
>> + writel_relaxed(0, mbox->int_reg);
>
>What does a write to mbox->int_reg do? Does it acknowledge an interrupt?
>It would be interesting to have an explaining comment here.
both of these are hang overs from the bare metal driver and will be dropped
>> +
>> + if (msg->cmd_data_size) {
>> + byte_buf = msg->cmd_data;
>> +
>> + for (index = 0; index < (msg->cmd_data_size / 4); index++)
>> + writel_relaxed(word_buf[index],
>> + mbox->mailbox_base + MAILBOX_REG_OFFSET + index);
>
>word_buf is uninitialized.
>
>In mpfs_mbox_rx_data, you access the registers at mbox->mailbox_base +
>MAILBOX_REG_OFFSET in steps of four bytes, here you increment the offset
>in steps of one byte, because the index isn't scaled. This seems wrong.
>
Thanks for catching this bug, both are related and were introduced during refactoring.
The only dependent drivers implemented so far don't use the full sending functionality
so it went unnoticed.
...
>
>
>> +
>> + mbox_chan_received_data(chan, (void *)data);
>> + devm_kfree(mbox->dev, data);
>> +}
>> +
>> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
>> +{
>> + struct mbox_chan *chan = (struct mbox_chan *)data;
>
>This cast and the one at the end of mpfs_mbox_rx_data are somewhat
>uncessary, because C allows implicit conversion of void pointers to and
>from other pointer types.
>
true, i had put them in thinking it made it more clear, but on reflection it doesnt.
...
>> +++ b/include/soc/microchip/mpfs.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + *
>> + * Microchip PolarFire SoC (MPFS) mailbox
>
>This is mpfs.h, but the comment implies it's only for mailbox-related
>code, not for all of the Microchip PolarFire SoC. Is this intentional?
>
yeah, while thats correct for now it wont remain that way for long. ill drop the mailbox reference
>
>Best regards,
>Jonathan Neuschäfer
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
2021-01-21 12:46 ` Conor.Dooley
@ 2021-01-21 13:38 ` Jonathan Neuschäfer
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-21 13:38 UTC (permalink / raw)
To: Conor.Dooley
Cc: j.neuschaefer, robh+dt, damien.lemoal, jassisinghbrar, aou,
paul.walmsley, palmer, devicetree, linux-riscv, Lewis.Hanly,
Cyril.Jean, Daire.McNamara, atish.patra, anup.patel,
david.abdurachmanov
[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]
On Thu, Jan 21, 2021 at 12:46:42PM +0000, Conor.Dooley@microchip.com wrote:
> On 02/01/2021 13:01, Jonathan Neuschäfer wrote
>
> >Hello,
> >
> >I've added review comments below. Some of them might be more detailed
> >than necessary, and reflect my opinion rather than something that must
> >be fixed. Anyway, I hope my comments make sense.
> >
> the more detailed feedback the better in my book, if i dont mention it youll probably see in changed in the next day or two
Great.
> >> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
> >> +{
> >> + struct mbox_chan *chan = (struct mbox_chan *)data;
> >
> >This cast and the one at the end of mpfs_mbox_rx_data are somewhat
> >uncessary, because C allows implicit conversion of void pointers to and
> >from other pointer types.
> >
>
> true, i had put them in thinking it made it more clear, but on reflection it doesnt.
The main problem that I personally have with explicit pointer casts is
that they are accepted by the compiler even when they would be a bad
idea (casting from one non-void pointer type to another (incompatible)
non-void pointer type), so they can hide a class of bugs.
Thanks,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
@ 2021-01-21 13:38 ` Jonathan Neuschäfer
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Neuschäfer @ 2021-01-21 13:38 UTC (permalink / raw)
To: Conor.Dooley
Cc: devicetree, damien.lemoal, aou, Lewis.Hanly, david.abdurachmanov,
Daire.McNamara, jassisinghbrar, j.neuschaefer, atish.patra,
anup.patel, robh+dt, palmer, paul.walmsley, Cyril.Jean,
linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 1228 bytes --]
On Thu, Jan 21, 2021 at 12:46:42PM +0000, Conor.Dooley@microchip.com wrote:
> On 02/01/2021 13:01, Jonathan Neuschäfer wrote
>
> >Hello,
> >
> >I've added review comments below. Some of them might be more detailed
> >than necessary, and reflect my opinion rather than something that must
> >be fixed. Anyway, I hope my comments make sense.
> >
> the more detailed feedback the better in my book, if i dont mention it youll probably see in changed in the next day or two
Great.
> >> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
> >> +{
> >> + struct mbox_chan *chan = (struct mbox_chan *)data;
> >
> >This cast and the one at the end of mpfs_mbox_rx_data are somewhat
> >uncessary, because C allows implicit conversion of void pointers to and
> >from other pointer types.
> >
>
> true, i had put them in thinking it made it more clear, but on reflection it doesnt.
The main problem that I personally have with explicit pointer casts is
that they are accepted by the compiler even when they would be a bad
idea (casting from one non-void pointer type to another (incompatible)
non-void pointer type), so they can hide a class of bugs.
Thanks,
Jonathan Neuschäfer
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-01-21 13:58 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-23 16:32 [PATCH v3 0/5] Add support for the PolarFire SoC system controller conor.dooley
2020-12-23 16:32 ` conor.dooley
2020-12-23 16:32 ` [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox conor.dooley
2020-12-23 16:32 ` conor.dooley
2021-01-02 13:01 ` Jonathan Neuschäfer
2021-01-02 13:01 ` Jonathan Neuschäfer
2021-01-21 12:46 ` Conor.Dooley
2021-01-21 12:46 ` Conor.Dooley
2021-01-21 13:38 ` Jonathan Neuschäfer
2021-01-21 13:38 ` Jonathan Neuschäfer
2020-12-23 16:33 ` [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox conor.dooley
2020-12-23 16:33 ` conor.dooley
2020-12-31 18:34 ` Rob Herring
2020-12-31 18:34 ` Rob Herring
2021-01-02 11:30 ` Jonathan Neuschäfer
2021-01-02 11:30 ` Jonathan Neuschäfer
2020-12-23 16:33 ` [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller conor.dooley
2020-12-23 16:33 ` conor.dooley
2021-01-05 21:25 ` Jonathan Neuschäfer
2021-01-05 21:25 ` Jonathan Neuschäfer
2021-01-08 2:31 ` Rob Herring
2021-01-08 2:31 ` Rob Herring
2021-01-06 19:25 ` [PATCH v3 0/5] Add support for the PolarFire SoC " Atish Patra
2021-01-06 19:25 ` Atish Patra
2021-01-07 11:49 ` Cyril.Jean
2021-01-07 11:49 ` Cyril.Jean
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.