From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753516AbdEESV7 (ORCPT <rfc822;w@1wt.eu>);
        Fri, 5 May 2017 14:21:59 -0400
Received: from mail-pg0-f45.google.com ([74.125.83.45]:33363 "EHLO
        mail-pg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752513AbdEESVz (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 5 May 2017 14:21:55 -0400
Date: Fri, 5 May 2017 11:21:51 -0700
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Andy Gross <andy.gross@linaro.org>, Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>, Ohad Ben-Cohen <ohad@wizery.com>,
        linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org,
        Devicetree List <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-remoteproc@vger.kernel.org
Subject: Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional
Message-ID: <20170505182151.GF15143@minitux>
References: <20170504200539.27027-1-bjorn.andersson@linaro.org>
 <CABb+yY12=OksvgMeo+fT+R8m=_Lv+39EQwW0VGsnKuHRBM-5uw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CABb+yY12=OksvgMeo+fT+R8m=_Lv+39EQwW0VGsnKuHRBM-5uw@mail.gmail.com>
User-Agent: Mutt/1.7.2 (2016-11-26)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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