| Message ID | 20170504200539.27027-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 04/05/17 21:05, Bjorn Andersson wrote: > Some mailbox hardware doesn't have to perform any additional operations > on startup of shutdown, so make these optional. > Quite handy. I can you this to get rid of empty shutdown that I added in[1] Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
On Fri, May 5, 2017 at 1:35 AM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > Some mailbox hardware doesn't have to perform any additional operations > on startup of shutdown, so make these optional. > Thanks, makes sense. > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c > index 4671f8a12872..c88de953394a 100644 > --- a/drivers/mailbox/mailbox.c > +++ b/drivers/mailbox/mailbox.c > @@ -137,6 +137,20 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer) > return HRTIMER_NORESTART; > } > > +static int mbox_startup(struct mbox_chan *chan) > +{ > + if (chan->mbox->ops->startup) > + return chan->mbox->ops->startup(chan); > + > + return 0; > +} > + > +static void mbox_shutdown(struct mbox_chan *chan) > +{ > + if (chan->mbox->ops->shutdown) > + chan->mbox->ops->shutdown(chan); > +} > + These functions are going to be called from exactly one place. So maybe we simply do the check before startup/shutdown calls? thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 05 May 03:33 PDT 2017, Jassi Brar wrote: > On Fri, May 5, 2017 at 1:35 AM, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > Some mailbox hardware doesn't have to perform any additional operations > > on startup of shutdown, so make these optional. > > > Thanks, makes sense. > Thanks > > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c > > index 4671f8a12872..c88de953394a 100644 > > --- a/drivers/mailbox/mailbox.c > > +++ b/drivers/mailbox/mailbox.c > > @@ -137,6 +137,20 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer) > > return HRTIMER_NORESTART; > > } > > > > +static int mbox_startup(struct mbox_chan *chan) > > +{ > > + if (chan->mbox->ops->startup) > > + return chan->mbox->ops->startup(chan); > > + > > + return 0; > > +} > > + > > +static void mbox_shutdown(struct mbox_chan *chan) > > +{ > > + if (chan->mbox->ops->shutdown) > > + chan->mbox->ops->shutdown(chan); > > +} > > + > These functions are going to be called from exactly one place. So > maybe we simply do the check before startup/shutdown calls? > I didn't want to add another indentation level at the bottom of mbox_request_channel(), but now that I check again it still would stay below 80 chars and doesn't look too unreasonable. The shutdown() was moved out just to get some symmetry. I'll inline the two checks and resend this together with the driver, once we have concluded on your question/comment there. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index 4671f8a12872..c88de953394a 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -137,6 +137,20 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer) return HRTIMER_NORESTART; } +static int mbox_startup(struct mbox_chan *chan) +{ + if (chan->mbox->ops->startup) + return chan->mbox->ops->startup(chan); + + return 0; +} + +static void mbox_shutdown(struct mbox_chan *chan) +{ + if (chan->mbox->ops->shutdown) + chan->mbox->ops->shutdown(chan); +} + /** * mbox_chan_received_data - A way for controller driver to push data * received from remote to the upper layer. @@ -352,7 +366,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index) spin_unlock_irqrestore(&chan->lock, flags); - ret = chan->mbox->ops->startup(chan); + ret = mbox_startup(chan); if (ret) { dev_err(dev, "Unable to startup the chan (%d)\n", ret); mbox_free_channel(chan); @@ -405,7 +419,7 @@ void mbox_free_channel(struct mbox_chan *chan) if (!chan || !chan->cl) return; - chan->mbox->ops->shutdown(chan); + mbox_shutdown(chan); /* The queued TX requests are simply aborted, no callbacks are made */ spin_lock_irqsave(&chan->lock, flags);
Some mailbox hardware doesn't have to perform any additional operations on startup of shutdown, so make these optional. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Changes since v3: - New patch drivers/mailbox/mailbox.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)