linux-kernel - Re: [RFC 3/3] mailbox: pl320: Introduce common API driver

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]

Message-ID: <CAJe_Zhe6=8F=HPZPw1xkDbuyALHK6iwJ-uZKq50yE7vQ6Bp4BQ@mail.gmail.com>
Date:	Mon, 29 Apr 2013 22:58:23 +0530
From:	Jassi Brar <jaswinder.singh@...aro.org>
To:	Mark Langsdorf <mark.langsdorf@...xeda.com>
Cc:	Suman Anna <s-anna@...com>, Jassi Brar <jassisinghbrar@...il.com>,
	"Loic PALLARDY (loic.pallardy@...com)" <loic.pallardy@...com>,
	Arnd Bergmann <arnd@...db.de>,
	lkml <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Andy Green <andy.green@...aro.org>
Subject: Re: [RFC 3/3] mailbox: pl320: Introduce common API driver

On 29 April 2013 22:36, Mark Langsdorf <mark.langsdorf@...xeda.com> wrote:
> On 04/29/2013 11:57 AM, Jassi Brar wrote:
>> On 29 April 2013 22:14, Suman Anna <s-anna@...com> wrote:
>>> On 04/27/2013 01:14 PM, jassisinghbrar@...il.com wrote:
>>>> From: Jassi Brar <jaswinder.singh@...aro.org>
>>>> Signed-off-by: Jassi Brar <jaswinder.singh@...aro.org>
>>>> ---
>>>>  drivers/cpufreq/highbank-cpufreq.c       |   22 +++-
>>>>  drivers/mailbox/Makefile                 |    2 +-
>>>>  drivers/mailbox/{pl320-ipc.c => pl320.c} |  194 ++++++++++++++++--------------
>>>>  include/linux/pl320-ipc.h                |   17 ---
>>>>  4 files changed, 125 insertions(+), 110 deletions(-)
>>>>  rename drivers/mailbox/{pl320-ipc.c => pl320.c} (51%)
>>>>  delete mode 100644 include/linux/pl320-ipc.h
>>>>
>>>> diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
>>>> index 3118b87..5c057e0 100644
>>>> --- a/drivers/cpufreq/highbank-cpufreq.c
>>>> +++ b/drivers/cpufreq/highbank-cpufreq.c
>>>> @@ -19,7 +19,7 @@
>>>>  #include <linux/cpu.h>
>>>>  #include <linux/err.h>
>>>>  #include <linux/of.h>
>>>> -#include <linux/pl320-ipc.h>
>>>> +#include <linux/mailbox_client.h>
>>>>  #include <linux/platform_device.h>
>>>>
>>>>  #define HB_CPUFREQ_CHANGE_NOTE       0x80000001
>>>> @@ -29,8 +29,26 @@
>>>>  static int hb_voltage_change(unsigned int freq)
>>>>  {
>>>>       u32 msg[HB_CPUFREQ_IPC_LEN] = {HB_CPUFREQ_CHANGE_NOTE, freq / 1000000};
>>>> +     struct ipc_client cl;
>>>> +     int ret = -ETIMEDOUT;
>>>> +     void *chan;
>>>>
>>>> -     return pl320_ipc_transmit(msg);
>>>> +     cl.rxcb = NULL;
>>>> +     cl.txcb = NULL;
>>>> +     cl.tx_block = true;
>>>> +     cl.tx_tout = 1000; /* 1 sec */
>>>> +     cl.cntlr_data = NULL;
>>>> +     cl.knows_txdone = false;
>>>> +     cl.chan_name = "pl320:A9_to_M3";
>>>> +
>>>> +     chan = ipc_request_channel(&cl);
>>>> +
>>>> +     if (ipc_send_message(chan, (void *)msg))
>>>> +             ret = msg[1]; /* PL320 updates buffer with FIFO after ACK */
>>>> +
>>>> +     ipc_free_channel(chan);
>>>
>>> I think I understand why you have done this, but do you really want to
>>> request and free every time in the highbank cpufreq driver?
>>>
>> Exactly my aim - make the API light enough to enable the client to
>> use-and-throw. And also because the channel are exclusively assigned,
>> acquire a channel only for as long as you need it.
>
> Do you have any numbers on the performance impact? Our cpufreq
> transition throughput is bad enough without adding additional delays.
>
Sorry no numbers.
However if you look closely you'll find for your usecase the message
directly reaches the controller and the reply from remote is directly
handed to your client driver. And since there is no context for
channel on your platform channel request/free shouldn't fail either
(or you could keep the channel allocated for the lifetime of the
client driver). So I would not expect any longer delays than the
original way.
 Compared with TI's framework of RX-via-notifier you should be better
off with this API, imho.

cheers.
--
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/