linux-kernel - Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 5 May 2017 11:21:51 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Jassi Brar <jassisinghbrar@...il.com>
Cc:     Andy Gross <andy.gross@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        Devicetree List <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional

On Fri 05 May 03:33 PDT 2017, Jassi Brar wrote:

> On Fri, May 5, 2017 at 1:35 AM, Bjorn Andersson
> <bjorn.andersson@...aro.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