Define a new operation status GB_OP_MALFUNCTION, which will be used
to represent that something unexpected happened while handling an
operation. This is intended as an indication similar to a BUG()
call--whatever went wrong should *never* happen and because it's
unexpected we need to treat it as a fatal error.
Define another new operation status GB_OP_UNKNOWN_ERROR, which
will represent the case where an operation ended in error, but
the error was not recognized to be properly represented by one
of the other status values.
Renumber the operation status values, defining those that are
produced by core operations code ahead of those that are more
likely to come from operation handlers. Represent the values in
hexadecimal to emphasize that they must be represented with 8 bits.
The Use 0xff for GB_OP_MALFUNCTION instead of GB_OP_TIMEOUT; the
latter is special, but a malfunction is in a class by itself.
Reorder the cases in gb_operation_status_map() to match their
numeric order.
Map GB_OP_UNKNOWN_ERROR to -EIO in gb_operation_status_map(). Map
GB_OP_MALFUNCTION to -EILSEQ in gb_operation_status_map(), since
that value is used to represent an implementation error.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Hide the setting and getting of the operation result (stored in
operation->errno) behind a pair of accessor functions. Only the
operation core should be setting the result, but operations that
complete asynchronously will need access to the result so expose
the function that provides that.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Everyone keeps doing the same create/send/destroy logic all over the
place, so abstract that out to a simple function that can handle any
arbritrary request and/or response. This will let us save lots of
duplicated logic in the protocol drivers.
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Cancel the operation--not just the request message--if waiting
for a synchronous operation to complete is interrupted. Return
the operation result (which in that case will be -EINTR). The
cancelation will result in the normal operation completion path
being taken before returning.
Make gb_operation_wait() private, since it's only ever used for
for synchronous operations.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
If an operation times out, we need to cancel whatever message it
has in-flight. Do that instead of completing the operation, in the
timeout handler. When the in-flight request message is canceled its
completion function will lead to the proper completion of the
operation.
Change gb_operation_cancel() so it takes the errno that it's
supposed to assign as the result of the operation.
Note that we want to preserve the original -ETIMEDOUT error, so
don't overwrite the operation result value if it has already been
set.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Grab an extra reference to an operation before sending it. Drop
that reference at the end of its completion handling.
It turns out gb_operation_get() got deleted along the way, so this
re-introduces it. We're assuming we only get a reference when
there's at least one in existence so we don't need a semaphore to
protect it. Emphasize this by *not* returning a pointer to
the referenced operation.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Change the operation "receive workqueue" to be just the operation
"workqueue". All it does is complete an operation in non-atomic
context. This is all that's required for an outgoing request.
Similarly, ignore any notion that a response will only exist
for outgoing requests in gb_operation_cancel().
I'm doing this in the interest of getting the outgoing request path
verified without the encumbrance of any preconceptions about how
incoming requests need to work. When I finally turn my full
attenion to incoming requests I'll adapt the code as needed.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
An in-core operation structure tracks the progress of an operation.
Currently it holds a result field that was intended to take the
status value that arrives in an operation response message header.
But operations can fail for reasons other than that, and it's
inconvenient to try to represent those using the operation status
codes.
So change the operation->result field to be an int, and switch to
storing negative errno values in it. Rename it "errno" to make
it obvious how to interpret the value.
This patch makes another change, which simplifies the protocol drivers
a lot. It's being done as part of this patch because it affects all
the same code as the above change does. If desired I can split this
into two separate patches.
If a caller makes a synchronous gb_operation_request_send() request
(i.e., no callback function is supplied), and the operation request
and response messages were transferred successfully, have
gb_operation_request_send() return the result of the request (i.e.,
operation->errno). This allows the caller (or more generally, any
caller of gb_request_wait() to avoid having to look at this field
for every successful send.
Any caller that does an asynchronous request will of course need
to look at request->errno in the callback function to see the
result of the operation.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Define greybus_data_sent(), which is a callback the host driver
makes when a buffer send request has completed. The main use for
this is to actively detect errors that can occur while sending.
(Something like this existed at one time and was removed.)
This also defines gb_hd_message_find(), which looks up a message
pointer associated with a buffer sent over a given host device.
This is now a pretty trival mapping.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Embed the buffer for message data into the message structure itself.
This allows us to use a single allocation for each message, and
more importantly will allow us to derive the message structure
describing a message from the buffer itself.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Have an operation's request and response messages be dynamically
allocated rather than embedded in an operation.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
The beginning of an operation message always contains the message
header. Rename the "buffer" field in an operation message to
be "header" to reflect this. Change its type as well.
The size of a message is the combined size of its header and its
payload. Rename the "buffer_size" field in a message header to
be simply "size", so message->size describes exactly that.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Define a common function that maps an operation status value to a
Linux negative errno.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
We (sort of) maintain the status of each message, but we shouldn't
need to. Right now we're not using it consistently in any case.
If a message fails to send, the caller will know to destroy the
operation that contained it.
If a message has been sent (i.e., handed to the host device layer)
it'll have a non-null cookie pointer.
If a does complete in error, we can update the status of the
operation that contains it. That isn't happening right now but
it will soon.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
We can derive the destination CPort id of any (outbound) message
from the connection it's operation is associated with. So we don't
need to store that information in every message.
As a result, we no longer need to record it at message initialization
time.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
The host device pointer doesn't have to be stored in every message.
It can be derived by following up the chain of pointers back to
the operation's connection.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
All the code has now been adjusted such that we can do away with the
old gbuf structure.
Three unused references remained in "greybus.h", so those are deleted.
Other than that most of the changes were done by simple global
substitution. The gb_message structure incorporates the fields that
were previously found its embedded gbuf structure. A few names have
been changed in the process:
gbuf->transfer_buffer message->buffer
gbuf->transfer_buffer_size message->buffer_size
gbuf->hcd_data; message->cookie
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Rework gb_connection_operation_recv() to be more oriented toward an
operation message, and to no longer use a struct gbuf local variable.
Rename it to be a little more wieldy.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Embed the gbuf structures for operation messages into the message
structure rather than pointing to a dynamically allocated one.
Use a null gbuf->transfer_buffer pointer rather than a null gbuf
pointer to indicate an unused gbuf.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
A Greybus buffer (gbuf) is a generic buffer used for data transfer
over a Greybus interconnect. We only ever use gbufs in operations,
which always involve exactly two of them. The lifetime of a gbuf is
therefore directly connected to the lifetime of an operation, so
there no real need to manage gbufs separate from operations.
This patch begins the process of removing the gbuf abstraction, on
favor of a new data type, gb_message. The purpose of a gb_message
is--like a gbuf--to represent data to be transferred over Greybus.
However a gb_message is oriented toward the more restrictive way
we do Greybus transfers--as operation messages (either a request or
a response).
This patch simply defines the structure in its initial form, and
defines the request and response fields in a Greybus operation
structure as embedded instances of that type. The gbuf pointer
is defined within the gb_message structure, and as a result lots
of code needs to be tweaked to reference the request and response
gbufs as subfields of the request and response structures.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
We no longer need struct gbuf defined in "greybus.h". An upcoming
patch will embed a gbuf struct (not a pointer) into the operation
structure, and to do that we'll need the struct defined prior to the
operation. Just move the gbuf definition into "operation.h".
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Add a reference counter to the operations structure. We'll
need this when operations are actually allowed to complete
asynchronously.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
This patch does some cleanup of gb_connection_operation_recv().
- Improve the header comments
- Verify message is big enough for header before interpreting
beginning of the message as a header
- Verify at buffer creation time rather than receive time that
no operation buffer is bigger than the maximum allowed. We
can then compare the incoming data size against the buffer.
- When a response message arrives, record its status in the
operation result, not in the buffer status.
- Record a buffer overflow as an operation error.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Greg raised the alarm when I first put in the red-black tree for
tracking pending operations. The reality as that we're not likely
to have that many operations in flight at any one time, so the
complexity of the red-black tree is most likely unwarranted. I
already
This pulls out the red-black tree and uses a simple list instead. A
connection maintains two lists of operations. An operation starts
on its connection's operations list. It is moved to the pending
list when its request message is sent. And it is moved back to
the operations list when the response message arrives. It is
removed from whatever list it's in when the operation is destroyed.
We reuse the single operation->links field for both lists.
Only outgoing requests are ever "pending." Incoming requests are
transient--we receive them, process them, send the response, and
then we're done.
Change a few function names so it's clear we're working with the
pending list.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
An operation is what can timeout, not a connection itself. So notify
the operation timedout, and the connection can then do with it as it
sees fit, if necessary.
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Define a new function operation_cancel() that cancels an
outstanding operation. Use it to clear out any operations that
might be pending at the time a connection is torn down.
Note: This code isn't really functional yet, partially because
greybus_kill_gbuf() is not implemented.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
It is expected that i2c writes may fail, and in that case the driver
simply retries some number of times before actually treating it as a
failure. Define a GB_OP_RETRY status, which is interpreted by the
i2c driver as an indication a retry is in order. We just translate
that into an EAGAIN error passed back to the i2c core.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Create a work queue to do the bulk of processing of received
operation request or response messages.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Give the operation layer a chance to examine incoming data so that
it can handle it appropriately.
Treat the data as an operation message header. If it's a response,
look up the operation it's associated with. If it's not, create a
new operation. Copy the incoming data into the request or response
buffer. The next patch adds a work queue to pick up handling
the request or response from there.
Get rid of gb_operation_submit(). Instead, we have two functions,
one for sending an operation's request message, the other for
sending an operation's response message.
Not fully functional yet, still just filling things in...
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Add a red-black tree indexed by operation id to a connection to
allow pending operations (whose requests are in-flight) to be
found when their matching response is recieved.
Assign the id at the time an operation is inserted, and update
the operation's message header(s) to include it.
Rename gb_connection_op_id() to be more consistent with the
naming conventions being used elsewhere.
(Noting now that this may switch to a simple list implementation
based on Greg's assertion that lists are faster than red-black trees
for up to a few hundred entries.)
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
We need to track both request messages and response messages in
operations. So add another gbuf (and payload pointer) field to
the operation structure, and rename them to indicate which one
is which. Allow the creator specify the size of the response
buffer; just leave it a null pointer if the size is 0.
Define a new helper function gb_operation_gbuf_create() to
encapsulate creating either a request or a response buffer.
Any buffer associated with a connection will (eventually) have been
created as part of an operation. So stash the operation pointer in
the gbuf as the context pointer. Whether a buffer is for the
request or the response can be determined by pointer comparison.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
The type of an operation belongs in the operation header, which
shouldn't be touched by users of the interface. So specify it
at operation creation time.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
This patch defines a new "operation" abstraction. An operation is a
request from by one end of a connection to the function (or AP) on
the other, coupled with a matching response returned to the requestor.
The request indicates some action to be performed by the target of
the request (such as "read some data"). Once the action has
completed the target sends back an operation response message.
Additional data can be supplied by the sender with its request,
and/or by the target with its resposne message.
Each request message has a unique id, generated by the sender.
The sender recognizes the matching response by the presence
of this id value. Each end of a connection is responsible
for creating unique ids for the requests it sends.
An operation also has a type, whose interpretation is dependent on
the function type on the end of the connection opposite the sender.
It is up to the creator of an operation to fill in the data (if any)
to be sent with the request.
Note that not all requests are initiated by the AP. Incoming data
on a module function can result in a request message being sent from
that function to the AP to notify of the data's arrival. Once the
AP has processed this, it sends a response to the sender.
Every operation response contains a status byte. If it's value
is 0, the operation was successful. Any other value indicates
an error.
Add a defintion of U16_MAX to "kernel_ver.h".
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
