From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6033FC4363C for ; Wed, 7 Oct 2020 11:42:12 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B9DD520782 for ; Wed, 7 Oct 2020 11:42:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="A4AaAxvZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B9DD520782 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0FLt/kOXijOREpzGZaSMM3epd724i486S2UtSCB54NA=; b=A4AaAxvZrV/L25d3PMDhTjzJS XGN08b0H8QhXPj9PFMzmu7j1/PpMHtOuVandhlgB8/hvv8m2mh6Kq0YvvjZT4EithjNIQWuTR4zkU bw/rvUZwA4iz4t028WcB04Am5/ocFhIybGxgabbq2z1emPAPQYz4VbcyRy6tgWt1f71GbZS8XVQWh jjYb+xo8qMTiT2qQ2+e89uvqvAkFTehDM8sOZPf2DdSy6mO3PnX8kqK3R01Tl8br0S6Z2cnEIe1tr xnMwyqBslGJbhpL2X5a4DTQJdvTiINzQrexXEQrWLPcO734LjRSK8O7Yu1XWfYNNo3tpxtpvVqiYV gXyTFeKXA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQ7oK-0004gv-2Z; Wed, 07 Oct 2020 11:40:44 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQ7oH-0004gE-Gl for linux-arm-kernel@lists.infradead.org; Wed, 07 Oct 2020 11:40:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 11E2D31B; Wed, 7 Oct 2020 04:40:39 -0700 (PDT) Received: from bogus (unknown [10.57.54.133]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E2BD33F71F; Wed, 7 Oct 2020 04:40:36 -0700 (PDT) Date: Wed, 7 Oct 2020 12:40:34 +0100 From: Sudeep Holla To: Jassi Brar Subject: Re: [PATCH 4/4] mailbox: arm_mhu: Add ARM MHU doorbell driver Message-ID: <20201007114034.rkiujybiknaedy7m@bogus> References: <20200928114445.19689-1-sudeep.holla@arm.com> <20200928114445.19689-5-sudeep.holla@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20171215 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201007_074041_650913_1F942481 X-CRM114-Status: GOOD ( 20.62 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: DTML , Vincent Guittot , Rob Herring , Viresh Kumar , LKML , Bjorn Andersson , Jassi Brar , Rob Herring , Sudeep Holla , Frank Rowand , ALKML Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Oct 02, 2020 at 02:42:37PM -0500, Jassi Brar wrote: > On Mon, Sep 28, 2020 at 6:45 AM Sudeep Holla wrote: > > > + > > +static void mhu_db_shutdown(struct mbox_chan *chan) > > +{ > > + struct mhu_db_channel *chan_info = chan->con_priv; > > + struct mbox_controller *mbox = &chan_info->mhu->mbox; > > + int i; > > + > > + for (i = 0; i < mbox->num_chans; i++) > > + if (chan == &mbox->chans[i]) > > + break; > > + > > + if (mbox->num_chans == i) { > > + dev_warn(mbox->dev, "Request to free non-existent channel\n"); > > + return; > > + } > > + > > + /* Reset channel */ > > + mhu_db_mbox_clear_irq(chan); > > + chan->con_priv = NULL; > > > request->free->request will fail because of this NULL assignment. > Maybe add a 'taken' flag in mhu_db_channel, which should also be > checked before calling mbox_chan_received_data because the data may > arrive for a now relinquished channel. > Good point, but the new 'taken' flag will have the same race as con_priv. We need a lock here and can we use chan->lock or do you prefer this driver maintains it own for this purpose. mbox_request_channel releases the lock before calling startup and mbox_free_channel acquires the after shutdown returns, so technically we can reuse the same lock. > > + > > +static struct mbox_chan *mhu_db_mbox_xlate(struct mbox_controller *mbox, > > + const struct of_phandle_args *spec) > > +{ > > + struct arm_mhu *mhu = dev_get_drvdata(mbox->dev); > > + struct mhu_db_channel *chan_info; > > + struct mbox_chan *chan = NULL; > > + unsigned int pchan = spec->args[0]; > > + unsigned int doorbell = spec->args[1]; > > + int i; > > + > > + /* Bounds checking */ > > + if (pchan >= MHU_CHANS || doorbell >= MHU_NUM_DOORBELLS) { > > + dev_err(mbox->dev, > > + "Invalid channel requested pchan: %d doorbell: %d\n", > > + pchan, doorbell); > > + return ERR_PTR(-EINVAL); > > + } > > + > > + for (i = 0; i < mbox->num_chans; i++) { > > + chan_info = mbox->chans[i].con_priv; > > + > > + /* Is requested channel free? */ > > + if (chan_info && > > + mbox->dev == chan_info->mhu->dev && > > + pchan == chan_info->pchan && > > + doorbell == chan_info->doorbell) { > > + dev_err(mbox->dev, "Channel in use\n"); > > + return ERR_PTR(-EBUSY); > > + } > > + > You may want to reuse mhu_db_mbox_to_channel. Good point, thanks for pointing that out, will update. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel