There are two cases where the AP may receive hotplug event for an
existing interface, without first getting a hot-unplug request for it.
- bootrom loading the firmware image and booting into that, which
only generates a hotplug event. i.e. no hot-unplug event, as the
module never went away.
- Or the firmware on the module crashed and sent hotplug request again
to the SVC, which got propagated to AP.
Handle such cases by first removing the interface, with a clear print
message shown to the user. And then following the normal hotplug
sequence to add the interface.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have a variable to access '&op->connection->dev' directly,
lets reuse it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Jean Pihet <jean.pihet@newoldbits.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The datapoint we are using to return metrics across modules and cports
shouldn't have a module identifier in it i.e.
/sys/kernel/debug/gb_loopback/raw_latency_endo0
not
/sys/kernel/debug/gb_loopback/raw_latency_endo0:X
This patch removes the module_id used up to this point. Including module_id
actually ends up making life harder in user-space so dropping it.
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>
dev_name() will return the endo0 component of the string on it's own,
there's no need to include it in the snprintf() when construting the
debugfs name. This fixes 'endo0' appearing more than once in the debugfs
name - shamefully slipped through testing cb570c93783f
('greybus/loopback: use dev_name to populate sysfsname').
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
If a thread is masked out it should not consume CPU cycles during a test.
We set an arbitrary 100 millisecond sleep time for each masked out thread.
Reasonably blunt instrument to ensure threads with nothing to do don't end
up thrashing the acquisition/release of mutexes.
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>
Currently we have sysfs entries that are created when the first incoming
connection is created as sub-nodes of the module associated with that
connection e.g. /sys/bus/grebus/devices/endo0:X where X is the module
identifier associated with the new connection. This is conceptually
incorrect since the sysfs entries we create actually aren't bound to a
module. Depending on the order connections are brought up we can also have
a situation where /sys/bus/greybus/devices/endo0:X has high-level control
sysfs data-points but /sys/bus/greybus/devices/endo0:Y does not. Rather
than needlessly replicate data-points across each endo0:X, endo0:Y, endo0:Z
sysfs directories it is more sensible to locate the entries in
/sys/bus/greybus/devices/endo0.
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>
This patch hooks tracepoints for the handoff point to/from hardware. With
these tracepoints in place we can view the time between gb_message_send and
usb_submit_urb and similarly we can view the time between cport_in_callback
and gb_message_recv_response/gb_message_recv_request
- trace_gb_host_device_send
- trace_gb_host_device_recv
It provides standard tracepoints at
/sys/kernel/debug/tracing/events/greybus/gb_host_device_send
/sys/kernel/debug/tracing/events/greybus/gb_host_device_recv
Giving outputs like
gb_host_device_recv: greybus:2-1 if_id=0000 l=10
gb_host_device_send: greybus:2-1 if_id=0000 l=10
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch adds new tracepoint declarations to greybus_trace.h to allow for
capture of greybus host device tx and rx events. These two tracepoints
allow an observer to see the point where the hardware interface driver
performs the relevant read or write to receive or write the data it's been
given from the higher layer greybus driver.
The following two new tracepoints are declared:
- trace_gb_host_device_send
- trace_gb_host_device_recv
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch drops tracking of internal latencies, it's possible to derive
these times via kernel tracepoints and some user-space scripting. Since
there's no other use of the internal timestamp than the loopback driver we
remove the connection.c, connection.h, es1.c, es2.c and loopback.c
inter-dependency in one go.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The Qualcomm kernel builds with -Werror so the existing es2.c driver
breaks the build due to unused static functions. As we are still
hashing out exactly how to implement this logic at the moment, just
comment out the functions to make the build be clean, no logic changes
happen here at all.
Reported-by: Michael Scott <michael.scott@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The file names here weren't in sync with what we have today and the
updates give a better picture of the same.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These host-driver callbacks were intended to allow host drivers to
prepare a cport, something which can now be handled by the cport
enable/disable callbacks instead.
The current create/destroy are somewhat confusingly named as they were
not supposed to create or destroy connections. They were however called
from the unrelated helper functions that do create and destroy SVC
connections.
Furthermore, no errors were returned should the create callback fail,
which should have caused the connection initialisation to fail.
Remove these unused callbacks for now, and let us use the cport
enable/disable callbacks that should be able handle all host cport
initialisation (possibly after also adding an interface to provide
information for endpoint-cport mapping).
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add optional cport enable and disable callbacks to the greybus host
drivers, that can be used to initialise and allocate/release resources
associated with a cport during connection setup/teardown (e.g. software
queues and hardware state).
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Rename helper to the more descriptive
gb_connection_control_disconnected().
Use u16 for cport number, remove redundant cport number from warning
message, and shorten a long line.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Clearly mark error-path labels as such and clean up control flow.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Move SVC-connection creation to its own helper.
Note that the connection_create host-driver callback is really
unrelated to the SVC connection, and will be removed by a later patch.
It is is included for now as the connection_destroy callback is
currently made from the SVC-connection-destroy helper.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The SVC Control request is obsolete and not used anymore. Remove the related
define.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Some fields in svc request were not being set with the correct
endianness, which will trigger the following sparse issues as example:
greybus/svc.c:116:22: warning: incorrect type in assignment (different base types)
greybus/svc.c:116:22: expected unsigned short [unsigned] [assigned] [usertype] attr
greybus/svc.c:116:22: got restricted __le16 [usertype] <noident>
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add the missing version-request definition that was falsely claimed to
be empty.
Update the generic version-request helper and SVC version-request
handler to use the request definition rather than rely on the response
happening to have the same layout, something which also improves
readability.
Remove a misplaced "Control Protocol" header while at it.
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>
The SVC-protocol driver currently accepts the version offered by the
SVC, but still responded with a hard-coded version.
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>
The device-id map was never deallocated on SVC-connection tear down.
Also make the map per-SVC-connection (there should still be only one)
rather than use a global pointer.
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Endpoints pair will only be managed by es2 driver.
map_cport_to_ep() and unmap_cport() should be static.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Some methods and variables name were a lot confusing.
Replace it or add ep_pair in methods or varaibles name
to make sources less confusing.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
A single global work-queue pointer was used for the per-connection
workqueue, something which would lead to memory leaks and all sorts of
bad things if there are ever more than one SDIO connection in a system.
Also add the missing error handling when allocating the queue.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The mmc-driver private data must not be accessed after mmc_free_host()
has released it.
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>
Do not release the minor number until after the device has been
deregistered.
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>
Module's Bootrom needs a way to know (currently), when to start sending
requests to the AP. The version request is sent before connection_init()
routine is called, and if the module sends the request right after
receiving version request, the connection->private field will be NULL.
Fix this TEMPORARILY by sending an AP_READY request.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
That's how the bootrom-tool names it, and that's how the kernel should
expect it.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
All the id-fields are 32 bit long instead of 16 bits and so we will need
8 characters per field instead of four. Also the stage field is only one
byte long and so needs just two characters to represent it.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
28 is the wrong value and should be 32 instead.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These are required to get/set DME attributes of the modules. This is
implemented based on the greybus specifications.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
dev_name() will give a nice string representing the end0:X:Y:Z:W name
mitigating the need to pick apart the various nested data structures and
print out their various identifiers.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Suggested-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Patrick Titiano <ptitiano@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In order to extract timestamps from gb_message instead of gb_connection we
will need access to the gb_operation structure. A first step to that is to
create our own gb_loopback_operation_sync which will call
gb_operation_request_send_sync() directly. Once loopback is using this
function internally it will be possible to convert to gb_message based
timestamps and drop gb_connection based timestamps in two seperate patches.
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 user-space we specify a list of connections as a bit-mask with the
assumption that a list such is indexed as indicated below.
end0:3:3:1:1 = 1
end0:3:3:2:3 = 2
end0:3:3:3:4 = 4
Current code assigns bitmask ids based on the order of discovery, however
user-space has no idea what the order of discovery is. This patch sorts the
linked list of connections associated with the loopback driver and assigns
a bit-id based on the sorted list - not the order of discovery. This
change therefore enforces the end-users idea that end0:3:3:1:1 above is
always denoted by bit 1 - even if from the AP's perspective it was the last
entry discovered.
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>
This patch adds a len field to the loopback protocol.
This field is validated in gb_loopback_transfer() and stuffed in
gb_loopback_request_recv().
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>
This patch fixes and invalid use of pr_info() in favour of dev_err();
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>
bdd4bba4 ('greybus/loopback: add module level sys/debug fs data points')
added a debugfs entry attached to gb_dev but omitted the cleanup on gb_init
error and gb_exit. This patchs fixes the missing debugfs_remove().
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
init doesn't have a lock for kzalloc so exit shouldn't have lock with the
corresponding kfree.
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>
This patch holds gb_dev.mutex for the duration of init and exit to reduce
complexity while ensuring that init and exit run atomically with respect
to slave threads @ gb_loopback_fn().
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>
This patches fixes a case where gb_dev.count is decremented too late in the
exit() routine.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Its a very useful piece of information, i.e. the cport id of the AP to
which the cport of the module is connected, and is required lots of
times. It isn't known in advance as it is allocated at runtime.
This patch creates another file 'ap_cport_id', only for the connection
directories, which will give the cport id of the AP.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We created two-way routes between the AP and module's interface on
hotplug, and forgot to remove them on hot-unplug. The same is also
required while handling errors in hotplug case.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The route-create request creates bi-directional routes and there is no
need to make separate calls for setting up routes on both the
directions.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This helps in removing special per-protocol code, with the help of
generic flags passed by protocol drivers.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
connection_create() is called right after svc is requested to create the
connection and so connection_destroy() must be called just before we
request the SVC to destroy the connection.
Over that, this fixes the inconsistency where connection_create() is
called for all connections except SVC connection, but
connection_destroy() is called always.
Acked-by: Alexandre Bailon <abailon@baylibre.com>
Fixes: 5313ca607afb ("Greybus driver: add a new callbacks to driver")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
While initializing a connection, the AP requests the SVC to create a
connection between a cport on AP and a cport on the Module.
The opposite of that is missing, when connection is destroyed or if
errors occur after creating the connection. Fix it.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There are two operations which very much work together:
- AP asks the SVC to create a connection between a cport of AP and a
cport of module.
- AP tells the module that the connection is created.
Its better (logically) to do these two operations together and so call
gb_svc_connection_create() from gb_connection_init() instead. Also check
its return value properly.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
If we fail to initialize a cport of a bundle, we abort the entire
bundle. But that leads to following (unnecessary) warnings as few of the
cport descriptors, belonging to the aborted bundle were never parsed:
"greybus: excess descriptors in interface manifest"
Fix that by releasing all cport descriptors for the aborted bundle.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
A 'bundle' represents a device in greybus. It may require multiple
cports for its functioning. If we fail to setup any cport of a bundle,
we better reject the complete bundle as the device may not be able to
function properly then.
But, failing to setup a cport of bundle X doesn't mean that the device
corresponding to bundle Y will not work properly. Bundles should be
treated as separate independent devices.
While parsing manifest for an interface, treat bundles as separate
entities and don't reject entire interface and its bundles on failing to
initialize a cport. But make sure the bundle which needs the cport, gets
destroyed properly.
We now release the bundle descriptor before parsing the cports, but that
shouldn't make any difference.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
gb_loopback_connection_exit does a kfree on a data structure associated
with a loopback connection but fails to do a corresponding list_del(). On
subsequent enumerations this can lead to a NULL pointer dereference. Each
list_add in gb_loopback_connection_init() must have a corresponding
list_del in gb_loopback_connection_exit(), this patch adds the relevant
list_del() and ensures that an appropriate mutex protecting gb_dev.list is
held while doing so.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The endpoint set 0 is currently considered as invalid.
But 0 mean muxed cports on ep1 and ep2,
then it must not return -EINVAL.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add connection_create and connection_destroy callbacks.
ES2 can map a cport to a pair of endpoints.
Because ES2 have only a few pair of endpoints, ES2 need to have
access to some high level connection information such as protocol id
to effectively map the cports.
These callback will provide these information and help ES2 to map cports.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
gb_connection_exit() is getting called from gb_connection_destroy() now,
which will get called from failure path of gb_connection_create_range()
(in a later commit). And at that point connection->protocol will be
NULL.
Don't print an error message if this happens in gb_connection_exit().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
gb_connection_init() can fail and will return proper error code in that
case, but the caller is ignoring it currently.
Fix that by properly handling errors returned from gb_connection_init()
and propagating them to callers of gb_connection_bind_protocol().
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Both the routines are always called together and in the same sequence.
Rather than duplicating this at different places, make
gb_connection_destroy() call gb_connection_exit().
This also makes it more sensible, as gb_connection_init() is never
called directly by the users and so its its counterpart shouldn't be
called directly as well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
We just got an error, propagate the exact return value instead of 0.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
connection->protocol will always be valid in gb_connection_init() as it
is called only from a single routine, after initializing the 'protocol'
field.
No need to check it again.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Failures from control-connected operations are fatal errors and
must be reported with dev_err() instead of dev_warn().
Fix it.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
[ johan: do not promote disconnected warnings, update summary ]
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Its not used by external users, mark it static. This required some
shuffling of the code.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Move the function to an earlier place, to kill the unnecessary forward
declaration.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
There are no external users of these, and probably would never be. Make
them static.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Not sure why they were created, but there is no need for them. Kill
them.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
These routines are responsible to destroy a connection that is going
away, the return value is of no use. At best, print an error message to
show that we got an error.
Make their return type void.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
These structures are exchanged between the AP and the module and must be
packed to avoid any unwanted holes.
Its all working currently because compiler doesn't add any pad bytes for
these structures, as their elements are already aligned to their size.
But these structures can change in future and we better mark them
packed.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
These structures are expected to be packed by the module firmware code,
but the kernel wasn't following it until now.
Its all working currently because compiler doesn't add any pad bytes for
these structures, as their elements are already aligned to their size.
But these structures can change in future and we better mark them
packed.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
All request/responses either have a structure representing them or a
comment saying the request/response payload doesn't exist.
The comment was missing for route create response message, add it.
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
The request sequence for SVC protocol is fixed at least upto SVC_HELLO
request. The first request has to be Protocol Version, followed by
SVC_HELLO. Any other request can follow them, but these two.
Add another field in 'struct gb_svc' that keeps track of current state
of the protocol driver. It tracks only upto SVC_HELLO, as we don't need
to track later ones.
Also add a comment, about the order in which the requests are allowed
and why a race can't happen while accessing 'state'.
This removes the WARN_ON() in gb_svc_hello() as we track state
transition with 'state' field.
This also fixes a crash, when the hotplug request is received before
fully initializing the svc connection. The crash mostly happens while
accessing svc->connection->bundle, which is NULL, but can happen at
other places too, as svc connection isn't fully initialized.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
[johan: add 0x-prefix to warning message ]
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
System headers should get included before greybus.h. Its followed
everywhere except svc.c. Fix it.
Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
For sdk related targets, the greybus build will error out due to missing
kernel dependency. Let's avoid those targets by checking TARGET_NO_KERNEL.
Signed-off-by: Vishal Bhoj <vishal.bhoj@linaro.org>
Signed-off-by: Michael Scott <michael.scott@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
The CPORT_ID_MAX define has been used by host drivers as a device limit,
but also for sanity checks when parsing manifests.
Now that it's only used for sanity checks we can increase it to the
specification maximum (4095) and get rid of the config-option that could
be used to override the previous limit (128).
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
The CPort count of es1 is now defined by CPORT_COUNT.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Make sure to return an errno when a host-device buffer-size check fails.
Fixes: 1f92f6404614 ("core: return error code when creating host device")
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
There is no need to store the endpoint number of the control requests since
the default control endpoint is used and the USB standard defines for it a fixed
endpoint number of 0.
Remove every instance of the field control_endpoint and replace it with a
hardcoded 0 value.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Use the control request REQUEST_CPORT_COUNT in order to get the number of
CPorts supported by the UniPro IP.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
In order to be able to dynamically determine the number of CPorts supported
by the UniPro IP instead of hardcoding the value we need to dynamically
allocate the array that is doing the cport-ep mapping.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
This commit is doing the preparation work in order to get the number of cports
supported from the UniPro IP instead of using a constant defined in a Kconfig
file.
Greybus host device is now holding the cport count, and all the code will now
use this value instead of the constant CPORT_ID_MAX when referring to an AP's
CPort ID.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
[johan: es1 supports 256 cports, minor style changes ]
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Rename the misnamed macro CPORT_MAX into CPORT_COUNT. CPORT_MAX could let
people think that the macro is holding the value of the last CPort ID
usable.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
There is no need to perform connection->bundle->intf->hd as the same can
be done with connection->hd.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Replace pr_err with the more descriptive dev_err. Also include the error
code on failure to register the PWM chip.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Use scnprintf in the generic attribute helper, which does not currently
check for buffer overflow.
The attribute helper is used to print generic strings, which could
potentially overflow the buffer. Note that the only strings currently
exported are taken from greybus string descriptors and should therefore
be limited to 255 chars.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Use GFP_KERNEL for hot-plug state allocation in
gb_svc_intf_hotplug_recv, which is called from a request handler (i.e.
a work queue).
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Use GFP_KERNEL for device-id allocation in svc_process_hotplug, which is
called from a work queue.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Use GFP_KERNEL for endo ida allocation in gb_endo_register, which is not
called from atomic context.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Broadcast command with response and without response where swapped
related to what is defined in greybus specification.
Make it coherent with the document.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Use snprintf when generating the firmware name to avoid stack corruption
if the fixed-size buffer overflows.
Note that the current buffer size appears to expect 16-bit ids while
the they are actually 32-bit, something which could trigger the
corruption.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Greybus messages with a multiple size of 512B generate timeouts
(any other message size doesn't).
512B is exactly the packet size of a bulk out endpoint.
Hence USB device is expecting a short (< 512B)
or zero-length packet to finish the transfer,
which is never generated and causes the timeout.
Set the transfer flag to send a zero-length packet in this situation.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Reviewed-by: Patrick Titiano <ptitiano@baylibre.com>
Reviewed-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Alex previously post a patch to fix this typo. Somehow it fell through the
cracks in the meantime. Do it again 'stastic' is a word 'statistic' is not.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Code this far has used the first connection's sysfs entry to present
variables intended to control the entire test - across multiple
connections. This patch changes that so that the module level variables
only appear at the end0:x level in sysfs.
Example:
Total counts for errors over the entire set of connections will be here
/sys/bus/greybus/devices/endo0:x/error_dev
In contrast an error for each connection will be presented like this
/sys/bus/greybus/devices/endo0❌y:z:w/error_con
x = <module-id>
y = <interface-id>
z = <bundle-id>
w = <cport-id>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
checkpatch.pl is choking on a later change to dev_stats_attrs, where
checkpatch expects to see the values encapsulated in curly brackets.
Encapsulating in curly brackets will cause a compiler error. To resolve the
dichotomy this patch drops the macros and adds the arrary declarations
directly.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Feature add which enables the ability to select a bit-mask of connections
to run when executing a loopback test set. This is a feature add to
facilitate testing on the firmware side minus the necessity to recompile
firmware to support unicast (v) multicast (v) bitmask.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch adds the ability to time the delta between all threads like this
t1 = timestmap();
thread1:gb_operation_sync();
thread2:gb_operation_sync();
t2 = timestamp();
In order to enable that behaviour without forcing an undesirable
checkpointing scheme this patch introduces a kfifo for each thread to store
the raw timestamps and calculate a time difference.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The __gb_loopback_calc_latency will be useful in later patches. Provide it
here and use as intended. Later on we just want to use the timestamp
rollover detection, so split it out now.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
A helper function to convert from a nanosecond value to a latency value
expressed in mircoseconds. This will be used again in subsequent patches.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The loopback code as currently implemented allows free running threads to
blast data to cports in isolation, however no overall stastics are gathered
for the aggregate throughput to a given module.
This patch moves derivation of stastics for all cports to one structure
so that 1 to n cports will report their overall stastics in one place.
Later patches update the sysfs/debugfs accessor functions to provide the
per-module and per-connection data separately.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Greg previously suggested switching over to debugfs instead of a char
interface to report raw samples to user-space. At the time we agreed not
to go for that change. However later patches in this series are made
simpler if debugfs is used instead of /dev, so it makes sense to make the
conversion now for that reason. This patch removes the char interface and
replaces it with a debugfs interface. Raw samples will be acquired from
/sys/kernel/debug/gb_loopback/raw_latency_endo0❌y:z:w where
x = <module-id>
y = <interface-id>
z = <bundle-id>
w = <cport-number>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch adds the ability to graph the latency of the overhead introduced
by the greybus stack in the roundtrip time AP->Module->AP.
Since this is a small number it is reported in nanoseconds, not
mircoseconds. This data can also be derived with tracepoints but it's being
provided in this format to export to CSV file more easily than with
tracepoints.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
'user-replicable' means something that a user can replicate.
'user-replaceable' means something that a user can replace. We defintely
mean to say replaceable not replicable here.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Greybus core now handle the _INVALID and PROTOCOL_VERSION types, so no
need to have specific protocol macros for this. Just drop them.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The order of arguments is wrong and that shows up as a warning only on
64 bit machines.
Fixes: cb0ef0c019ab ("operation: print message type on errors")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
[[, == and echo -e are bash/zsh-ism and not POSIX, so when using a POSIX
shell the kernel_cmp can issue some warnings and not work properly.
Use only POSIX operators for kernel version compare.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Tested-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch adds lights implementation for Greybus Lights class, it
allows multiplexing of lights devices using the same connection. Also
adds two sysfs entries to led class (color, fade) which are commonly
used in several existing LED devices.
It support 2 major class of devices (normal LED and flash type), for
the first it registers to led_classdev, for the latest it registers in
the led_classdev_flash and v4l2_flash, depending on the support of the
kernel version.
Each Module can have N light devices attach and each light can have
multiple channel associated:
glights
|->light0
| |->channel0
| |->channel1
| | ....
| |->channeln
|->...
|->lightn
|->channel0
|->channel1
| ....
|->channeln
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add a function to check kernel versions and append the necessary
options to support LEDS_CLASS, LEDS_CLASS_FLASH and V4L2_FLASH_LED_CLASS
depending of the kernel version.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This can be very useful debug information, print it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We are already doing put_device() here and so don't need to free
resources directly, except ida.
Fixes: afde17fe0b61 ("greybus/connection: fix jump label on device_add failure")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Need to destroy the wq created earlier, do it.
Fixes: d0f1778a6b67 ("greybus/connection: add a timestamp kfifo to track connection handoff")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These must be exposed to external modules, like gbsim. Move them to
greybus_protocols.h file.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These must be exposed to external modules, like gbsim. Move them to
greybus_protocols.h file.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These must be exposed to external modules, like gbsim. Move them to
greybus_protocols.h file.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The greybus specifications clearly say (for all protocols) that the
sender is responsible for sending the highest version of protocol it
supports, while it requests the same from the receiver.
But the greybus code never followed that.
Fix, this by always sending AP's version of the protocol, while
requesting the same from svc/module.
This also renames 'response' to 'version' in gb_protocol_get_version()
as it is used for both request/response.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
As per greybus specs, we need to send the protocol version for firmware
protocol and so this special case Hack.
Probably we should always send the protocol version AP supports and kill
this hack completely. But then it requires updates to specs as well, and
that should be done after some discussion.
For now, add a FIXME for that and a special case for firmware protocol.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This adds firmware protocol driver based on the latest specs available
on mailing lists. This uses the firmware framework present in kernel.
Refer Documentation/firmware_class/README on how it works.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The version request can only send the version of protocol for which it
is initiated and gb_protocol_get_version() has all the information to
create the request structure.
Replace the 'request' and 'request_size' arguments to
gb_protocol_get_version() with a bool to know if the version information
of the protocol should be sent or not.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This shall be used later to find a firmware blob for the interface, lets
save it in the interface structure.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This patch ensures we account for roll-over in the loopback driver.
do_gettimeofday() is used to grab a timestamp. Two timestamps are derived
one before and one after a gb_operation_sync(), however since
do_gettimeofday() returns the number of seconds and mircoseconds that have
elapsed today - we need to account for a situation where the timestamp
starts at say 23:59:999us rolls over and the end time is now earlier than
the start time.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We have a pattern similar to this over and over again gb->elapsed_nsecs =
timeval_to_ns(&te) - timeval_to_ns(&ts); good software practice dictates we
functionally decompose this. This patch decomposes into
gb_loopback_calc_latency().
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
In order to facilitate grabbing a timestamp that doesn't include greybus
overhead, this patch adds a timestamp right before usb_submit_urb() for
both es1.c and es2.c. Long term the timestmaping of messages like this
probably wants to go away but, for the moment it may have some use to the
firmware people instrumenting the performance of the system.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
For the ES2 test activity it may be beneficial to have a performance
metric that doesn't include any of the greybus stack malloc, workqueues
etc. In order to faciltate, this patch adds a simple kfifo structure to
hold two timestamp values. One timestamp will represent the last reasonable
point a greybus outbound timestamp can be taken, the other timestamp will
represent the first reasonable point an inbound timestamp can be taken. In
order to facilitate this model, tracking the timestamps in the connection
structure appears to be the best place to keep store of this data.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
On device_add() failure in gb_connection_create_range() we jump to
err_remove_ida. Instead we should be jumping to err_free_connection, so
change the flow to accomodate.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Some of the parameters are not really required, drop them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These weren't preserved earlier, save them in the connection structure
instead of creating its own fields..
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This is done from a common place now, no need to replicate it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This can (should) be done from a common place as its required for most
of the protocols. Do it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Save major/minor number supported by the module inside connection
structure, as this can be used later.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Some request numbers (like invalid and get_version) are same across all
protocols. Create common macros for them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Its possible for connection_init() to fail, in such cases the
disconnected event must be sent to the module.
It is missing currently, fix it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
!= was used in place of <, while comparing expected and actual payload
size. The module may be running a higher version of the protocol and
might have some extra fields (towards the end) in the structure, and the
AP needs to ignore them.
This also updates the print (expected-payload-size <
actual-payload-size), when the size doesn't match for requests received
by the module. This gives more details required for debugging.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This increases readability a bit more, as it tells which value is actual
size and which one is expected size.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Its handled by the firmware driver now, drop the FIXME comment from svc
code.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
We already have macros for these, use them instead of writing fixed
values.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
!= was used in place of <, while comparing expected and actual payload
size. The module may be running a higher version of the protocol and
might have some extra fields (towards the end) in the structure, and the
AP needs to ignore them.
This also updates the print (expected-payload-size <
actual-payload-size), when the size doesn't match for requests received
by the module. This gives more details required for debugging.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Bringing up a module can be time consuming, as that may require lots of
initialization on the module side. Over that, we may also need to
download the firmware first and flash that on the module.
In order to make other hotplug events to not wait for all this to
finish, handle most of module hotplug stuff outside of the hotplug
callback, with help of a workqueue.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Print (expected-payload-size actual-payload-size), when the size doesn't
match for requests received by the module. This gives more details
required for debugging the issue.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Print (expected-payload-size actual-payload-size), when the size doesn't
match for requests received by the module. This gives more details
required for debugging the issue.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Print (expected-payload-size actual-payload-size), when the size doesn't
match for requests received by the module. This gives more details
required for debugging the issue.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
These were not defined, and I just posted patches that use them.
Signed-off-by: Alex Elder <elder@linaro.org>
Tested-by: Mark Greer <mgreer@animalcreek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The error paths in gb_loopback_connection_init() are kind of screwed
up--not in proper order, and the label naming convention seems a
little inconsistent.
Fix this, ensuring each error cleans up the setup that's been done
up to that point. Use the convention that the label indicates the
first thing that needs to be cleaned up.
Reorder the statements in gb_loopback_connection_exit() to match
the order of cleanup in gb_loopback_connection_init().
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Stop recording and updating the average every time a sample
is recorded. Instead, compute it from the sum and count only
when it's required.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Define a separate macro for displaying the average of the samples
collected. This will be used so we can calculate the average only
when requested, rather than every time a new value gets recorded.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The only values passed to the gb_loopback_ro_attr() macro are
unsigned 32-bit values. So there's no need to pass a "type"
format specifier.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The error count is unsigned, so fix the format specifier used in its
attribute definition.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The count of statistical samples recorded is currently a 64-bit
value. 32 bits is sufficient, and in fact anything more than
that won't work for the do_div() call it's pass to anyway. So make
the count field be 32 bits.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The minimum and maximum values for stats values are always 32 bits.
Change the type for these fields to reflect this.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The only values supplied to gb_loopback_update_stats() are 32-bits,
so change the type of the second argument to reflect that.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Use the largest representable value when initializing the "min"
field when resetting loopback statistics.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
There is no need to cast a void pointer to a particular type.
Drop the casts used in this way, mainly in the attribute definition
macros.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fix two misspellings. And add spaces around a '%' operator.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Add a public declaration for gb_interface_destroy(), matching
gb_interface_create().
It's not yet used outside "interface.c" but I suppose it
could be, and its scope is currently public.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
That's the style we follow for kernel code.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Handle some merge conflicts in greybus_protocols.h due to changes on the
same structure in both branches.
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
For consistency reasons, use only type attributes for message packing.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fix misspelled variable name in comment.
Reviewed-by: Alex Elder <elder@linaro.org>
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>
Get the USB driver fixes in this branch to prevent oopses from happening
when the USB host driver protocol is found in a manifest.
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
The USB bridged-PHY protocol driver currently depends on changes to USB
core that are not yet upstream.
Disable for now.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fix allocation, deregistration and deallocation of USB HCD, and update
the hcd_priv helper functions.
The HCD private data was not allocated correctly, something which would
lead to a crash when accessed in hcd_start. The HCD was neither
reregistered or deallocated on connection tear down.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Viresh Kumar