From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751741AbbE1Fph (ORCPT ); Thu, 28 May 2015 01:45:37 -0400 Received: from mail-bl2on0100.outbound.protection.outlook.com ([65.55.169.100]:60048 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751143AbbE1Fp2 (ORCPT ); Thu, 28 May 2015 01:45:28 -0400 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: <5566AB68.8000108@xilinx.com> Date: Thu, 28 May 2015 07:45:12 +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: [PATCHv2 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox. References: <1432751745-17473-1-git-send-email-moritz.fischer@ettus.com> <1432751745-17473-3-git-send-email-moritz.fischer@ettus.com> In-Reply-To: <1432751745-17473-3-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-21572.005 X-TM-AS-User-Approved-Sender: Yes;Yes X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11OLC005;1:cQQxezJdNNk153UdE9aQNKPxD4pd1BvS2Fxsirp9dFl8NTIzRF5Udz6IaF+y6VbbToOrPK+dLwfg1FCXxdOb2UlplwK7xiipd2LhmP3ArjS4/B0S/0iftI5R6IN1rZvnUrYJRHJEMbg2Rzxx7cslttMfd/uQq6adbE79sakWjxs/juP288ojDUovypSOSWoVfGWKmQ86B1O54kbG5YOi2wMRnT0jXafFM++6XqfbITcsOL6hetLy9fHfQ3CcXacufHdzat/dv/GY8E2uoojabA== X-Forefront-Antispam-Report: CIP:149.199.60.100;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(438002)(51704005)(164054003)(199003)(189002)(377454003)(479174004)(24454002)(83506001)(5001920100001)(64706001)(19580395003)(23746002)(5001960100002)(5001770100001)(5001830100001)(47776003)(50986999)(6806004)(76176999)(54356999)(106466001)(19580405001)(575784001)(65816999)(87936001)(87266999)(63266004)(86362001)(80316001)(5001860100001)(33656002)(65806001)(65956001)(46102003)(50466002)(62966003)(77156002)(2950100001)(92566002)(59896002)(36386004)(64126003)(4001350100001)(77096005)(81156007)(4001540100001)(189998001)(36756003)(107986001)(2004002)(5001870100001);DIR:OUT;SFP:1101;SCL:1;SRVR:BY2FFO11HUB010;H:xsj-pvapsmtpgw02;FPR:;SPF:Pass;PTR:xapps1.xilinx.com,unknown-60-100.xilinx.com;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11HUB010;2:YpS9m3gTGJxsmbXfV4SZQDkwpYC6HLhSWjyV/8dXHXalN+ZaHKl282qiF3EkN9ew;2:+NoaJLJ3Y3qN0jzIIbc3eY1p6AvNSFvpvC4exhs8+nA5+fSnb7BubISGgVnDiD7NF9EHEfpGBwGzew+N7yXByMrmCRRqoTyLX88T5sV8EFDgtPAeXrRpcGO0eJjD4LS5wrrTmNooUryKlAmLooZJqb75cD8sn0YZA8S1oQB5zXipY1KNpuXZVZqZj9xNgKi/aF1fLYRG6L7dLBsdUfZCrmwE7s+qXfcqTstnuCzs269ttNQf9LsyQqeUkYjpEAQq;6:ogz1TLt8sVZNUJURnXxIuNbQ3ggCFUZqVmconPvLTW4dFL1RG3S8IRrDp5+9e09a2S9meSrONwAc/jnjDyp9kWw0RRmnHjSNOdDxEry5J8ubVCdybGYvL4/oQxO4BHUT+m+rjCA/116M//iYcyiD5ciyZDgmK0iF1CJ0OziLOi9Rx0XEy9YYgCEFkNNl/mEMTICJ+CwwogjnDphOgCrMQRInYE/fkfCQPll5Zo48IuwbQ7iCujl9uamdSsXekq1VKb47+0i8Y+oavGkHAnpP5Af5P5bStfnZC6dLpCzSlgn1AXJN7ufJ0BiDLspoq9vRska5SwLQyZFSa5YRsWBrluTyufJxK/i7aabpuZ1QvAln+3DHl6m/OQlw8stm7Ywict7bUhSryyS2eqc0mt0AS3H35MpasZ37FttHTu/ZUMDqkvhLE5Jrc/KZcheK44LnIDNDiUjTfy05BrqwFhwCmVJk4HymxFbG2hQ5xfF1JyKnrZcLeH4e57eefky++XBe X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2FFO11HUB010; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(520003)(5005006)(3002001);SRVR:BY2FFO11HUB010;BCL:0;PCL:0;RULEID:;SRVR:BY2FFO11HUB010; X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11HUB010;3:Zp7Rbz0yBt2aGBjJENS/hz4iEObjeOvU5l/HoFZfv2py5DU5lIesftJdbLokr/yLuSMVQCa8Wl7AJUijSsywSGbcD12YcG/KzdEESt0qhE9zjb6haTPQz4nbi2SK8RXrQXXJ8+d5ZsoQfG2VOuo0jFsU0xw98r+3jnuPpEYME7FhPR4hh/+acXACpDcm5Bz5Gk0UBYMJMyJmkSSilbicF+2vibidWe83bk1TfNU80FhbPbTbV7Bh/Nuix6IGZt2GOoxoQnEfjbJ0oQy9a1Lv7XKckjWbdtY1MiTQBD4u0DZBX+8bTO3JxLzhApawWaN6 X-Forefront-PRVS: 0590BBCCBC X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;BY2FFO11HUB010;9:Ur5HbxOHLvSbRWo9hUIUz0lI19Fq60DoQbtwo?= =?Windows-1252?Q?lg89n2yxqLuGHKhLa8/oqt9db0Ao4wjZc2zB8a4qxUUa77jd9Iehs3gn?= =?Windows-1252?Q?7SkCFRbK1fY+u2JilgPqfezfA/gFkQ34nEiBQkVDI54ASHSISEC4lo29?= =?Windows-1252?Q?JDrou6IeuWdXEoN3q0XsDxVIWIPPfnEl1FwFuIXFnEDpXPUj2zwPjB63?= =?Windows-1252?Q?PoBDqNEAmaB/f+sb0jiM+v0FhUPliLXWNfyFqo79NX/fGI6yiU4A9vWw?= =?Windows-1252?Q?IzilydTxQ8dqcOuFlsC1e/jsTmu4oC+WYJxgS7a2SCsW2fk9nnQue2Jz?= =?Windows-1252?Q?xF6vt7JMusQHmm7m3D72NcJOz7T4p4lc0GxY+hJVjgEkJHX7YjRjtKUn?= =?Windows-1252?Q?icbsg8Ew240clXgEWCYmlPIbQxUyHQZnxAX+HF8SpNg1YHR37ik7LQX8?= =?Windows-1252?Q?ALwy5TRd+SvhG7BouVpuD6oL7LgXO+wv++CZOvDx3XzddG0GnyYY1aLx?= =?Windows-1252?Q?votNNBoFZw5rlKD9VU6k9DZImrP/ZDJPey/SgkyTI6GfJtL1pD/08sWM?= =?Windows-1252?Q?wh4cEQdwkdwKfKutSFBH7+LjCtqooyn9Gug4HS6kFf2z5B0D10cN5MWF?= =?Windows-1252?Q?VBGBZBNKoaHVk7XrvJDjWMYY4/xr6tXWl6u3O2opbeL0tcUjRbVvyYsi?= =?Windows-1252?Q?ZWpooexXAO3mVaK1Rqb5zeOR+aNRoEwZQwap8GW/v9b2A91wpBn6itPd?= =?Windows-1252?Q?LfJTs6Li4mtlPkDrZAIIzwMV3L8dzV7WJmD/76cf01S/weE6TLRTGCak?= =?Windows-1252?Q?DDgUk/ZXnBx5jMRh293dXpQpKiUUz4SsBAbd90mCyEt5JDKuBrwBkgNP?= =?Windows-1252?Q?WCeq1/urBpZQAbrne11G5v0zbJ86P9mDFm1CiwKVo9y5RJojjAmWmlff?= =?Windows-1252?Q?RRph4hfLUX+LJJ3CQUswIQhFzbmFJ5MRoCTaqnsWdv2OVqxRrONVYfhb?= =?Windows-1252?Q?4khmynI4XWyxEvTaS1TTv16xza4v/HahJLvDqaATADxHFpIj4hOQP8N5?= =?Windows-1252?Q?FZpGJypMBxhX41TTfaXCF3Yea9X3TtiYJrDtw5uLRBKnt+iFj06Whyq3?= =?Windows-1252?Q?fe6qnTDM0xkareRY7lTvZhso3xGqtuOqvZiee0RTZ0yV4EFotF9+Gqum?= =?Windows-1252?Q?0seROxXKK8lrEkBp3Lb4uIQbOZdXiG766ei8LrMnikN9VN2df0HxPGJF?= =?Windows-1252?Q?vd3YSlMIna8gn8FTg+HBPNwrHtiUhKbo0uRk1V976aMm7F1PYYI/MsI5?= =?Windows-1252?Q?jnA4j/7AZ94PchppXPOEuf1L9Dx7nTjvgJ1ytOqAtq3Dxe1CFJ6ZKJC8?= =?Windows-1252?Q?cSUddVtpOCLj87WqXjz8Kp5hIoDzdXbpyKJhjtoSc3LMXVWsI+7hbNCB?= =?Windows-1252?Q?AvvojGoJHMuHgfEsyN8OHJA9ul7wFkAPCrS/RtRjaDzxaCMivBb/8x7H?= =?Windows-1252?Q?8T0fFk=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11HUB010;3:jQB1JMyZ695emFyqdJHOmI1X7XZARsC92Fq9jkpJdw/u4XmrSlbR7TvvBg+yX+2GUSV541hH9OvcJ2QcTVgg2JX6D0iGKvfQaDgc8jdNHEX5MdylT5dLVVHR6ToIZ3aCGK9iN0i14r1KoNxTv6fzpQ==;10:LPzZdCgW3wznIEpPSJGv7RM84EJDexnB+YBcWA5/LaRCiREgCD2VJfQWqpGSVwO7wFW33ZQkZ8EwxYy0ICQjf4/bxl36y0+CchjpzLI9Tkg= X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 May 2015 05:45:25.0836 (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: BY2FFO11HUB010 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/27/2015 08:35 PM, 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. > > Changes from v1: > - Added common clock framework support > - Deal with IRQs that happend before driver load, > since HW will not let us know about them when we enable IRQs > > Changes from v0: > - Several stylistic issues > - Dropped superfluous intr_mode member > - Really masking the IRQs on mailbox_shutdown > - No longer using polling by accident in non-IRQ mode > - Swapped doc and driver commits > > Signed-off-by: Moritz Fischer > --- > MAINTAINERS | 7 + > drivers/mailbox/Kconfig | 8 + > drivers/mailbox/Makefile | 2 + > drivers/mailbox/mailbox-xilinx.c | 349 ++++++++++++++++++++++++++++++++++ > 4 files changed, 366 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index f8e0afb..f1f0d10 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10986,6 +10986,13 @@ M: John Linn > S: Maintained > F: drivers/net/ethernet/xilinx/xilinx_axienet* > > +XILINX MAILBOX DRIVER > +M: Moritz Fischer > +L: linux-kernel@vger.kernel.org > +S: Maintained > +F: drivers/mailbox/mailbox-xilinx.c > +F: Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt > + > XILINX UARTLITE SERIAL DRIVER > M: Peter Korsgaard > L: linux-serial@vger.kernel.org > 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..fd1cdf2 > --- /dev/null > +++ b/drivers/mailbox/mailbox-xilinx.c > @@ -0,0 +1,349 @@ > +/* > + * 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 > +#include > + > +#define DRIVER_NAME "xilinx-mailbox" > + > +/* register offsets */ > +#define MAILBOX_REG_WRDATA 0x00 > +#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 */ > + > +struct xilinx_mbox { > + int irq; > + void __iomem *mbox_base; > + struct clk *clk; > + 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 bool xilinx_mbox_full(struct xilinx_mbox *mbox) > +{ > + u32 status; > + > + status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS); > + > + return status & STS_FULL; > +} > + > +static inline bool xilinx_mbox_pending(struct xilinx_mbox *mbox) > +{ > + u32 status; > + > + status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS); > + > + return !(status & STS_EMPTY); > +} > + > +static void xilinx_mbox_intmask(struct xilinx_mbox *mbox, u32 mask, bool enable) > +{ > + u32 mask_reg; > + > + mask_reg = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IE); > + if (enable) > + mask_reg |= mask; > + else > + mask_reg &= ~mask; > + writel_relaxed(mask_reg, mbox->mbox_base + MAILBOX_REG_IE); > +} > + > + > +static inline void xilinx_mbox_rx_intmask(struct xilinx_mbox *mbox, bool enable) > +{ > + xilinx_mbox_intmask(mbox, INT_ENABLE_RTI, enable); > +} > + > +static inline void xilinx_mbox_tx_intmask(struct xilinx_mbox *mbox, bool enable) > +{ > + xilinx_mbox_intmask(mbox, INT_ENABLE_STI, enable); > +} > + > +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); > + } > + > + writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IS); empty line here please. > + 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->irq >= 0) { > + 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); > + goto polling; /* use polling if failed */ This is the problematic path. For case that request_irq failed - you are using polling mode but in shutdown below mbox_irq > 0 and you will free_irq not call del_timer_sync. I understand that you want to be smart here but it should be enough just to return error here and you are done. But up2you I expect that this path wasn't tested. > + } > + > + xilinx_mbox_rx_intmask(mbox, true); > + > + /* if fifo was full already, we didn't get an interrupt */ > + while (xilinx_mbox_pending(mbox)) > + xilinx_mbox_rx_data(chan); > + > + return 0; > + } > + > +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->irq >= 0) > + 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); > +} > + > +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); > +} > + > +static void xilinx_mbox_shutdown(struct mbox_chan *chan) > +{ > + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan); > + > + if (mbox->irq >= 0) { look below. > + /* mask all interrupts */ > + writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE); > + free_irq(mbox->irq, chan); > + } else { > + del_timer_sync(&mbox->rxpoll_timer); > + } > +} > + > +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); > + if (!mbox) > + return -ENOMEM; > + > + /* get clk and enable */ > + mbox->clk = devm_clk_get(&pdev->dev, "mbox"); > + clk_prepare_enable(mbox->clk); huh you should probably check error first before enabling. Also if you call enable there, you should also call clk_disable_unprepare when any error here happen. Probably make sense to move this clock enabling below. > + if (IS_ERR(mbox->clk)) { > + dev_err(&pdev->dev, "Couldn't get clk.\n"); > + return PTR_ERR(mbox->clk); > + } > + > + /* 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) 0 also suggest error - it means here should be just > 0; > + mbox->controller.txdone_irq = true; missing {} around if - look at coding style guide > + else { > + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n"); > + mbox->controller.txdone_poll = true; > + mbox->controller.txpoll_period = MBOX_POLLING_MS; > + } > + > + 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; > + > + ret = mbox_controller_register(&mbox->controller); > + if (ret) { > + dev_err(&pdev->dev, "Register mailbox failed\n"); > + return ret; > + } > + > + platform_set_drvdata(pdev, mbox); > + > + return 0; > +} > + > + remove this one empty line. > +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); > + > + clk_disable_unprepare(mbox->clk); > + > + 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"); > Thanks, Michal