On Wednesday 04 February 2015 08:57:43 Jassi Brar wrote:
On 3 February 2015 at 20:55, Arnd Bergmann [off-list ref] wrote:
quoted
On Tuesday 03 February 2015 14:46:11 Russell King - ARM Linux wrote:
quoted
On Tue, Feb 03, 2015 at 08:09:34PM +0530, Jassi Brar wrote:
quoted
I had expected to see here something like:
static int mhu_send_data(struct mbox_chan *chan, void *data)
{
struct mhu_link *mlink = chan->con_priv;
u32 *arg = data;
writel_relaxed(*arg, mlink->tx_reg + INTR_SET_OFS);
}
i.e. dereferencing the pointer instead of using the actual value.
OK, just curious how is this (dereferencing to the u32 variable on
stack of the client driver) better?
The API as I understand is defined to use the pointer to point to
a chunk of data of fixed size, with the size being known to both
the client driver and the mailbox driver. This is the reason for
having a pointer in the first place.
Using the bits of the pointer as the message instead of pointing
to the message feels like an abuse of the API.
Maybe it would have been better to pass the size explictly as
a third argument in the API to make that clear.
Arnd
On 4 February 2015 at 15:59, Arnd Bergmann [off-list ref] wrote:
On Wednesday 04 February 2015 08:57:43 Jassi Brar wrote:
quoted
On 3 February 2015 at 20:55, Arnd Bergmann [off-list ref] wrote:
quoted
On Tuesday 03 February 2015 14:46:11 Russell King - ARM Linux wrote:
quoted
On Tue, Feb 03, 2015 at 08:09:34PM +0530, Jassi Brar wrote:
quoted
quoted
I had expected to see here something like:
static int mhu_send_data(struct mbox_chan *chan, void *data)
{
struct mhu_link *mlink = chan->con_priv;
u32 *arg = data;
writel_relaxed(*arg, mlink->tx_reg + INTR_SET_OFS);
}
i.e. dereferencing the pointer instead of using the actual value.
OK, just curious how is this (dereferencing to the u32 variable on
stack of the client driver) better?
The API as I understand is defined to use the pointer to point to
a chunk of data of fixed size, with the size being known to both
the client driver and the mailbox driver. This is the reason for
having a pointer in the first place.
Yes, we are on the same page. I just have a slightly more liberal view
about the usage of 'void* data'.
Using the bits of the pointer as the message instead of pointing
to the message feels like an abuse of the API.
I can see your POV.
Now consider a client, like mine, that sends a u32 value as the data.
But unlike me, the client uses the mailbox api in 'async' mode i.e,
register a callback function, submit a 32bit message and move on. It
is perfectly doable, but doesn't kalloc'ing a u32 for each submission,
seem overkill? Lets say what the client and controller drivers do in
their bedroom is none of the API's business.
Maybe it would have been better to pass the size explictly as
a third argument in the API to make that clear.
Actually that did come up for consideration. But since the structure
of 'data' packet is already known to the controller driver, having to
also tell the size of that structure seems redundant.
Thanks.
On Wed, Feb 04, 2015 at 11:29:55AM +0100, Arnd Bergmann wrote:
On Wednesday 04 February 2015 08:57:43 Jassi Brar wrote:
quoted
On 3 February 2015 at 20:55, Arnd Bergmann [off-list ref] wrote:
quoted
On Tuesday 03 February 2015 14:46:11 Russell King - ARM Linux wrote:
quoted
On Tue, Feb 03, 2015 at 08:09:34PM +0530, Jassi Brar wrote:
quoted
quoted
I had expected to see here something like:
static int mhu_send_data(struct mbox_chan *chan, void *data)
{
struct mhu_link *mlink = chan->con_priv;
u32 *arg = data;
writel_relaxed(*arg, mlink->tx_reg + INTR_SET_OFS);
}
i.e. dereferencing the pointer instead of using the actual value.
OK, just curious how is this (dereferencing to the u32 variable on
stack of the client driver) better?
The API as I understand is defined to use the pointer to point to
a chunk of data of fixed size, with the size being known to both
the client driver and the mailbox driver. This is the reason for
having a pointer in the first place.
Using the bits of the pointer as the message instead of pointing
to the message feels like an abuse of the API.
I agree on those two points. However, passing the address of something
on the stack to mbox_send_message() is also not particularly on - it
may save the pointer to use later on if its operating on non-blocking
mode.
A possible alternative would be if the user of mbox_send_message()
stored the message in an array, operated as a circular buffer, and
passed the address of the word to send. That would avoid the need
to repeatedly allocate and free memory (which would be expensive for
the sake of a u32.)
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.