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@linaro.org wrote:
From: Sagar Dharia sdharia@codeaurora.org
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@codeaurora.org Tested-by: Naveen Kaje nkaje@codeaurora.org Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
[...]
+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