Johan Hovold [Tue, 13 Oct 2015 17:10:24 +0000 (19:10 +0200)]
greybus: protocol: warn on bad deregistration
A protocol should be deregistered exactly once when the protocol module
is being unloaded. This means that protocol deregister will never be
called with active users as we take a module reference when looking up a
protocol.
Remove comment suggesting that we could one day forcefully stop a user
of a protocol, and issue a big warning if a protocol is deregistered
more than once or at some other time than during module unload (e.g.
with active users).
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Fabien Parent [Tue, 13 Oct 2015 15:34:50 +0000 (17:34 +0200)]
greybus: es2: implement cport reset control request
Toshiba UniPro IP requires to reset the CPort that has been used in a previous
connection. This commit implement a new control request in order to
reset CPorts on an APBridgeA.
In kernel versions bellow 3.15, the mmc_card_is_removable helper
function has an extra check used for a suspend/resume hack. This made
the gd_sdio_process_event to behave badly handling the module card
insert event in that versions.
So, just test bit the flag that we need, instead of using the helper
function. This way will work in all kernel versions.
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Viresh Kumar [Wed, 7 Oct 2015 19:40:24 +0000 (15:40 -0400)]
greybus: svc: skip setting flags for boot over unipro
We need to skip setting E2EFC and other flags to the SVC connection
create request, for all cports, on an interface that need to boot over
unipro, i.e. interfaces required to download firmware.
This also adds a FIXME as we need to do it differently for ES3.
Tested-by: Eli Sennesh <esennesh@leaflabs.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off by: Eli Sennesh <esennesh@leaflabs.com> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Currently, the greybus module .ko files are quite large and the
following error was observed during Android build for each greybus module:
strip: Unable to recognise the format of the input file `<module>`
Fix the strip build step by replacing the undefined KERNEL_TOOLCHAIN_PATH
variable with the GREYBUS_CC_PREFIX variable. Also used as the
CROSS_COMPILER value for the module make.
Signed-off-by: Michael Scott <michael.scott@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Michael Scott [Fri, 2 Oct 2015 19:05:33 +0000 (12:05 -0700)]
greybus: build: android: replace Linaro build specific logic with AOSP equivalents
When using greybus build in a different Android setup, it was noted that
several portions of this makefile rely on Linaro specific build items.
Replace these with more generic build steps.
- ANDROID_64 is only defined by Linaro build tasks, use TARGET_ARCH to
establish ARCH= parameter for greybus build
- KERNEL_TOOLS_PREFIX is only defined by Linaro build tasks. AOSP has
a near equivalent variable: TARGET_TOOLS_PREFIX
- build-greybus was dependant on subtask: android_kernel a task defined
only by Linaro build tasks. Replace with a generic dependancy to
the kernel binary located in $OUT (INSTALLED_KERNEL_TARGET).
End result is the same: kernel must be built before greybus modules
Signed-off-by: Michael Scott <michael.scott@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Johan Hovold [Sat, 26 Sep 2015 21:37:59 +0000 (14:37 -0700)]
greybus: es1/2: fix use-after-free in completion callback
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>
greybus: firmware: Don't send control-disconnected event for firmware protocol
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.
greybus: interface: gb_interface_create() isn't called for existing interfaces
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.
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>
greybus: svc: Allow consecutive hotplug events for the same module
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.
greybus: loopback: drop redundant endo0 string from debugfs entry name
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').
greybus: loopback: masked out threads should sleep
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.
greybus: loopback: move sysfs entries to /sys/bus/greybus/devices/endo0
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.
greybus: es1, es2: hook tracepoints to hardware send/recv operations
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
greybus: tracepoints: add tracepoints for host_device tx/rx
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
greybus: loopback: drop dependency on internal timestamps
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.
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>
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>
Johan Hovold [Thu, 17 Sep 2015 11:17:26 +0000 (13:17 +0200)]
greybus: hd: add optional cport enable and disable callbacks
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>
Johan Hovold [Thu, 17 Sep 2015 11:17:21 +0000 (13:17 +0200)]
greybus: connection: clean up svc-connection creation
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>
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.
Johan Hovold [Mon, 14 Sep 2015 18:19:04 +0000 (20:19 +0200)]
greybus: sdio: fix work-queue leak and use-after-free
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>
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.
greybus: firmware: Fix incorrect firmware file's name
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.
greybus: loopback: use dev_name to populate sysfsname
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.
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.
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.
greybus: loopback: ensure sysfs entries are cleaned up on exit
bdd4bba4 ('greybus/loopback: add module level sys/debug fs data points')
added a sysfs entry attached to gb_dev but missed the jump to out_sysfs_dev
This patchs fixes the missing jump to out_sysfs_dev.
greybus: loopback: ensure debugfs entires are cleaned up on exit
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().
greybus: loopback: hold a coarse lock while init/exit run
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().
greybus: connection: Add sysfs 'ap_cport_id' file for connections
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.
greybus: svc: destroy the route on module hot-unplug
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.
greybus: connection: Call connection_destroy() from gb_connection_svc_connection_destroy()
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>
greybus: connection: call gb_svc_connection_create() from gb_connection_init()
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.
greybus: manifest: release cport descriptors to avoid 'excess descriptors' warning
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.
greybus: manifest: don't reject the interface on failing to initialize a cport
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.
greybus: loopback: make sure to list_del on connection_exit
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.
greybus: Greybus driver: add a new callbacks to driver
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.
Viresh Kumar [Mon, 31 Aug 2015 11:51:15 +0000 (17:21 +0530)]
greybus: connection: protocol can be NULL in gb_connection_exit()
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>
Viresh Kumar [Mon, 31 Aug 2015 11:51:13 +0000 (17:21 +0530)]
greybus: connection: call gb_connection_exit() from gb_connection_destroy()
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>
Viresh Kumar [Mon, 31 Aug 2015 11:51:05 +0000 (17:21 +0530)]
greybus: svc: No need to return errors from [gb_]svc_connection_destroy()
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>
projects / mirror_ubuntu-zesty-kernel.git / log