From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755960AbbEVHIb (ORCPT ); Fri, 22 May 2015 03:08:31 -0400 Received: from mail-bl2on0098.outbound.protection.outlook.com ([65.55.169.98]:53437 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752364AbbEVHI1 (ORCPT ); Fri, 22 May 2015 03:08:27 -0400 X-Greylist: delayed 5069 seconds by postgrey-1.27 at vger.kernel.org; Fri, 22 May 2015 03:08:27 EDT Authentication-Results: spf=pass (sender IP is 149.199.60.100) smtp.mailfrom=xilinx.com; ettus.com; dkim=none (message not signed) header.d=none; Message-ID: <555ED5D6.2080600@xilinx.com> Date: Fri, 22 May 2015 09:08:06 +0200 From: Michal Simek User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Moritz Fischer , CC: , , , , , , , , , , , , , , , Subject: Re: [PATCH 1/3] mailbox: Adding driver for Xilinx LogiCORE IP mailbox. References: <1432251450-26352-1-git-send-email-moritz.fischer@ettus.com> <1432251450-26352-2-git-send-email-moritz.fischer@ettus.com> In-Reply-To: <1432251450-26352-2-git-send-email-moritz.fischer@ettus.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-RCIS-Action: ALLOW X-TM-AS-Product-Ver: IMSS-7.1.0.1224-8.0.0.1202-21560.005 X-TM-AS-User-Approved-Sender: Yes;Yes X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11FD012;1:4WLgmZPb73k5Go//LUmkGMEN/ElhV4f92GbaddcchA0PD0ghBWFXWtSTLd63U3xcCSgGhNPNRAoz5zj53BHJ4duFdp1DEv9Geuo9tudeIU8S3saNy3jTTunw1lIaFrlrO+txci66ka3syNw0mfpFnSFKsSf28r8O9hH73S9tbZ9WD1L3rM0Ixl4ZdoUsrTpGQtBcf2729DZ6GV4f+F4QhSVH/rxlSNv0Xw7PuPpRwQD0DqsKU1CEMB+EM0RBCKoJ1IdMkbm2lNUyK6QqqDLvNg== X-Forefront-Antispam-Report: CIP:149.199.60.100;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(438002)(377454003)(479174004)(189002)(199003)(24454002)(51704005)(164054003)(5001770100001)(65806001)(59896002)(5001830100001)(65956001)(5001860100001)(36756003)(106466001)(50466002)(5001960100002)(87266999)(76176999)(92566002)(50986999)(87936001)(65816999)(189998001)(54356999)(63266004)(19580405001)(64126003)(77096005)(83506001)(47776003)(23746002)(80316001)(36386004)(19580395003)(62966003)(77156002)(46102003)(33656002)(4001540100001)(81156007)(6806004)(86362001)(4001350100001)(2950100001)(64706001)(575784001)(107986001)(2004002)(5001870100001);DIR:OUT;SFP:1101;SCL:1;SRVR:BL2FFO11HUB017;H:xsj-pvapsmtpgw02;FPR:;SPF:Pass;PTR:unknown-60-100.xilinx.com,xapps1.xilinx.com;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11HUB017;2:I1mnBuJxH4gcB8202QHLv2Jcb2JjU9cmrwJwzVUxcIveiXspRe+zKtF7exdExvOb;2:rx6vTaMe75x21b6Foc4vzcG1plAG2lt19/IhOC0yfwWKzSlRHIlBeoml82RS2q6xwyEtzP8O4HDXort2jBLGemJgX6f7xiRzh+ST3+pqokyRzJMN8g1thEM+JWBjM9XAoDNbhLY+Pzm066XvJ9pobgbSSQKJVj9Moo1rC2LrMj00lbDDDfz47IIVynGb0zuBFUj30sEY30oIbSwPxpeAzvwmxJ8FsEGLRgsYJW679QdX8Dx0nMf/tspJR18vqEty;6:AeHoK7zNoTbYGqLxa/j8iiUHVtshXgtg3VUrEHlQALfyJ0ybpMlQ1aex+jTo5MJ41+uNwmnXyjibINU8w2w5jzfvcdOp62ks+PwCaRbMAkRJa7O34HPmXBIqplBFfrDzIuxG2rInEN7lkjiqz3QBbM3cj6VhS7fugy1wVDCxZQnWpKlsuIkIXWuCooHZC4X6EQ6fL35MZFP9fxfofEE7IZVAqC7Ppi4Y4CDUSSEiCRzAb88c+9lPjnoProQV3R7jmD9btutcagt2MPVBrKJ0yVfYxRgYuQFaslfQuXrKETOEk/1fVaKndC8v0tvIKlV5oTM3wLwAG5YbU6OZwlAh0LMocA2L1togNDDjGnXdT+jSW+FAjlqumz/mt3WwgrmvD7XI3oSSWFYbFy6WfX+f61//7Nq8yjfU/zqz0KbUyH1vVT61DUdujoOdPGmSvoOVkmlXyx9pIluELwYP+wm4k+HEDjWlDDQXFhdNe95SylYUwaegmScJr1MfBjmQvtnT X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BL2FFO11HUB017; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(520002)(5005006)(3002001);SRVR:BL2FFO11HUB017;BCL:0;PCL:0;RULEID:;SRVR:BL2FFO11HUB017; X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11HUB017;3:URs53lkDKOHYyWDt5DhcSReBIBW8sPOhHfinmgseWZjUDiBX+lbtS2PVnzSb31Lb78D3BFAlo1kIQghis6tM0OqbYrJKlL2C1YzNyh2SsqyacOTp8oL7otqDBu15yT+Gf+NAsNAKCCazOZGXFr1rg0UtD0vIP+smYf8UHlggYnqnnLc5r4eZd5pAXCXHDGMI/JCzRvKpsXGXxIxUHTsn3fv6JT51lKaR81gCKxulXMHCB0qi8XOhouLDdxpzJ6PxFZhiraSamjWDEar3YPMysnZO+DxU2daY/Xp9kTljWLKak5N+XrsVqA/4FPj23SOM X-Forefront-PRVS: 058441C12A X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;BL2FFO11HUB017;9:RQprsYnAA8McjBpF5o8Q8gepPvd/1BRPsvE/b?= =?Windows-1252?Q?YUV7Cnkt5v9kF5dolTVZNEbHqFGlNi6DQW9+hYCdgvVsvWZ9b9GuH5Bm?= =?Windows-1252?Q?tyqDuc3JrIoFWmGi9byrfZk7kTwMW9I0VVUxXC1AH7tOnKQ+gH8kAhTr?= =?Windows-1252?Q?vtk/jTDyc3xKcT7GoqiJvPb1xtm9hs+1wc0ZsrxL/vy5uM8nm64K9e0v?= =?Windows-1252?Q?xuVx7A57VJY+wWx5DRS/q65JJO7+6odQZnrDVOy1pudN5zmY+fwOJXaI?= =?Windows-1252?Q?Om0FIcqjyTlAljnRy97lyuZEL+ffFdSKkvlPF3WYO4Ygx/6uxT8odSIj?= =?Windows-1252?Q?h3UD23GJVpnYqYuQHvPTPii8HU9bn3WaFfQ4JcHq80WUc5MnqHX3I91b?= =?Windows-1252?Q?VHVRVSNchnCqF6y4Den+Ci7c+qq6WGVmECqa91p5xZow1ZPv4OBwutxY?= =?Windows-1252?Q?AV+Tj0HBemTEJXH/OObSp1i88eiknQymANEaFi57uQfX14Ud2Ow4vCAS?= =?Windows-1252?Q?7ORaVQovffpQZEfyIs+9h0JIaCgaxVtsXt+RKtm6duojsbwnJMWTBSe7?= =?Windows-1252?Q?IUyNk6XuV4MKLdJ7VRa1w9y/qyaT88IkiXH9bhTVKNpkp5Qwi25HIjCQ?= =?Windows-1252?Q?88BMTORZLDIn+jv5B70HmzOH8rNhWAc7zAG/YsOp6hH6yR+HYaENDW12?= =?Windows-1252?Q?Z/p5XUEEO7GlUhTv+3vC68S/FHl6TyHWUNcRCUj0pEoZVCvxEV+vnfJR?= =?Windows-1252?Q?49bIT9/lTQXUCtRf2xKPpTzCAvLDfl8SpN3sbZHVYBIX5utIuR/zNUhr?= =?Windows-1252?Q?gg5V1C+0B/TUASsi9blsMfC7MNaMbbzZITjQ6ssl2KaRMA/592dqj/8K?= =?Windows-1252?Q?uo0+FHeUuwwrNhYJGmZB9mCTneUmQrNQ2nG8kKI0NCP60t1pt0ojkb1g?= =?Windows-1252?Q?2Rt2qCTMvBGGPwT2575HbO1IN3Qifm3dQG7yz/GeY+WKWlm4QEUyocJ/?= =?Windows-1252?Q?JHE0goNYMzgrEBDRERG1PSQY26eHYMyzeOK85fSCkuyVQs/7sYDSEiRU?= =?Windows-1252?Q?PyN6Y4cK/NvHna1PpzRR7ssX5NTvm07gtugcv2Novc3c0Ct2+r0fnTK8?= =?Windows-1252?Q?39gV3DJur7hiqPk/vYnWr1523z81fZN7L9ayyXwP3qMV+t/xCu6I/Szm?= =?Windows-1252?Q?YUJ9dBgITb3xRyExceRMvZyUkM2vVnvzRG3adwVY8XbhAlRRHQZYYsek?= =?Windows-1252?Q?8hZBQM0wV49S95b0O3udSa5NJP6/sXVTT8+rBRZ8E5ziafpRU7VLDqzB?= =?Windows-1252?Q?Vy07c0r7NHE+763HVCQ+pYsDp2Ct6P9cAVsH0puVOvV4SgPCAdoyoCHe?= =?Windows-1252?Q?ryRfwNQYVAAHgcxwlFfjytB7a8RZUwukuhW+JQFh3sikjUduJflvGrWP?= =?Windows-1252?Q?7Q2dwHserXD71lqEHht+c8XvNDBQSmvcTxBFx/pVQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11HUB017;3:P6HllrJY0CWnOjra4sYUOu5jVhx9lQTDF4ciZ7NxtZimaIsN7VNfwI7hzujj6GfPR2QEbXg7HjFBL4ACHxZtjokpJPaRb6sjxLjmUj1OudPaA08ql+MRfUKes2JLxe9Nklrm4WSS4G4+yWL/6Hoxcg==;10:Wc9jugw6xKQYFHuSpPqWHskes/y7t+kReErcSjA3TF88BPYNFUmFda4B62S1Brr5YtCqtyElrpzOe02ZagPpQ5fhGmrLUU4zu6e8HWaeGLc= X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 May 2015 07:08:24.6991 (UTC) X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.100];Helo=[xsj-pvapsmtpgw02] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL2FFO11HUB017 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/22/2015 01:37 AM, Moritz Fischer wrote: > The Xilinx LogiCORE IP mailbox is a FPGA core that allows for > interprocessor communication via AXI4 memory mapped / AXI4 stream > interfaces. > > It is single channel per core and allows for transmit and receive. > > Signed-off-by: Moritz Fischer > --- > drivers/mailbox/Kconfig | 8 + > drivers/mailbox/Makefile | 2 + > drivers/mailbox/mailbox-xilinx.c | 339 ++++++++++++++++++++++++++++++++++ > 3 files changed, 349 insertions(+) > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 84b0a2d..e11e4b2 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -60,4 +60,12 @@ config ALTERA_MBOX > An implementation of the Altera Mailbox soft core. It is used > to send message between processors. Say Y here if you want to use the > Altera mailbox support. > + > +config XILINX_MBOX > + tristate "Xilinx Mailbox" > + help > + An implementation of the Xilinx Mailbox soft core. It is used > + to send message between processors. Say Y here if you want to use the > + Xilinx mailbox support. > + > endif > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index b18201e..d28a028 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -11,3 +11,5 @@ obj-$(CONFIG_OMAP2PLUS_MBOX) += omap-mailbox.o > obj-$(CONFIG_PCC) += pcc.o > > obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o > + > +obj-$(CONFIG_XILINX_MBOX) += mailbox-xilinx.o > diff --git a/drivers/mailbox/mailbox-xilinx.c b/drivers/mailbox/mailbox-xilinx.c > new file mode 100644 > index 0000000..8d8aa17 > --- /dev/null > +++ b/drivers/mailbox/mailbox-xilinx.c > @@ -0,0 +1,339 @@ > +/* > + * Copyright (c) 2015, National Instruments Corp. All rights reserved. > + * > + * Driver for the Xilinx Logicore mailbox IP block > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRIVER_NAME "xilinx-mailbox" > + > +/* register offsets */ > +#define MAILBOX_REG_WRDATA 0x00 I prefer to use #definemacro_namevalue but up to you. > +#define MAILBOX_REG_RDDATA 0x08 > +#define MAILBOX_REG_STATUS 0x10 > +#define MAILBOX_REG_ERROR 0x14 > +#define MAILBOX_REG_SIT 0x18 > +#define MAILBOX_REG_RIT 0x1c > +#define MAILBOX_REG_IS 0x20 > +#define MAILBOX_REG_IE 0x24 > +#define MAILBOX_REG_IP 0x28 > + > +/* status register */ > +#define STS_RTA BIT(3) > +#define STS_STA BIT(2) > +#define STS_FULL BIT(1) > +#define STS_EMPTY BIT(0) > + > +/* error register */ > +#define ERR_FULL BIT(1) > +#define ERR_EMPTY BIT(0) > + > +/* mailbox interrupt status register */ > +#define INT_STATUS_ERR BIT(2) > +#define INT_STATUS_RTI BIT(1) > +#define INT_STATUS_STI BIT(0) > + > +/* mailbox interrupt enable register */ > +#define INT_ENABLE_ERR BIT(2) > +#define INT_ENABLE_RTI BIT(1) > +#define INT_ENABLE_STI BIT(0) > + > +#define MBOX_POLLING_MS 5 /* polling interval 5ms */ and you are using tab here - it looks like c&p from somewhere. :-) > + > +struct xilinx_mbox { > + bool intr_mode; look below. > + int irq; > + void __iomem *mbox_base; > + struct device *dev; > + struct mbox_controller controller; > + > + /* if the controller supports only RX polling mode */ > + struct timer_list rxpoll_timer; > +}; > + > +static struct xilinx_mbox *mbox_chan_to_xilinx_mbox(struct mbox_chan *chan) > +{ > + if (!chan || !chan->con_priv) > + return NULL; > + > + return (struct xilinx_mbox *)chan->con_priv; > +} > + > +static inline int xilinx_mbox_full(struct xilinx_mbox *mbox) > +{ > + u32 status; > + > + status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS); empty line here please for easier reading. > + return status & STS_FULL; > +} > + > +static inline int xilinx_mbox_pending(struct xilinx_mbox *mbox) > +{ > + u32 status; > + > + status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS); ditto. > + return !(status & STS_EMPTY); > +} > + > +static void xilinx_mbox_rx_intmask(struct xilinx_mbox *mbox, bool enable) > +{ > + u32 mask; > + > + mask = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IE); > + if (enable) > + mask |= INT_ENABLE_RTI; > + else > + mask &= ~INT_ENABLE_RTI; > + writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IE); > +} > + > +static void xilinx_mbox_tx_intmask(struct xilinx_mbox *mbox, bool enable) > +{ > + u32 mask; > + > + mask = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IE); > + if (enable) > + mask |= INT_ENABLE_STI; > + else > + mask &= ~INT_ENABLE_STI; > + writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IE); > +} > + > +static void xilinx_mbox_rx_data(struct mbox_chan *chan) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + u32 data; > + > + if (xilinx_mbox_pending(mbox)) { > + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA); > + mbox_chan_received_data(chan, (void *)data); > + } > +} > + > +static void xilinx_mbox_poll_rx(unsigned long data) > +{ > + struct mbox_chan *chan = (struct mbox_chan *)data; > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + xilinx_mbox_rx_data(chan); > + > + mod_timer(&mbox->rxpoll_timer, > + jiffies + msecs_to_jiffies(MBOX_POLLING_MS)); > +} > + > +static irqreturn_t xilinx_mbox_interrupt(int irq, void *p) > +{ > + u32 mask; > + struct mbox_chan *chan = (struct mbox_chan *)p; > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + mask = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IS); > + if (mask & INT_STATUS_RTI) > + xilinx_mbox_rx_data(chan); > + > + /* mask irqs *before* notifying done, require tx_block=true */ > + if (mask & INT_STATUS_STI) { > + xilinx_mbox_tx_intmask(mbox, false); > + mbox_chan_txdone(chan, 0); > + } > + > + if (mask & INT_STATUS_ERR) > + dev_err(mbox->dev, "Got error IRQ!\n"); > + > + writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IS); one more empty line here. > + return IRQ_HANDLED; > +} > + > +static int xilinx_mbox_startup(struct mbox_chan *chan) > +{ > + int ret; > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + if (mbox->intr_mode) { look below > + ret = request_irq(mbox->irq, xilinx_mbox_interrupt, 0, > + dev_name(mbox->dev), chan); > + if (unlikely(ret)) { > + dev_err(mbox->dev, > + "failed to register mailbox interrupt:%d\n", > + ret); > + mbox->intr_mode = false; > + goto polling; /* use polling if failed */ > + } > + > + xilinx_mbox_rx_intmask(mbox, true); Are you also setting up polling timer for interrupt mode? Or here should be return? > + } > + > +polling: > + /* setup polling timer */ > + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx, > + (unsigned long)chan); > + mod_timer(&mbox->rxpoll_timer, > + jiffies + msecs_to_jiffies(MBOX_POLLING_MS)); > + > + return 0; > +} > + > +static int xilinx_mbox_send_data(struct mbox_chan *chan, void *data) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + u32 *udata = (u32 *)data; > + > + if (!mbox || !data) > + return -EINVAL; > + > + if (xilinx_mbox_full(mbox)) > + return -EBUSY; > + > + /* enable interrupt before send */ > + if (mbox->intr_mode) > + xilinx_mbox_tx_intmask(mbox, true); > + > + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA); > + > + return 0; > +} > + > +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + /* return false if mailbox is full */ > + return xilinx_mbox_full(mbox) ? false : true; > +} > + > +static bool xilinx_mbox_peek_data(struct mbox_chan *chan) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + return xilinx_mbox_pending(mbox) ? true : false; > +} > + > +static void xilinx_mbox_shutdown(struct mbox_chan *chan) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + if (mbox->intr_mode) { > + /* unmask all interrupt masks */ > + writel_relaxed(~0, mbox->mbox_base + MAILBOX_REG_IE); > + free_irq(mbox->irq, chan); > + } else > + del_timer_sync(&mbox->rxpoll_timer); Also {} around else part. > +} > + > +static struct mbox_chan_ops xilinx_mbox_ops = { > + .send_data = xilinx_mbox_send_data, > + .startup = xilinx_mbox_startup, > + .shutdown = xilinx_mbox_shutdown, > + .last_tx_done = xilinx_mbox_last_tx_done, > + .peek_data = xilinx_mbox_peek_data, > +}; > + > +static int xilinx_mbox_probe(struct platform_device *pdev) > +{ > + struct xilinx_mbox *mbox; > + struct resource *regs; > + struct mbox_chan *chans; > + int ret; > + > + mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), > + GFP_KERNEL); it should fit to one line - no reason for this indentation. > + if (!mbox) > + return -ENOMEM; > + > + /* allocated one channel */ > + chans = devm_kzalloc(&pdev->dev, sizeof(*chans), GFP_KERNEL); > + if (!chans) > + return -ENOMEM; > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + > + mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs); > + if (IS_ERR(mbox->mbox_base)) > + return PTR_ERR(mbox->mbox_base); > + > + mbox->irq = platform_get_irq(pdev, 0); > + /* if irq is present, we can use it, otherwise, poll */ > + if (mbox->irq >= 0) > + mbox->intr_mode = true; > + else > + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n"); Is mbox->intr_mode needed at all? You can simple detect it from mbox->irq anytime. > + > + mbox->dev = &pdev->dev; > + > + /* Hardware supports only one channel. */ > + chans[0].con_priv = mbox; > + mbox->controller.dev = mbox->dev; > + mbox->controller.num_chans = 1; > + mbox->controller.chans = chans; > + mbox->controller.ops = &xilinx_mbox_ops; > + > + if (mbox->intr_mode) { > + mbox->controller.txdone_irq = true; > + } else { > + mbox->controller.txdone_poll = true; > + mbox->controller.txpoll_period = MBOX_POLLING_MS; > + } why is this in the different block? Just put it together with above error message. > + > + ret = mbox_controller_register(&mbox->controller); > + if (ret) { > + dev_err(&pdev->dev, "Register mailbox failed\n"); > + goto err; Just return ret here > + } > + > + platform_set_drvdata(pdev, mbox); > +err: > + return ret; return 0; > +} > + > + > +static int xilinx_mbox_remove(struct platform_device *pdev) > +{ > + struct xilinx_mbox *mbox = platform_get_drvdata(pdev); > + > + if (!mbox) > + return -EINVAL; > + > + mbox_controller_unregister(&mbox->controller); > + > + return 0; > +} > + > +static const struct of_device_id xilinx_mbox_match[] = { > + { .compatible = "xlnx,mailbox-2.1" }, > + { /* Sentinel */ } > +}; > + > +MODULE_DEVICE_TABLE(of, xilinx_mbox_match); > + > +static struct platform_driver xilinx_mbox_driver = { > + .probe = xilinx_mbox_probe, > + .remove = xilinx_mbox_remove, > + .driver = { > + .name = DRIVER_NAME, > + .of_match_table = xilinx_mbox_match, > + }, > +}; > + > +module_platform_driver(xilinx_mbox_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Xilinx mailbox specific functions"); > +MODULE_AUTHOR("Moritz Fischer "); > +MODULE_ALIAS("platform:xilinx-mailbox"); This is fine but have you tested it as a plain platform driver? Thanks, Michal