greybus: svc: Add helpers to create AP<->SVC connection
SVC connection is required before the AP knows its position on the endo
and type of endo. To enable message processing between the AP and SVC at
this time, we need a partially initialized connection which can handle
these messages.
Once the AP receives more information from the SVC, it can discard this
partially initialized connection and create a proper one, tied to a
bundle and interface.
Destroying the partially initialized connection is a bit tricky, as it
is required to send a response to svc-hello. That part will be properly
fixed separately.
greybus: interface: Update gb_create_control_connection() to support SVC protocol
We need to create bundle/connection for svc cport after the endo layout
and interface id is known to the AP. gb_create_control_connection() can
be reused for this, but it should be renamed to something more
appropriate, as its not about control-connection anymore.
greybus: connection: Allow a bundle-less connection
We need a bundle-less connection for AP's SVC protocol, as that will be
used much before the endo layout and interface-id of the AP is known to
greybus core.
This updates gb_connection_create_range() to take few more arguments,
which were earlier fetched from the 'bundle' pointer.
greybus: connection: Create gb_connection_create_range() to specify hd-cport-id range
We need to allocate specific hd-cport-id for AP's control/svc protocols.
Support that by splitting functionality of gb_connection_create() into a
new routine, which takes range of hd_cport_id's to allocate from.
For now, the plan is to use a single cport for both control and svc
protocol. Defining separate macros for control and svc protocol's
cport/bundle would make the code more flexible, in case we need two
separate cports in future.
Lets define cport/bundle for svc protocol as well.
Johan Hovold [Tue, 14 Jul 2015 13:43:36 +0000 (15:43 +0200)]
greybus: operation: allow drivers to define custom timeouts
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.
Make sure the request handler has submitted the response before
cancelling it during operation cancellation.
This prevents cancelling not-yet-submitted messages. It currently also
avoids us ending up with an active message on a stalled connection (e.g.
due to E2EFC).
Note that the call to gb_operation_result_set() is now redundant but is
kept as a precaution to guarantee that a response has indeed been
allocated as part of response submission.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Tue, 14 Jul 2015 13:43:33 +0000 (15:43 +0200)]
greybus: operation: fix operation look-up race
Make sure to fully initialise the operation before adding it to the
active list when sending a request.
The operation should be fully initialised before adding it to the active
list to avoid racing with operation look up when receiving a response,
something which could potentially lead to a match against some earlier
(or intermediate) value of the id field.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Tue, 14 Jul 2015 13:43:32 +0000 (15:43 +0200)]
greybus: connection: fix protocol tear-down race
Make sure to cancel all active operations before calling protocol
connection_exit to prevent use-after-free issues when the protocol state
is being deallocated (e.g. late processing of already-queued requests or
completions).
Note that already-queued requests or completions will be processed as
part of cancellation.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Tue, 14 Jul 2015 13:43:31 +0000 (15:43 +0200)]
greybus: operation: fix connection tear down
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>
Johan Hovold [Tue, 14 Jul 2015 13:43:27 +0000 (15:43 +0200)]
greybus: operation: complete operations on cancellation
Make sure to call the operation completion callback also when the
operation is being cancelled.
The completion callback may need to release resources allocated at
submission and the driver should be informed that the operation has
failed due to cancellation.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fixing needless redefinition of operation types in gbsim reveals this typo
GB_UART_TYPE_SET_BREAK -> GB_UART_TYPE_SEND_BREAK. This patch should be
applied in lock-step to the patch to gbsim 'gbsim/uart: remove unnecessary
redefinition of operation types' since gbsim does not contain the typo.
greybus: loopback: update throughput metrics to improve granularity
Throughput capture should account for the entire size of the data going out
on the wire. In addition throughput should be captured for each supported
loopback operation.
greybus: loopback: truncate maximum loop data to link size
Get maximum payload by way of gb_operation_get_payload_size_max() and
truncate any requested loopback size greater than this value. RX of data
from firmware over the specified size will not be accepted.
Loopback sink command will operate as an amalgam of the ping and tranfer
operations. Sink will send an ACK'd variable size operation over greybus.
Unlike the transfer type which transmits the received data back, the sink
type will simply ACK without sending the received data back.
This patch adds the kernel side of the sink command.
greybus: loopback: make loopback type input equivalent to protocol type
Sepcifying loopback operation type with a type value that is internal to
the loopback driver is non-intunitive and requires reading code to
understand. Remove confusing duplicate definitions and update code to
accept the greybus-specification function identity defintiions as the
appropriate type values for initiating loopback operations.
See greybus-spec section 10.16.1 'Greybus Loopback Message Types' for a
full list of valid messages to set this type field to.
This wouldn't happen normally, but in a buggy corner case it is possible
that all the connections aren't removed properly and they are still
present after the interfaces and endo are removed.
Warn in such a case and cleanup connections, so that /sys/bus/greybus/
can be removed while removing greybus modules.
greybus: properly cleanup ida and idr structures when shutting down
idr and ida structures have internal memory allocated that needs to be
freed when modules are removed. So call the proper idr_destroy() or
ida_destroy() functions on the module exit path to free the memory.
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com> Reviewed-by: Alex Elder <elder@linaro.org>
greybus: svc: save pointer to struct gb_svc in struct gb_interface
Its another special protocol (just like control protocol) and is
required to be accessed from other files, lets save a pointer to it in
interface structure.
greybus: connection: Exit connection before destroying it
gb_connection_create() can initialize a connection if bundle->device id
is valid. And so the connection must be destroyed by calling
gb_connection_exit() before gb_connection_destroy(). This wasn't done in
the code that is parsing the manifest.
greybus: sdio: fix transfer buffer handling and blocks counting
Fix copy to/from scatterlist destination buffer offset, fix
calculation of blocks to be transfer and make a more verbose out of
error when the blocks receive/send do not match.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
As we do not have, yet, a event callback to notify core about changes
we add the MMC_CAP_NEEDS_POLL capability to the rest of the caps
received from the module.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Between the time connection with module is up and the host is added,
we can receive events (card inserted/removed, write protection
switch), so until the setup is complete we queue the events received
and handle them after.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Wed, 1 Jul 2015 10:37:31 +0000 (12:37 +0200)]
greybus: operation: fix cancellation of responses
An operation with a response in-flight will already have set the
operation result and would therefore never be cancelled by the current
implementation.
Note that the reference taken when sending the result will be dropped
in the message completion callback.
Also note that an incoming, non-unidirectional messages will always have
an allocated response if its result has been set.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Wed, 1 Jul 2015 10:37:29 +0000 (12:37 +0200)]
greybus: operation: drop redundant oom message
Drop redundant OOM message, which would already have been logged by the
memory subsystem.
Also remove a FIXME about possibly sending a pre-allocated response,
which is neither possible or desirable. If we ever run out of memory we
have bigger problems then getting a response back to firmware.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Wed, 1 Jul 2015 10:37:26 +0000 (12:37 +0200)]
greybus: operation: add incoming-operation flag
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>
The final reference of an operation will be put after its completion
handler has run, so we must not drop the reference if it has already
been scheduled to avoid use-after-free.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Wed, 1 Jul 2015 10:37:24 +0000 (12:37 +0200)]
greybus: operation: make response helper static
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>
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>
The responsibility of this routine is to configure ARA switch to
establish a connection between a cport on the AP and a cport on an
interface.
The SVC protocol is responsible for such requests and we already have
connection_{create|destroy}_operation() to take care of this.
Moreover, this request is not served by the firmware or gbsim today and
they just print a debug message on this request. And so it is safe to
drop svc_set_route_send() function and fix the required functionality of
configuring the switch in svc protocol driver later.
greybus: connection: intf_cport_id is local to an interface
intf_cport_id is local to an interface and same intf_cport_id can be
used for two separate interface blocks. For finding a connection for an
interface, we are matching intf_cport_id of the connection with
cport_id, passed as argument. This is wrong, as it can match for a
connection on a different interface block.
Fix it by also comparing interface block for which connection is
requested.
greybus: connection: bind protocol after the connection is operational
We may bind protocol with a connection from gb_connection_create(), if
bundle's device_id is already set. That's not the case until now.
But if the protocol is initialized with a call to
protocol->connection_init() from this place, kernel will crash.
This will happen because the connection isn't fully initialized yet, for
example its operation list isn't initialized yet. And as soon as the
protocol driver tries to send a request to the module from its
connection_init() callback, we will add an operation to this
uninitialized list. And it will crash while doing:
prev->next = new;
Try to bind the connection with a protocol only after the connection is
ready for operations.
Bryan O'Donoghue [Mon, 29 Jun 2015 17:09:15 +0000 (18:09 +0100)]
greybus: uart: Update UART to reflect field size changes
The greybus UART protocol specification was updated to reduce the size of
the control field in serial-state-request and line-state-request. This
patch updates the kernel protocol driver to reflect the specification
changes. Once applied gbsim changes will be also be updated automatically
since gbsim depends on the header being modified directly.
Bryan O'Donoghue [Mon, 29 Jun 2015 17:09:14 +0000 (18:09 +0100)]
greybus: uart: Add support for UART error signals
After reviewing the UART specification for greybus break, parity, framing
and over-run errors were moved to the receive-data message. This patch
implements that specification change in the UART protocol driver. Matching
code in gbsim has been tested with this change.
Parity/overrun/framing and break signals have been moved to the
receive-data message to more easily associate the signals with the TTY API.
Update the definitions in the protocol header and add a flags field to the
receive-data structure to facilitate transmission of those signal with the
receive-data message.
Mark Greer [Fri, 26 Jun 2015 23:38:46 +0000 (16:38 -0700)]
greybus: gb-audio: Ensure i2c adapter struct exists before using
The current audio code uses i2c_get_adapter() without checking
that a non-NULL pointer is returned (i.e., that the i2c device
actually exists). When that happens, the system panics.
Fix the potential panic by erroring out with -ENODEV when
i2c_get_adapter() returns NULL.
CC: John Stultz <john.stultz@linaro.com> Signed-off-by: Mark Greer <mgreer@animalcreek.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Tue, 23 Jun 2015 12:17:41 +0000 (14:17 +0200)]
greybus: esx: fix null-deref on hotplug events
We must be prepared to receive hotplug events as soon as we submit the
SVC URB. Since commit 2eb8a6a947d7 ("core: don't set up endo until host
device is initialized") this is no longer the case as the endo would not
have been setup, something which may lead to a null-pointer dereference
in endo_get_module_id() when the interface is created (see oops below
with an added dev_dbg for hd->endo).
Fix this by setting up the endo before submitting the SVC URB.
Rui Miguel Silva [Wed, 24 Jun 2015 22:20:26 +0000 (23:20 +0100)]
greybus: sdio: fix defines for older kernels
Some of the options for mmc host, are not defined in older kernels.
MMC_CAP2_HS400_1_2V, MMC_CAP2_HS400_1_8V, MMC_TIMING_MMC_DDR52 and
MMC_TIMING_MMC_HS400.
To not use them for older versions.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Tested-by: Mark Greer <mgreer@animalcreek.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Viresh Kumar [Mon, 22 Jun 2015 11:12:30 +0000 (16:42 +0530)]
greybus: control: Warn if non-control cport/bundles have control protocol/classes
It is possible that (by mistake) the manifest contains non-control
cports with their protocol set as control-protocol or non-control bundle
with their class set as control-class.
Catch such cases, WARN for them and finally ignore them.
Also WARN if the control cport doesn't have its protocol as
control-protocol and control bundle doesn't have its class as
control-class.
Viresh Kumar [Mon, 22 Jun 2015 11:12:27 +0000 (16:42 +0530)]
greybus: interface: Get manifest using Control protocol
Control protocol is ready to be used for fetching manifest. Lets do it.
This changes few things:
- Creates/initializes bundle/connection for control protocol initially
and skips doing the same later.
- Manifest is parsed at link-up now, instead of hotplug which was the
case earlier. This is because we need device_id (provided during
link-up) for registering bundle.
- Manifest is fetched using control protocol.
Viresh Kumar [Mon, 22 Jun 2015 11:12:26 +0000 (16:42 +0530)]
greybus: Add control protocol support
Add control protocol driver that is responsible for handling operations
on control CPort. The AP also needs to support incoming requests on its
control port. Features not implemented yet are marked as TODO for now.
NOTE: This also fixes cport-bundle-id to 0 and cport-id to 2 for control
protocol.
Viresh Kumar [Mon, 22 Jun 2015 11:06:47 +0000 (16:36 +0530)]
greybus: manifest: Allow bundles/connections list to be non-empty at manifest parsing
A connection and a bundle will be created for interfaces at the very
beginning for control protocol's functioning. And so the list of bundles
and connections for a interface will be non-empty by the time manifest
is parsed.
Currently we are firing a WARN when these lists are found to be
non-empty. Lets fix that to contain single bundle and connection for
control protocol.
Rui Miguel Silva [Mon, 22 Jun 2015 13:03:52 +0000 (14:03 +0100)]
greybus: sdio: extend sdio implementation
Extend sdio implementation, as it for now it was basically stubs.
This implementation is compile tested only since there is no fw or
simulation support yet.
Next step is to add sdio support to gbsim and test it with success
using the mmc_test facility.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>