1. 03 Dec, 2014 8 commits
    • Alex Elder's avatar
      greybus: only record message payload size · 7cfa6995
      Alex Elder authored
      An asynchronous operation will want to know how big the response
      message it receives is.  Rather than require the sender to record
      that information, expose a new field "payload_size" available to
      the protocol code for this purpose.
      
      An operation message consists of a header and a payload.  The size
      of the message can be derived from the size of the payload, so
      record only the payload size and not the size of the whole message.
      Reorder the fields in a message structure.
      
      Update the description of the message header structure.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      7cfa6995
    • Alex Elder's avatar
      greybus: don't let i2c code assume non-null payload pointer · 7a9366aa
      Alex Elder authored
      This is in preparation for an upcoming patch, which makes the
      payload pointer be NULL when a message has zero bytes of payload.
      
      It ensures a null payload pointer never gets dereferenced.  To do
      this we pass the response structure to gb_i2c_transfer_response()
      rather than just its data, and if it's null, returning immediately.
      
      Rearrange the logic in gb_i2c_transfer_operation() a bit.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      7a9366aa
    • Alex Elder's avatar
      greybus: set up connection->private properly · 93bbe859
      Alex Elder authored
      The connection->private pointer should refer to a protocol-specific
      data structure.  Change two protocol drivers (USB and vibrator) so
      they now set this.
      
      In addition, because the setup routine may need access to the
      data structure, the private pointer should be set early--as
      early as possible.  Make the UART, i2c, and GPIO protocol drivers
      set the private pointer earlier.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      93bbe859
    • Alex Elder's avatar
      greybus: fix an error message · 62749a05
      Alex Elder authored
      The error message printed by gb_operation_sync() if the operation
      fails is wrong.  Fix it.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      62749a05
    • Alex Elder's avatar
      greybus: introduce gb_operation_request_send_sync() · c25572ca
      Alex Elder authored
      Define a new function used to initiate a synchronous operation.
      It sends the operation request message and doesn't return until
      the response has been received and/or the operation's result
      has been set.
      
      This gets rid of the convention that a null callback pointer
      signifies a synchronous operation.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      c25572ca
    • Alex Elder's avatar
      greybus: make op_cycle atomic (again) · 4afb7fd0
      Alex Elder authored
      There's no need to protect updating a connections operation id cycle
      counter with the operations spinlock.   That spinlock protects
      connection lists, which do not interact with the cycle counter.
      All that we require is that it gets updated atomically, and we
      can express that requirement in its type.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      4afb7fd0
    • Alex Elder's avatar
      greybus: get rid of pending operations list · afb2e134
      Alex Elder authored
      A connection has two lists of operations, and an operation is always
      on one or the other of them.  One of them contains the operations
      that are currently "in flight".
      
      We really don't expect to have very many in-flight operations on any
      given connection (in fact, at the moment it's always exactly one).
      So there's no significant performance benefit to keeping these in a
      separate list.  An in-flight operation can also be distinguished by
      its errno field holding -EINPROGRESS.
      
      Get rid of the pending list, and search all operations rather than
      the pending list when looking up a response message's operation.
      Rename gb_pending_operation_find() accordingly.
      
      There's no longer any need to remove operations from the pending
      list, and the insertion function no longer has anything to do with a
      pending list.  Just open code what was the insertion function (it
      now has only to do with assigning the operation id).
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      afb2e134
    • Alex Elder's avatar
      greybus: don't use 0 as an operation id · 0ba02c4d
      Alex Elder authored
      Stop allowing 0x0000 to be used as an operation id.  That id will be
      reserved for use by operations that will return no response message.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      0ba02c4d
  2. 02 Dec, 2014 23 commits
  3. 25 Nov, 2014 9 commits
    • Alex Elder's avatar
      greybus: protect cookie with a mutex · 43cdae5c
      Alex Elder authored
      When a Greybus message is sent, the host driver supplies a cookie
      for Greybus to use to identify the sent message in the event it
      needs to be canceled.  The cookie will be non-null while the message
      is in flight, and a null pointer otherwise.
      
      There are two problems with this, which arise out of the fact that a
      message can be canceled at any time--even concurrent with it getting
      sent (such as when Greybus is getting shut down).
      
      First, the host driver's buffer_send method can return an error
      value, which is non-null but not a valid cookie.  So we need to
      ensure such a bogus cookie is never used to cancel a message.
      
      Second, we can't resolve that problem by assigning message->cookie
      only after we've determined it's not an error.  The instant
      buffer_send() returns, the message may well be in flight and *should*
      be canceled at shutdown, so we need the cookie value to reflect
      that.
      
      In order to avoid these problems, protect access to a message's
      cookie value with a mutex.  A spin lock can't be used because the
      window that needs protecting covers code that can block.  We
      reset the cookie value to NULL as soon as the host driver has
      notified us it has been sent (or failed to).
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      43cdae5c
    • Alex Elder's avatar
      greybus: ignore a null cookie when canceling buffer · f34541d7
      Alex Elder authored
      It's possible for an in-flight buffer to be recorded as sent *after*
      a thread has begin the process of canceling it.  In that case the
      Greybus message cookie will be set to NULL, and that value can end
      up getting passed to buffer_cancel().  Change buffer_cancel() so
      it properly handles (ignores) a null cookie pointer.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      f34541d7
    • Alex Elder's avatar
      greybus: update operation result atomically · 894cbc31
      Alex Elder authored
      An operation result can be set both in and out of interrupt context.
      For example, a response message could be arriving at the same time a
      timeout of the operation is getting processed.  We therefore need to
      ensure the result is accessed atomically.
      
      Protect updates to the errno field using the operations spinlock.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      894cbc31
    • Alex Elder's avatar
      greybus: enforce receive buffer size · aa3a4d12
      Alex Elder authored
      When an operation is created its receive buffer size is specified.
      In all current cases, the size supplied for the receive buffer is
      exactly the size that should be returned.  In other words, if
      any fewer than that many bytes arrived in a response, it would be
      an error.
      
      So tighten the check on the number of bytes arriving for a response
      message, ensuring that the number of bytes received is *exactly the
      same* as the number of bytes available (rather than just less than).
      We'll expand our interpretation of of -EMSGSIZE to mean "wrong
      message size" rather than just "message too long."
      
      If we someday encounter an actual case where we want to be able to
      successfully receive something less than the full receive buffer we
      can adjust the code to handle that (and give it a way to tell the
      receiver how many bytes are present).
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      aa3a4d12
    • Alex Elder's avatar
      greybus: fix some error codes · 1a365154
      Alex Elder authored
      Change the message result values used in two cases.
      
      First, use -EMSGSIZE rather than -E2BIG to represent a message
      that is larger than the buffer intended to hold it.  That is
      the proper code for this situation.
      
      Second, use -ECANCELED rather than -EINTR for an operation that
      has been canceled.  The definition of that error is literally
      "Operation Canceled" so it seems like the right thing to do.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      1a365154
    • Alex Elder's avatar
      greybus: use special operation result valus · 3deb37d4
      Alex Elder authored
      This is more or less re-implementing this commit:
          96f95d4 greybus: update gbuf status for completion handlers
      But this time we're doing this for an operation, not the gbuf.
      
      Define an initial operation result value (-EBADR) to signify that no
      valid result has been set.  Nobody should ever set that value after
      the operation is initially created.  Since only the operation core
      code sets the result, an attempt to set -EBADR would be a bug.
      
      Define another known operation result value (-EINPROGRESS) for an
      outgoing operation whose request has been sent but whose response
      has not yet been successfully received.  This should the first
      (non-initial) result value set, and it should happen exactly once.
      Any other attempt to set this value once set would be a bug.
      
      Finally, once the request message is in flight, the result value
      will be set exactly once more, to indicate the final result of
      the operation.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      3deb37d4
    • Alex Elder's avatar
      greybus: first operation error prevails · abe9a300
      Alex Elder authored
      If an operation already has an error result recorded, don't
      overwrite it with a new error code.
      
      In order to ensure a request completes exactly once, return a
      Boolean indicating whether setting the result was successful.  If
      two threads are racing to complete an operation (for example if a
      slow-but-normal response message arrives at the same time timeout
      processing commences) only the one that sets the final result
      will finish its activity.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      abe9a300
    • Alex Elder's avatar
      greybus: encapsulate operation result access · ba986b5a
      Alex Elder authored
      Hide the setting and getting of the operation result (stored in
      operation->errno) behind a pair of accessor functions.  Only the
      operation core should be setting the result, but operations that
      complete asynchronously will need access to the result so expose
      the function that provides that.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      ba986b5a
    • Greg Kroah-Hartman's avatar
      greybus: uart-gb: clean up send_line_coding · 9f240f20
      Greg Kroah-Hartman authored
      We always pass the same option to send_line_coding() for the line_coding
      structure, which is already in the struct gb_tty variable, so just
      remove the second parameter as it's not needed.
      
      This logic came from the cdc-acm.c driver, where it's also not needed
      anymore, I'll go fix up that later on when I get a chance.
      Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
      9f240f20