If an operation times out or otherwise returns an error, val->intval
should not be set and an error-code should be returned.

Fixes a panic on unload while receiving -ENOTCONN.
Signed-off-by: Perry Hung <perry@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Its not referenced by svc or any other code anymore, lets stop exposing
it to rest of the files.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Perry Hung <perry@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Its not updated/used anymore, remove it. Also move back the struct
gb_svc to svc.c as its not referenced by external users anymore.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Perry Hung <perry@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

We no longer create a fresh connection on receiving svc-hello message,
but rather update the initial one.

Update 'initial_svc_connection' after the connection is fully
initialized. Look for the partially initialized connection while
removing hd, as hd might be removed before getting svc-hello requests
from svc.

Also update gb_svc_connection_init() to initialize id_map on the first
(and the only) call to connection-init function.

We also can't update connection->bundle->intf->svc, as its a bundle-less
connection. Lets stop updating intf->svc as its not really used.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Perry Hung <perry@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

SVC hello message is received now and we should fully initialize the
partially initialized connection. This can be done by removing and
re-adding the device corresponding to the connection.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Perry Hung <perry@leaflabs.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Current code will incrementally poll for gb->type == 0 and sleep.
This type of polling strategy wastes cycles.

This patch changes the sleep strategy by introducing a wait-queue which
waits for gb->type != 0 or kthread_should_stop() to wake-up and work or
to wake-up and terminate.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

This adds the ability to poll on "iteration_count" in sysfs and be woken
up when it changes, saving some cycles constantly hammering on the file
waiting for it to change.
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Add dedicated bound work queue for operation completions and use the
connection work queues for incoming requests only.

There is no need to keep responses ordered internally or with respect to
requests. Instead allow operations to complete as soon as possible when
a response arrives (or the operation is cancelled).

Note that this also allows synchronous requests to be submitted from
request handlers as responses will no longer be blocked on the same
single-threaded work queue. Similarly, operations can now also be
cancelled from a request handler.
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
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>

Replace the global operation work queue with per-connection work queues.

There is no need to keep operations strictly ordered across connections,
something which only adds unnecessary latency.
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
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>

Clean up connection-creation error paths.
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
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>

Add variable for the host cport id rather than overload retval.
Remove redundant automatic variable for the id map.
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
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>

If we don't have a protocol assigned to a connection, don't oops when
trying to read the "protocol_id" sysfs file.

Fixes Jira SW-968.
Reported-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Fix below warnings that only generate for a 64 bit system:

