From: Suman Anna <s-anna@ti.com> To: Jassi Brar <jaswinder.singh@linaro.org> 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: Thu, 9 May 2013 18:43:59 -0500 [thread overview] Message-ID: <518C34BF.7080408@ti.com> (raw) In-Reply-To: <CAJe_Zhffm9FOk83dAdX194gN3r5JnG78sdcWNv6WnWCsTF3YuA@mail.gmail.com> Jassi, >>> >>> 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(). The reason that I didn't suggest that way is because we do not use ipc_client for any runtime API, and we would have to store the returned handle anyway. I see ipc_client simply as a ipc_channel_request_info structure, a one-time usage perspective. I made the suggestion as it seemed in line if you had a xxx_register_callback API wherein you would use a void *context if you want something back. > > 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. Yes, that would be nice too. regards Suman
WARNING: multiple messages have this Message-ID (diff)
From: s-anna@ti.com (Suman Anna) To: linux-arm-kernel@lists.infradead.org Subject: [PATCHv2 2/4] mailbox: Introduce a new common API Date: Thu, 9 May 2013 18:43:59 -0500 [thread overview] Message-ID: <518C34BF.7080408@ti.com> (raw) In-Reply-To: <CAJe_Zhffm9FOk83dAdX194gN3r5JnG78sdcWNv6WnWCsTF3YuA@mail.gmail.com> Jassi, >>> >>> 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(). The reason that I didn't suggest that way is because we do not use ipc_client for any runtime API, and we would have to store the returned handle anyway. I see ipc_client simply as a ipc_channel_request_info structure, a one-time usage perspective. I made the suggestion as it seemed in line if you had a xxx_register_callback API wherein you would use a void *context if you want something back. > > 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. Yes, that would be nice too. regards Suman
next prev parent reply other threads:[~2013-05-09 23: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 2013-05-09 18:49 ` Jassi Brar 2013-05-09 23:43 ` Suman Anna [this message] 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=518C34BF.7080408@ti.com \ --to=s-anna@ti.com \ --cc=arnd@arndb.de \ --cc=jassisinghbrar@gmail.com \ --cc=jaswinder.singh@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=loic.pallardy@st.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: linkBe 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.