linux-kernel - Re: [PATCHv5 2/4] mailbox: Introduce framework for mailbox

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:	Wed, 28 May 2014 09:50:21 +0530
From:	Jassi Brar <jassisinghbrar@...il.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Suman Anna <s-anna@...com>,
	Loic Pallardy <loic.pallardy@...com>,
	LeyFoon Tan <lftan.linux@...il.com>,
	Craig McGeachie <slapdau@...oo.com.au>,
	Courtney Cavin <courtney.cavin@...ymobile.com>,
	Rob Herring <robherring2@...il.com>,
	Arnd Bergmann <arnd@...db.de>,
	Josh Cartwright <joshc@...eaurora.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Kumar Gala <galak@...eaurora.org>,
	Girish K S <ks.giri@...sung.com>,
	Jassi Brar <jaswinder.singh@...aro.org>
Subject: Re: [PATCHv5 2/4] mailbox: Introduce framework for mailbox

On Wed, May 21, 2014 at 10:57 PM, Mark Brown <broonie@...nel.org> wrote:
> On Thu, May 15, 2014 at 11:41:00AM +0530, Jassi Brar wrote:
>> Introduce common framework for client/protocol drivers and
>> controller drivers of Inter-Processor-Communication (IPC).
>
> This looks pretty nice, though I do have a few *very* small nits beyond
> those Arnd had.
>
>> +     if (chan->cl->tx_block && chan->active_req) {
>> +             int ret;
>> +             init_completion(&chan->tx_complete);
>
> reinit_completion().
>
>> +     if (!cl->tx_tout) /* wait for ever */
>> +             cl->tx_tout = msecs_to_jiffies(3600000);
>> +     else
>> +             cl->tx_tout = msecs_to_jiffies(cl->tx_tout);
>
> Is the default wait for ever the best timeout - I'm not sure it's best
> from a defensiveness point of view.  It should be fine either way,
> it's just a matter of taste.
>
The client wants the call to be blocking. Out of 'zero', 'infinity'
and some 'valid' delay, it makes better sense to have 'infinity' than
zero or another value that might be valid for some platform. I assume
1hr to be 'infinity', though I am open to better suggestions. Maybe
put a WARN() ?


>> +     ret = chan->mbox->ops->startup(chan);
>> +     if (ret) {
>> +             pr_err("Unable to startup the chan\n");
>
> Perhaps print the error codes?  Might be helpful to users.
>
OK.


BTW, I have not converted Highbank's PL320 and OMAP's controller and
client drivers. I believe Highbank's can't be converted to DT now and
Suman would want to convert the OMAP himself.

Also, maybe mailbox patches could be upstreamed via, say, arm-soc tree?

Regards,
Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/