Thanks for the comments.
On 07/10/17 07:42, Jonathan Neuschäfer wrote:
Hi,
On Fri, Oct 06, 2017 at 05:51:31PM +0200, srinivas.kandagatla@xxxxxxxxxx wrote:
From: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
Slimbus devices use value-element, and information elements to
control device parameters (e.g. value element is used to represent
gain for codec, information element is used to represent interrupt
status for codec when codec interrupt fires).
Messaging APIs are used to set/get these value and information
elements. Slimbus specification uses 8-bit "transaction IDs" for
messages where a read-value is anticipated. Framework uses a table
of pointers to store those TIDs and responds back to the caller in
O(1).
Caller can opt to do synchronous, or asynchronous reads/writes. For
asynchronous operations, the callback will be called from atomic
context.
TX and RX circular rings are used to allow queuing of multiple
transfers per controller. Controller can choose size of these rings
based of controller HW implementation. The buffers are coerently
s/based of/based on/
s/coerently/coherently/
Yep.. will fix this in next version.
mapped so that controller can utilize DMA operations for the
transactions without remapping every transaction buffer.
Statically allocated rings help to improve performance by avoiding
overhead of dynamically allocating transactions on need basis.
Signed-off-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
Tested-by: Naveen Kaje <nkaje@xxxxxxxxxxxxxx>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
---
[...]
+int slim_xfer_msg(struct slim_controller *ctrl,
+ struct slim_device *sbdev, struct slim_val_inf *msg,
+ u8 mc)
+{
+ DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
+ struct slim_msg_txn *txn = &txn_stack;
+ int ret;
+ u16 sl, cur;
+
+ ret = slim_val_inf_sanity(ctrl, msg, mc);
+ if (ret)
+ return ret;
+
+ sl = slim_slicesize(msg->num_bytes);
+
+ dev_dbg(&ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
+ msg->start_offset, msg->num_bytes, mc, sl);
+
+ cur = slim_slicecodefromsize(sl);
+ txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));
Shouldn't this be (cur | (1 << 3)?
cur seems to be redundant TBH, the only difference between cur and sl is
that the slim_slicesize() can give slice size to program for any lengths
between 1-16 bytes. However the slim_slicecodefromsize() can only
handle 1,2,3,4, 6,8,12,16 byte sizes.
So we can delete slim_slicecodefromsize() call and function together.
looks like it was a leftover from downstream.
(Also, what does cur mean? Cursor? Current?)
No Idea!! :-) it is supposed to return slice size as per number of bytes.
+
+ switch (mc) {
+ case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
+ case SLIM_MSG_MC_CHANGE_VALUE:
+ case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
+ case SLIM_MSG_MC_CLEAR_INFORMATION:
+ txn->rl += msg->num_bytes;
+ default:
+ break;
+ }
+
+ if (slim_tid_txn(txn->mt, txn->mc))
+ txn->rl++;
+
+ return slim_processtxn(ctrl, txn);
+}
+EXPORT_SYMBOL_GPL(slim_xfer_msg);
[...]
+/*
+ * slim_request_val_element: change and request a given value element
name should be fixed here..
+ * @sb: client handle requesting elemental message reads, writes.
+ * @msg: Input structure for start-offset, number of bytes to write.
+ * context: can sleep
+ * Returns:
+ * -EINVAL: Invalid parameters
+ * -ETIMEDOUT: If transmission of this message timed out (e.g. due to bus lines
+ * not being clocked or driven by controller)
+ * -ENOTCONN: If the transmitted message was not ACKed by destination device.
Does rbuf contain the old value after this function finishes?
Yep, device should send a reply value with the old value with matching tid.
+ */
+int slim_request_change_val_element(struct slim_device *sb,
+ struct slim_val_inf *msg)
+{
+ struct slim_controller *ctrl = sb->ctrl;
+
+ if (!ctrl)
+ return -EINVAL;
+
+ return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_CHANGE_VALUE);
+}
+EXPORT_SYMBOL_GPL(slim_request_change_val_element);
[...]
+/**
+ * struct slim_pending: context of pending transfers
+ * @cb: callback for this transfer
+ * @ctx: contex for the callback function
s/contex/context/
Will fix all these instances.
+ * @need_tid: True if this transfer need Transaction ID
+ */
+struct slim_pending {
+ void (*cb)(void *ctx, int err);
+ void *ctx;
+ bool need_tid;
+};
Thanks,
Jonathan Neuschäfer
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
