| Message ID | 20201223163255.28992-1-conor.dooley@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/5] mbox: add polarfire soc system controller mailbox | expand |
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
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
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
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__ */