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>
