[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <538605B7.50209@ti.com>
Date: Wed, 28 May 2014 10:50:15 -0500
From: Suman Anna <s-anna@...com>
To: Jassi Brar <jassisinghbrar@...il.com>,
Mark Brown <broonie@...nel.org>
CC: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Greg KH <gregkh@...uxfoundation.org>,
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 05/27/2014 11:20 PM, Jassi Brar wrote:
> 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.
Yes, I will get to this next week, especially as there are new SoCs like
DRA7 and AM437x that need special handling.
regards,
Suman
>
> 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/
Powered by blists - more mailing lists