Add sanity checks for cport_quiesce and cport_clear before invoking the
callbacks as these function pointers are not required during the host
device registration. This follows the logic implemented elsewhere for
various other function pointers.
Signed-off-by: Jason Hrycay <jhrycay(a)gmail.com>
---
drivers/staging/greybus/connection.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 833f83b..aa040b1 100644
--- a/drivers/staging/greybus/connection.c
+++ b/drivers/staging/greybus/connection.c
@@ -358,6 +358,9 @@ static int gb_connection_hd_cport_quiesce(struct gb_connection *connection)
size_t peer_space;
int ret;
+ if (!hd->driver->cport_quiesce)
+ return 0;
+
peer_space = sizeof(struct gb_operation_msg_hdr) +
sizeof(struct gb_cport_shutdown_request);
@@ -381,6 +384,9 @@ static int gb_connection_hd_cport_clear(struct gb_connection *connection)
struct gb_host_device *hd = connection->hd;
int ret;
+ if (!hd->driver->cport_clear)
+ return 0;
+
ret = hd->driver->cport_clear(hd, connection->hd_cport_id);
if (ret) {
dev_err(&hd->dev, "%s: failed to clear host cport: %d\n",
--
2.10.0
Two simple patches here resulting from using greybus on gbsim and
developing support for async. I found a bug in the user-space tool and
while doing that decided to update the kernel thread to be better behaved
when waiting for test completions.
V2:
Use available bundle pointer for dev_dbg printout - Johan
Bryan O'Donoghue (2):
staging: greybus: loopback: use gb_loopback_async_wait_all don't spin
staging: greybus: loopback_test: Fix race preventing test completion
drivers/staging/greybus/loopback.c | 11 +++++++++++
drivers/staging/greybus/tools/loopback_test.c | 10 +---------
2 files changed, 12 insertions(+), 9 deletions(-)
--
2.7.4
V2:
Scotty : "What are you standing around for - do you not know a jailbreak
when you see one" ?
Making sure to run format-patch against the right tree this time..
Updated meta-comments to highlight adding a generic async timeout - Johan
Captured the result code of schedule_work/queue_work - Bryan
Drop reference count on failure to submit schedule_work/queue_work - Bryan
Added timeout suffix back to new gb_operation_request_send - Johan/Greg
Updated Signed-off-by to my nexus-software.ie address - Bryan
Iterated requisite Star Trek Quote - Bryan/Johan
V4-private -> V1-public:
McCOY: You've got him, Jim! You've got him where you want him.
This patchset adds async operations to greybus-core. Rather than have
different drivers do variations on the same theme of launching and timing
out asynchronous operations, it makes sense instead to provide this
functionality via greybus-core.
This patchset makes it possible to launch asynchronous operations and have
the completion callback for an operation indicate -ETIMEDOUT for timeouts
without drivers having to manage that locally. This set doesn't convert the
existing synchronous operations away from
wait_for_completion_interruptible_timeout() since that would involve adding
an extra work-queue item to each synchronous call, with no added benefit
for synchronous operations. Synchronous operations can already detect
-ETIMEDOUT by blocking on the synchronous send operations, asynchronous
operations and the driver associated with those operations OTOH must
implement a completion handler. The main improvement this patchset makes is
to pass the -ETIMEDOUT completion status into that completion handler -
hiding the setup/timeout of operations away from a driver-level and into a
core-level set of logic.
Loopback as an example can have lots and lots of redundant code removed and
given we previously had some in-flight effort to add asynchronous
operations to SDIO it makes sense to move the generic types of things we
need to do on the asynchronous path into greybus-core so that SDIO and
other bundle drivers can reuse instead of reimplement asynchronous
operations.
Note: we had approximately three internal versions of this on Project-Ara.
Here's the log for completeness.
V3-private -> V4-private:
Fix bug in loopback conversion - Mitch Tasman
V2-private -> V3-private:
remotes/ara-main/sandbox/bodonoghue/ara-main-current+async_op3
Drop patch #6 converting sync operations to new t/o structure. Johan had a
concern about doing an in-depth analysis on that change and considering our
compressed timelines now, this analysis won't happen. OTOH the new async
API is the right thing for loopback and for potential later greybus
users/implementers to reuse.
Although it wasn't part of the motive for this change - I observe slightly
better performance than baseline with loopback tests with this change in
place - which shouldn't be surprising since we have less aggregate context
switching for operation timeouts.
V1-private -> V2-private:
Rename async_private => private - Greg
gb_operation_request_send_async_timeout ->
gb_operation_request_send_async() - Greg
Using existing gb_operation_completion_wq - Viresh
Split out gb_operation_cancel() - timeout need not wait synchronously on
cancellation queue - Bryan
Move timeout to work-queue - Greg/Viresh
cancel timeout workqueue @ operation completion point - Viresh/Bryan
Bryan O'Donoghue (5):
staging: greybus: operation: add asynchronous gb_operation_cancel
staging: greybus: operation: add generic asynchronous timeout
operation support
staging: greybus: operation: add private data with get/set accessors
staging: greybus: loopback: convert loopback to use generic async
operations
staging: greybus: loopback: convert to use msecs internally
drivers/staging/greybus/loopback.c | 181 ++++++++----------------------------
drivers/staging/greybus/operation.c | 85 ++++++++++++++++-
drivers/staging/greybus/operation.h | 19 ++++
3 files changed, 140 insertions(+), 145 deletions(-)
--
2.7.4
Two simple patches here resulting from using greybus on gbsim and
developing support for async. I found a bug in the user-space tool and
while doing that decided to update the kernel thread to be better behaved
when waiting for test completions.
Bryan O'Donoghue (2):
staging: greybus: loopback: use gb_loopback_async_wait_all don't spin
staging: greybus: loopback_test: Fix race preventing test completion
drivers/staging/greybus/loopback.c | 12 ++++++++++++
drivers/staging/greybus/tools/loopback_test.c | 10 +---------
2 files changed, 13 insertions(+), 9 deletions(-)
--
2.7.4
OK now that we have lists and IRC in place, it's time to start
using them. Bryan (at least) has posted some patches for review
but if Greybus is going to provide some greater long-term usefulness
it will need to be adapted to be more easily used in contexts outside
Project Ara.
There was some informal conversation about this at ELCE. I have a
few of my own thoughts but mainly I just want to get some discussion
started to see if we have some pieces we can agree on, and take things
from there. Here's a short summary of what's below:
- Retargeting Greybus for use in IoT seems reasonable
- We will need to adjust the abstract model currently implemented for
Greybus to match the new target
- Certain core functionality should be useful as-is; but some other
features will need to be re-thought and redefined.
- We will need to evolve the Greybus kernel code to define what is
truly "core" functionality, and what that core needs from its
environment.
- We will need to ensure the Greybus API is stable, and that our
versioning design is sound.
One thing that came up multiple times in October was the notion that
Greybus could serve as the basis for communication among entities in
an Internet of Things system. I agree with this, at least in part.
We have set up a system with some well-defined mechanisms for
encapsulating messages, along with a model of remote procedure calls
that provide a pretty robust way of getting things done across a
network. The core code implements a lot of features (like timeouts
and message cancellation) that are very useful, and which should
really be done in a generic way. We've got code and conventions in
the existing protocol drivers that provide good examples of how
to use the core functionality. I think it's reasonable to pursue
adapting Greybus to be usable in IoT environments.
In any case, one thing that occurred to me during that week and
those discussions was that we really need to define a new *target*
for Greybus. That is, we no longer have the Ara phone as the
hardware target, and we need to somehow define another ultimate
goal for the hardware or environment in which Greybus operates.
This should be more abstract than a hardware definition; instead
it ought to define properties of and services provided by the
entities communicating in a Greybus system. In some ways we
already have that (Interfaces and Modules are abstract, but
the details of what they represent may no longer match what
is required).
I think the things that Greybus defines now that can be pretty
universally useful are the core concepts:
- generic Greybus host device driver interface
- connections with fixed protocols that define how messages
carried over them are interpreted
- the format of messages, and the generic message header
- the overall messaging model, including sending, receiving
cancelling messages
- the operations RPC model, including completion semantics
(asynchronous completion, unidirectional operations, first
error result prevails, and the ability to have multiple
outstanding operations)
>From there we get into some things that are almost certainly
useful, but there may need to be some changes:
- The grouping of connections into bundles, which attach to
Linux device drivers. (I think we need this, but there
remains some work to do, especially in relation to the
next item.)
- Manifests to describe module/interface functionality
- Control connection and how it's used
And we can keep going into other things that become more and
more likely to be unnecessary or just plain wrong when we
are targeting something different from an Ara phone:
- SVC and the SVC protocol
The above is not at all exhaustive; I just wanted to try to
identify a few specific things we can maybe start to agree
on.
Currently, the control protocol and SVC protocol are very
much biased toward the Ara hardware. (Other protocols are
too, like time sync and maybe firmware update.) Furthermore,
pieces of these have necessarily made their way into core
Greybus code. As we define a new non-Ara target, we need
to figure out what parts really belong in the Greybus core,
and which things need to be supplied either by external
entities or by protocol drivers. One way to do that might
be to implement the core code as a library, but to be honest
I think the real work lies in teasing things apart so the
core can be defined to supply certain well-known services,
with hooks to allow other capabilities. For a (contrived)
example of what I mean, we may need to provide hooks for
setting up connection routing rather than implementing
it in the core code. In any case, I think we need to look
at what constitutes the true "core" of Greybus services
and what that core needs from its environment in order to
be used.
If Greybus is going to provide a more general service we need
to ensure it provides a stable API. We have structured things
this way since the beginning, but I fear we may have gotten
sloppy because we basically always packaged everything together.
As it is, all the versioning is in place but to my knowledge
we've never really tested it to make sure it works.
I had another point which I've now forgotten... In any case,
I really just hope to get some conversation going.
What do you think?
-Alex
