All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jassi Brar <jaswinder.singh@linaro.org>
To: Suman Anna <s-anna@ti.com>
Cc: Jassi Brar <jassisinghbrar@gmail.com>,
	"Loic PALLARDY (loic.pallardy@st.com)" <loic.pallardy@st.com>,
	Arnd Bergmann <arnd@arndb.de>,
	lkml <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCHv2 2/4] mailbox: Introduce a new common API
Date: Fri, 10 May 2013 00:19:31 +0530	[thread overview]
Message-ID: <CAJe_Zhffm9FOk83dAdX194gN3r5JnG78sdcWNv6WnWCsTF3YuA@mail.gmail.com> (raw)
In-Reply-To: <518BE566.5090504@ti.com>

On 9 May 2013 23:35, Suman Anna <s-anna@ti.com> wrote:

>>
>> Perhaps we should change the following
>>
>>    void ipc_link_txdone(struct ipc_link *link, enum xfer_result r)
>> to
>>    void ipc_link_txdone(struct ipc_link *link, enum xfer_result r, void *data)
>>
>> So that the API could pass that onto clients ?
>
> That's if the controller needs to pass some data back to client. I am
> fine with that as well,
No, I misunderstood you wanted request_token_t to be replaced with the
pointer of request that was executed.

> but I am talking mainly about providing a client
> user data ptr back to it during callbacks.
>
> struct ipc_client {
>         char *chan_name;
> +       void *cl_data; /* store it to ipc_chan as well */
> -       void (*rxcb)(void *data);
> -       void (*txcb)(request_token_t t, enum xfer_result r);
> +       void (*rxcb)(void *cl_data, void *data);
> +       void (*txcb)(request_token_t t, enum xfer_result r, void *cl_data);
>         ...
> }
>
> I am obviously interested in the rxcb. The controller implementations do
> not see the cl_data.
>
OK I see what you mean. However the API storing and passing back
ad-hoc data to clients doesn't seem very neat.

Such purposes are usually served by :

- void (*rxcb)(void *data);
+ void (*rxcb)(struct ipc_client *cl, void *data);  /* client for
which data was received */

- void (*txcb)(request_token_t t, enum xfer_result r);
+ void (*txcb)(struct ipc_client *cl, request_token_t t, enum
xfer_result r); /* client whose data was sent */

You could then get relevant omap_rproc using container_of() on 'cl',
in rxcb() and txcb().


Apart from this, in txcb, perhaps we should drop request_token_t in
favor of the request's pointer (void *data) that was last executed.
That should make things easier for clients.

regards,
-jassi

WARNING: multiple messages have this Message-ID (diff)
From: jaswinder.singh@linaro.org (Jassi Brar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 2/4] mailbox: Introduce a new common API
Date: Fri, 10 May 2013 00:19:31 +0530	[thread overview]
Message-ID: <CAJe_Zhffm9FOk83dAdX194gN3r5JnG78sdcWNv6WnWCsTF3YuA@mail.gmail.com> (raw)
In-Reply-To: <518BE566.5090504@ti.com>

On 9 May 2013 23:35, Suman Anna <s-anna@ti.com> wrote:

>>
>> Perhaps we should change the following
>>
>>    void ipc_link_txdone(struct ipc_link *link, enum xfer_result r)
>> to
>>    void ipc_link_txdone(struct ipc_link *link, enum xfer_result r, void *data)
>>
>> So that the API could pass that onto clients ?
>
> That's if the controller needs to pass some data back to client. I am
> fine with that as well,
No, I misunderstood you wanted request_token_t to be replaced with the
pointer of request that was executed.

> but I am talking mainly about providing a client
> user data ptr back to it during callbacks.
>
> struct ipc_client {
>         char *chan_name;
> +       void *cl_data; /* store it to ipc_chan as well */
> -       void (*rxcb)(void *data);
> -       void (*txcb)(request_token_t t, enum xfer_result r);
> +       void (*rxcb)(void *cl_data, void *data);
> +       void (*txcb)(request_token_t t, enum xfer_result r, void *cl_data);
>         ...
> }
>
> I am obviously interested in the rxcb. The controller implementations do
> not see the cl_data.
>
OK I see what you mean. However the API storing and passing back
ad-hoc data to clients doesn't seem very neat.

Such purposes are usually served by :

- void (*rxcb)(void *data);
+ void (*rxcb)(struct ipc_client *cl, void *data);  /* client for
which data was received */

- void (*txcb)(request_token_t t, enum xfer_result r);
+ void (*txcb)(struct ipc_client *cl, request_token_t t, enum
xfer_result r); /* client whose data was sent */

You could then get relevant omap_rproc using container_of() on 'cl',
in rxcb() and txcb().


Apart from this, in txcb, perhaps we should drop request_token_t in
favor of the request's pointer (void *data) that was last executed.
That should make things easier for clients.

regards,
-jassi

  reply	other threads:[~2013-05-09 18:49 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-06  7:22 [PATCHv2 0/4] mailbox: Common API Jassi Brar
2013-05-06  7:22 ` Jassi Brar
2013-05-06  7:23 ` [PATCHv2 1/4] mailbox: rename pl320-ipc specific mailbox.h Jassi Brar
2013-05-06  7:23   ` Jassi Brar
2013-05-06  7:24 ` [PATCHv2 2/4] mailbox: Introduce a new common API Jassi Brar
2013-05-06  7:24   ` Jassi Brar
2013-05-09 16:31   ` Suman Anna
2013-05-09 16:31     ` Suman Anna
2013-05-09 16:41     ` Jassi Brar
2013-05-09 16:41       ` Jassi Brar
2013-05-09 16:40       ` Suman Anna
2013-05-09 16:40         ` Suman Anna
2013-05-09 17:48         ` Jassi Brar
2013-05-09 17:48           ` Jassi Brar
2013-05-09 18:05           ` Suman Anna
2013-05-09 18:05             ` Suman Anna
2013-05-09 18:49             ` Jassi Brar [this message]
2013-05-09 18:49               ` Jassi Brar
2013-05-09 23:43               ` Suman Anna
2013-05-09 23:43                 ` Suman Anna
2013-05-13 19:09   ` Loic PALLARDY
2013-05-06  7:24 ` [PATCHv2 3/4] mailbox: pl320: Introduce common API driver Jassi Brar
2013-05-06  7:24   ` Jassi Brar
2013-05-07  1:58   ` Rob Herring
2013-05-07  1:58     ` Rob Herring
2013-05-07 16:56     ` Jassi Brar
2013-05-07 16:56       ` Jassi Brar
2013-05-06  7:24 ` [PATCHv2 4/4] mailbox: omap2: " Jassi Brar
2013-05-06  7:24   ` Jassi Brar
2013-05-07  0:02 ` [PATCHv2 0/4] mailbox: Common API Suman Anna
2013-05-07  0:02   ` Suman Anna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJe_Zhffm9FOk83dAdX194gN3r5JnG78sdcWNv6WnWCsTF3YuA@mail.gmail.com \
    --to=jaswinder.singh@linaro.org \
    --cc=arnd@arndb.de \
    --cc=jassisinghbrar@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.pallardy@st.com \
    --cc=s-anna@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.