Remove legacy interface to "destroy" operations, which is now just a
wrapper for gb_operation_put.
The old interface name hides the fact that all operations are refcounted
and may live on even after having "destroyed" them.
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Reset the hcpriv field before returning the message to greybus core in
the OUT-URB completion callback.
This fixes a use-after-free bug when sending responses to incoming
requests as the final reference is then dropped when the message is
returned.
Reported-by: Michael Scott <michael.scott@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
After downloading the firmware for the next boot stage, module's
firmware (for current boot stage) jumps into it and the new firmware and
sends hotplug request to SVC. On hotplug request from the SVC, the AP
first removes the existing interface.
At this time, there is no point sending disconnected event for the
firmware protocol, for the firmware used in previous stage, as the new
firmware wouldn't be aware about it.
Set flags for firmware protocol to skip control-disconnected operations.
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>
The spec says that it doesn't need it, so dropping it.
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>
The callers are ensuring that another interface doesn't exist with the
same interface id and so there is no need to check that from
gb_interface_create() anymore.
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>
The callers already have a valid interface pointer and there is no need
for gb_interface_remove() to find the interface again.
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>
As per the module's boot sequence diagram, the AP needs to read/clear
T_TstSrcIncrement attribute on hotplug (svc) events.
Implement that.
FIXME: This is module-hardware dependent and needs to be extended for
every type of module we want to support.
[ Based on work by Marti & Eli to clear the attribute with DME set]
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>
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>
Johan Hovold