| Age | Commit message (Collapse) | Author |
|---|
|
Add a struct timer_list to struct gb_operation and use that to implement
generic operation timeouts.
This simplifies the synchronous operation handling somewhat while also
providing a generic timeout mechanism that drivers can use for
asynchronous operations.
Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Add a new core operation flag and a helper that core can use to create
core operations.
This will be used to implement the ping operations that core sends as
part of connection tear down.
Note that a new trace point is also added.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
The invalid request type has been redefined as 0x7f.
Also remove the redundant redefinition of the invalid type from the
operation header.
Note that operation type 0x00 has been repurposed for the new generic
ping operation, which will be used to implement proper connection tear
down.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add helper functions for initiating unidirectional operations and
waiting for them to have been acknowledged as sent by the host device.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add support for initiating unidirectional operations, that is, sending
requests that do not require responses.
Note that we already handle incoming unidirectional operations.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add support for operations with short responses.
So far we have assumed that the initiator of an operation always knows
the exact size of the expected response. This is however not always the
case and we've worked around this limitation in a couple of places by,
for example, first requesting the size of a resource before fetching the
actual data.
To avoid such workarounds and simplify our protocols, add a
short-response flag that can be set when allocating an operation. When
this flag is set on an operation, core will accept a response that is
shorter than the size of the (pre-allocated) response payload buffer.
For now, we update the response-message payload_size field to reflect
the actual length of the response received.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Rename host-device structure gb_host_device to match our other
structures.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Remove legacy interface to "destroy" operations, which is now just a
wrapper for gb_operation_put.
The old interface name hides the fact that all operations are refcounted
and may live on even after having "destroyed" them.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
This should be exposed to external users (like gbsim). Move it to
greybus_protocols.h.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
The buffers allocated for message header is already 64 bit aligned and
we have explicit pad bytes in the header structure, to 64 bit align the
operation specific data.
And so there is no need to add the aligned attribute to the operation
message header. Drop it.
Suggested-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Response allocation also needs a GFP-flags argument as a response is
allocated as part of an outgoing operation.
Fixes: 9aa174d202e5 ("operation: allow atomic operation allocations")
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add gb_operation_sync_timeout convenience function, which allows drivers
to configure the operation timeout.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add new interface gb_operation_request_send_sync_timeout, which allows
drivers to define a custom operation timeout instead of the default
one-second timeout.
The timeout is expected to depend on protocol and operation and
therefore needs to be configurable.
Note that that a timeout of zero is used to wait indefinitely.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Split incoming and outgoing operation-cancellation helpers.
Incoming operations are only cancelled as part of connection tear down
and is specifically not needed in the driver API.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Fix connection tear down, which was done in an unsafe way that could
result in use-after-free as the per-connection list of operations was
iterated without any locking or refcounting.
Specifically, the operations list was iterated without holding any locks or
operation refcounts even though operations were being both removed from
the list and deallocated during per-operation cancellation. Any
operation completing during tear down could also cause corruption.
Change the per-connection operation list to only contain active
operations and use the recently introduced active counter to maintain
the list.
Add new helper that is called on connection tear down to cancel all
outstanding operations in a safe way by using proper locks and making
sure to hold a reference to any operation being cancelled.
Note that by verifying the connection state before incrementing the
active count we can make sure that all active operations have been
cancelled and that no new ones have been started when the helper
returns.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Make sure to wait for the operation to become inactive before returning
after having cancelled an operation.
This makes sure that any ongoing operation completion callbacks have
finished.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add active counter to track operations that are in use.
Note that the active count is always less than the reference count.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Remove obsolete operation-field comments.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add flag to identify unidirectional operations.
Use convenience helper rather than open coding the identification when
suppressing response messages.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add flag field to struct gb_operation, and a first flag
GB_OPERATION_FLAG_INCOMING to identify incoming operations.
Pass an initial set of flags when allocating new operations, and use
these to identify incoming operations rather than overloading the
meaning of GB_OPERATION_TYPE_INVALID. This also allows us to set the
type for all operations during allocation.
Also add convenience helper to identify incoming operations.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Since commit 46ce118a2678 ("operation: refactor response handling")
sending operation responses is handled by greybus core so there is
currently no need to export the response helper.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add gfp mask argument to gb_operation_request_send in order to allow
submissions from atomic context.
Note that responses are currently always sent from non-atomic context as
incoming requests are processed in a work queue.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add gfp mask argument to gb_operation_create to allow operations to be
allocated in atomic context.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
The current host-controller message-cancellation implementation suffer
from a lifetime bug as dynamically allocated URBs would complete and be
deallocated while being unlinked as part of cancellation.
The current locking is also insufficient to prevent the related race
where the URB is deallocated before being unlinked.
Fix this by pushing the cancellation implementation from greybus core
down to the host-controller drivers, and replace the "cookie" pointer
with a hcpriv field that those drivers can use to maintain their state
with the required locking and reference counting in place.
Specifically the drivers need to acquire a reference to the URB under a
lock before calling usb_kill_urb as part of cancellation.
Note that this also removes the insufficient gb_message_mutex, which
also effectively prevented us from implementing support for submissions
from atomic context.
Instead the host-controller drivers must now explicitly make sure that
the pre-allocated URBs are not reused while cancellation is in progress.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
__init does not belong in a .h file, as it does not do anything there,
so remove all instances of it.
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Define init and exit functions to do one-time setup and teardown
of endo-related functionality. Currently they're place holders;
the next patch will populate them.
Note that we now call gb_operation_exit() from gb_init(), so
we can no longer mark that function with __exit.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
The top-level functions gb_init() and gb_exit() are tagged with
__init and __exit, respectively. These functions call a few
other functions that are similarly used only at initialization
and termination time. So mark those functions accordingly.
Note that, because gb_ap_exit() and gb_debugfs_cleanup()
are called by gb_init() in error paths, these functions
cannot be declared with the __exit attribute.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add helper to retrieve the maximum payload size for operations on a
specific connection.
Note that the helper is not inlined due to how the header files are
currently organised, but it is not expected to be called after a
connection has been initialised either.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Make sure we never end up with a host device with maximum buffer size
smaller than the shortest Greybus message.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Increase the maximum allowed buffer size to the full 16-bit range
supported by the protocol.
Note that host devices will generally use smaller buffers than the
maximum.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Host devices impose buffer-size constraints on Greybus core which are
taken into account when allocating messages.
Make sure to verify these constraints when the host device is allocated,
rather than when the first message is allocated.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
All protocols use the same value to distinguish between request and
response message types. This is a requirement.
Use GB_MESSAGE_TYPE_RESPONSE rather than GB_OPERATION_TYPE_RESPONSE
for the name of the flag used to distiguish between request and
response messages.
Get rid of the redundant response flag definitions that are
associated with specific protocols.
Describe the symbolic values as "operation types" rather than
"message types" where they are defined. The message type for a
request is the same as the operation type; the message type for a
response is the operation type OR'd with GB_MESSAGE_TYPE_RESPONSE.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Add explicit pad bytes to the message header.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Make sure to allocate the message transfer-buffer separately from the
containing message structure to avoid data corruption on systems without
DMA-coherent caches.
The message structure contains state that is updated while the buffer
may be used for DMA, something which could lead to data corruption due
to cache-line sharing on some architectures.
Use the (renamed) message cache for the message structure itself and
allocate the buffer separately.
If the additional allocation is a concern, the message structures
could eventually be allocated as part of the operation structure.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Pass structured greybus messages rather than buffers to the host
drivers.
This will allow us to separate the transfer buffers from the message
structures.
Rename the related functions to reflect the new interface.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
Move operation message-header to operation.h so that it can be used
by host drivers.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
|
|
If an operation is issued and the response never comes back,
gb_operation_timeout() cancels the operation but never wakes up the
waiter in gb_operation_request_send().
This patch removes the timeout workqueue and changes the request wait to
wait_for_completion_interruptible_timeout(), with timeout set to
OPERATION_TIMEOUT_DEFAULT.
Signed-off-by: Perry Hung <perry@leaflabs.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
I was asked to add a Linaro copyright to all Greybus source files
that anyone at Linaro has modified. This patch does that.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
The i2c protocol needs a way to indicate an i2c device doesn't exist
(which is not necessarily an error). Define GB_OP_NONEXISTENT to
indicate this, and updating the status<->errno mapping functions
accordingly.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
I've gone back and forth on this, but now that I'm looking at
asynchronous operations I know that the asynchronous callback will
want to know what type of operation it is handling, and right now
that's only available in the message header.
So record an operation's type in the operation structure, and use
it in a few spots where the header type was being used previously.
Pass the type to gb_operation_create_incoming() so it can fill
it in after the operation has been created.
Clean up the crap comments above the definition of the operation
structure.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
An asynchronous operation will want to know how big the response
message it receives is. Rather than require the sender to record
that information, expose a new field "payload_size" available to
the protocol code for this purpose.
An operation message consists of a header and a payload. The size
of the message can be derived from the size of the payload, so
record only the payload size and not the size of the whole message.
Reorder the fields in a message structure.
Update the description of the message header structure.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
Define a new function used to initiate a synchronous operation.
It sends the operation request message and doesn't return until
the response has been received and/or the operation's result
has been set.
This gets rid of the convention that a null callback pointer
signifies a synchronous operation.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
A connection has two lists of operations, and an operation is always
on one or the other of them. One of them contains the operations
that are currently "in flight".
We really don't expect to have very many in-flight operations on any
given connection (in fact, at the moment it's always exactly one).
So there's no significant performance benefit to keeping these in a
separate list. An in-flight operation can also be distinguished by
its errno field holding -EINPROGRESS.
Get rid of the pending list, and search all operations rather than
the pending list when looking up a response message's operation.
Rename gb_pending_operation_find() accordingly.
There's no longer any need to remove operations from the pending
list, and the insertion function no longer has anything to do with a
pending list. Just open code what was the insertion function (it
now has only to do with assigning the operation id).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
Use a symbolic constant (rather than just "0") to represent an
explicitly invalid operation type. The protocols have all reserved
that value for that purpose--this just makes it explicit in the core
code (since we now leverage its existence). Fix the code so it uses
the new symbolic value.
Define it in "operation.h" for all to see. Move the common
definition of the GB_OPERATION_TYPE_RESPONSE flag mask there
as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
Define a helper function gb_operation_response_alloc() and use it
to allocate the response buffer for outgoing operations in
gb_operation_create_common(.
Use it also in gb_operation_response_send() if the caller has not
allocated a response buffer.
Once a response buffer is allocated, fill in its result code and
send it.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
Define gb_operation_errno_map(), which maps an operation->errno
into the u8 value that represents it in the status field of an
operation response header. It'll be used in an upcoming patch.
Make gb_operation_status_map() a private function. It's not used
outside "operation.c" and I don't believe it ever should be.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
Change gb_operation_response_send() so it takes an errno to assign
as an operation's result. This emphasizes that setting the result
should be the last thing done to an incoming operation before
sending its response.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
|
|
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>
|
Johan Hovold
Johan Hovold
Viresh Kumar
Greg Kroah-Hartman
Alex Elder
Perry Hung
Greg Kroah-Hartman