mbox: add polarfire soc system controller mailbox
Subsystems (3)
[email protected]
maintainerPATCH v5 2/5 mbox: add polarfire soc system controller mailbox
From: Conor Dooley <[email protected]>
This driver adds support for the single mailbox channel of the MSS system controller on the Microchip PolarFire SoC.
--- drivers/mailbox/Kconfig | 12 ++ drivers/mailbox/Makefile | 2 + drivers/mailbox/mailbox-mpfs.c | 272 +++++++++++++++++++++++++++++++++ include/soc/microchip/mpfs.h | 56 +++++++ 4 files changed, 342 insertions(+) create mode 100644 drivers/mailbox/mailbox-mpfs.c create mode 100644 include/soc/microchip/mpfs.h
drivers/mailbox/Kconfig +12 -0
@@ -160,6 +160,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
drivers/mailbox/Makefile +2 -0
@@ -41,6 +41,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
drivers/mailbox/mailbox-mpfs.c new file +272 -0
272 lines not on this page · raw message
include/soc/microchip/mpfs.h new file +56 -0
56 lines not on this page · raw message
Signature · 1 line
2.31.1
Jassi Brar
maintainerdrivers/mailbox/mailbox-mpfs.c line 67
Quoted 11 lines · On Mon, Apr 12, 2021 at 11:04 AM <[email protected]> wrote:
drivers/mailbox/mailbox-mpfs.c +8
-0+ +struct mpfs_mbox { +struct mbox_controller controller; +struct device *dev; +int irq; +void __iomem *mbox_base; +void __iomem *int_reg; +struct mbox_chan *chans;
As I said previously .... struct mbox_chan chans[1] ?
Quoted 4 lines
drivers/mailbox/mailbox-mpfs.c +4
-0+static bool mpfs_mbox_busy (struct mpfs_mbox *mbox) +{ +u32 status; +
drivers/mailbox/mailbox-mpfs.c line 85
Quoted 10 lines
drivers/mailbox/mailbox-mpfs.c +10
-0+ status =readl_relaxed (mbox->mbox_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 ; +
The return value is not always checked before use in the caller. And I don't see how could 'chan' ever be null. So, maybe just discard this function ?
Quoted 7 lines
drivers/mailbox/mailbox-mpfs.c +7
-0+return (struct mpfs_mbox *)chan->con_priv; +} + +static int mpfs_mbox_send_data (struct mbox_chan *chan,void *data) +{ +u32 val =0u ; +u16 opt_sel;
drivers/mailbox/mailbox-mpfs.c line 102
Quoted 10 lines
drivers/mailbox/mailbox-mpfs.c +10
-0+u32 tx_trigger; +struct mpfs_mss_msg *msg = data; +struct mpfs_mbox *mbox =mbox_chan_to_mpfs_mbox (chan); + + mbox->response = msg->response; + mbox->resp_offset = msg->resp_offset; + +if (mpfs_mbox_busy (mbox)) +return -EBUSY ; +
This check should be unnecessary because the api won't call send_data() unless the last one is done.
drivers/mailbox/mailbox-mpfs.c line 145
Quoted 9 lines
drivers/mailbox/mailbox-mpfs.c +9
-0+static inline bool mpfs_mbox_pending (struct mpfs_mbox *mbox) +{ +u32 status; + + status =readl_relaxed (mbox->mbox_base +SERVICES_SR_OFFSET ); + +return !(status &SCB_STATUS_BUSY_MASK ); +} +
Isn't this !mpfs_mbox_busy() ?
drivers/mailbox/mailbox-mpfs.c line 152
Quoted 7 lines
drivers/mailbox/mailbox-mpfs.c +7
-0+static void mpfs_mbox_rx_data (struct mbox_chan *chan) +{ +struct mpfs_mbox *mbox =mbox_chan_to_mpfs_mbox (chan); +u32 i; +u16 num_words =ALIGN ((mbox->response->resp_size), (4 )) /4U ; +struct mpfs_mss_response *response = mbox->response; +
Usually the decs are in ascending or descending order. Please choose one.
drivers/mailbox/mailbox-mpfs.c line 189
Quoted 10 lines
drivers/mailbox/mailbox-mpfs.c +9
-0+ +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);
scripts/checkpatch should catch missing newline before "ret =" ?
cheers.
[email protected]
maintainerQuoted 16 lines · Jassi Brar
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Quoted 12 lines · On Mon, Apr 12, 2021 at 11:04 AM <[email protected]> wrote:
drivers/mailbox/mailbox-mpfs.c +11
-0+return (struct mpfs_mbox *)chan->con_priv; +} + +static int mpfs_mbox_send_data (struct mbox_chan *chan,void *data) +{ +u32 val =0u ; +u16 opt_sel; +u32 tx_trigger; +struct mpfs_mss_msg *msg = data; +struct mpfs_mbox *mbox =mbox_chan_to_mpfs_mbox (chan); +
Quoted 10 lines
drivers/mailbox/mailbox-mpfs.c line 102
Quoted 6 lines
drivers/mailbox/mailbox-mpfs.c +6
-0+ mbox->response = msg->response; + mbox->resp_offset = msg->resp_offset; + +if (mpfs_mbox_busy (mbox)) +return -EBUSY ; +This check should be unnecessary because the api won't call
send_data()unless the last one is done.
will act on the other observations/requests, this one should stay as is though - possible for something other than this driver to cause the system controller to be busy.
cheers