greybus/svc.c:202:16: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
   dev_err(dev, "%s: Illegal size of hello request (%d %d)\n",
                ^
greybus/svc.c:202:16: warning: format ‘%d’ expects argument of type ‘int’, but argument 5 has type ‘long unsigned int’ [-Wformat=]

Fixes: 9cbe73e0cf45 ("svc: Implement SVC requests")
Reported-by: Greg KH <gregkh@google.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Make sure connection is fully initialised before registering the
connection device and adding it to the host-device and bundle lists.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Add connection variable to greybus_message_sent.

This will be put to more use by a follow-up up patch.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Make the operation work queue single threaded.

The operation work queue was meant to be single threaded, but due to a
missing flag instead allowed one active task per CPU, something which
could lead to requests being processed out of order on SMP systems.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Tested-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

The name frequency does not adequately describe the data-point tracking the
number of greybus operations performed in a second by the loopback code.
Firmware team is moving towards calling this variable requests-per-second
or similar. This patch renames to keep the namespace consistent.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Patrick Titiano <ptitiano@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Add some missing comments.
Add a TODO to look at doing iteration_count with KOBJ_CHANGE event instead
of having user-space poll the value reported in iteration_count to
determine when a test set is complete.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Patrick Titiano <ptitiano@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

In order to extract a meaningful set of data out of loopback metrics it
makes sense to have the ability to specify how many times a loopback
operation should run and then to stop when the threshold is hit.

This will allow exactly the same loopback data pattern to be run again and
again, which is a fundamental prerequisite to instrumenting and
characterizing the loopback interface over time.

This patch introduces a simple sysfs controlled variable iteration_max.
iteration_max is the maximum number of iterations to run for a given set.
iteration_count is used internally to count from zero to iteration_max
if-and-only-if iteration_max is non zero. If iteration_max is zero then the
original behaviour of running the test command ad infinitum is maintained.
User-space should incrementally poll the iteration_count to determine when
the sequence is finished.

To accomplish this we move away from running as many commands as possible
in a one second window and instead run a fixed number of commands and log
the time it takes for those commands to complete in aggregate. Since we are
no longer resetting counters every one second, the tracker variables have
been moved from u32 to u64 to allow for reasonably long tests to gather
reasonably large amounts of data, without fear of over-flowing the
data-points.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Old comment needs updating to match 8a282c411ba0 ('greybus/loopback: make
loopback type input equivalent to protocol type')
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Patrick Titiano <ptitiano@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

In the current code if the ts variable is not initialized then any data
already gathered in a previous loopback command is dropped instead of
logged. Also the timestamping of ts is done after the greybus operation.

This delayed time-stamping means that the delta between the before and
after timestamps is incorrect and if a delay in-between loopback operations
has been specified by the user, the ts timestamp will be very skewed
indeed.

- Move the ts initialization directly before the greybus operation.
- Remove the continue statement on first initialization of the ts variable.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

This patch converts a dangling pr_err in the manifest parsing error path to
a dev_err in order to remain consistent with similar error messages
elsewhere.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

5ae6906e ('interface: Get manifest using Control protocol') in
gb_create_control_connection introduces the concept that the Control
Protocol is at cport_id 2 and bundle_id 0. Currently the manifest parsing
code does not enforce that concept and as a result it is possible for a
manifest to declare the Control Protocol at a different address.

Based on that change 6a6945c9684e ('greybus-spec/control: Formally define
Control Protocol reserved addresses') makes the above coding convention a
formal requirement of the greybus specification. This patch implements the
change introduced in the specification @ 6a6945c9684e.

This patch will reject a manifest if it doesn't match the critiera laid
down in the spec.

This patch makes three changes:
- Changes gb_manifest_parse_cports so that only GB_CONTROL_CPORT_ID may
  have a protocol_id of GREYBUS_PROTOCOL_CONTROL, otherwise the manifest
  will be rejected.
- Changes gb_manifest_parse_bundles so that only GB_CONTROL_BUNDLE_ID may
  have a class of GREYBUS_CLASS_CONTROL, otherwise the manifest will be
  rejected.
- gb_connection_exit() and gb_connection_destroy() are removed from
  gb_manifest_parse_cports on error - since gb_manifest_parse_bundles()
  already has a call into gb_bundle_destroy() which will again call
  gb_connection_exit() and gb_connection_destroy() leading to an oops.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

SVC already provided version to the AP on version-request, no need to
ask for that again. Drop it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Some of the SVC request are already partially implement, whereas few
others are not. This patch implements and updates these requests.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

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.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

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.

Lets name it gb_create_bundle_connection().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

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.
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

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.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

SVC is also a special protocol (like control) and AP doesn't need to
send (dis)connected events for its cport.

Lets skip them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

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.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Define device-ids for AP, SVC and modules (starting id only).
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Version response structure is also required by external entities like
gbsim and so its structure should be moved to greybus_protocols.h.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Class types aren't in sync with protocol types, lets keep them in sync
for now.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

We don't do what the comment says, drop it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Replace '* *' with '*'.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

The recently added GFP-flags argument to gb_message_send was never used.

Fixes: 9218fac2a24d ("operation: allow atomic request submissions")
Signed-off-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 list_last_entry macro for kernels older than 3.13.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
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>