On 16-05-18, 17:51, Srinivas Kandagatla wrote:
This patch adds suppor to Qualcomm SLIMBus Non-Generic Device (NGD)
/s/suppor/support
+/* NGD (Non-ported Generic Device) registers */ +#define NGD_CFG 0x0 +#define NGD_CFG_ENABLE BIT(0) +#define NGD_CFG_RX_MSGQ_EN BIT(1) +#define NGD_CFG_TX_MSGQ_EN BIT(2) +#define NGD_STATUS 0x4 +#define NGD_LADDR BIT(1) +#define NGD_RX_MSGQ_CFG 0x8 +#define NGD_INT_EN 0x10 +#define NGD_INT_RECFG_DONE BIT(24) +#define NGD_INT_TX_NACKED_2 BIT(25) +#define NGD_INT_MSG_BUF_CONTE BIT(26) +#define NGD_INT_MSG_TX_INVAL BIT(27) +#define NGD_INT_IE_VE_CHG BIT(28) +#define NGD_INT_DEV_ERR BIT(29) +#define NGD_INT_RX_MSG_RCVD BIT(30) +#define NGD_INT_TX_MSG_SENT BIT(31) +#define NGD_INT_STAT 0x14 +#define NGD_INT_CLR 0x18 +#define DEF_NGD_INT_MASK (NGD_INT_TX_NACKED_2 | NGD_INT_MSG_BUF_CONTE | \
NGD_INT_MSG_TX_INVAL | NGD_INT_IE_VE_CHG | \NGD_INT_DEV_ERR | NGD_INT_TX_MSG_SENT | \NGD_INT_RX_MSG_RCVD)+/* Slimbus QMI service */ +#define SLIMBUS_QMI_SVC_ID 0x0301 +#define SLIMBUS_QMI_SVC_V1 1 +#define SLIMBUS_QMI_INS_ID 0 +#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01 0x0020 +#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_V01 0x0020 +#define SLIMBUS_QMI_POWER_REQ_V01 0x0021 +#define SLIMBUS_QMI_POWER_RESP_V01 0x0021 +#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_REQ 0x0022 +#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_RESP 0x0022 +#define SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN 14 +#define SLIMBUS_QMI_POWER_RESP_MAX_MSG_LEN 7 +#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_MAX_MSG_LEN 14 +#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_MAX_MSG_LEN 7 +#define SLIMBUS_QMI_CHECK_FRAMER_STAT_RESP_MAX_MSG_LEN 7 +/* QMI response timeout of 500ms */ +#define SLIMBUS_QMI_RESP_TOUT 1000
Tabs or spaces, take your pick and use one, not both...
+static void qcom_slim_qmi_power_resp_cb(struct qmi_handle *handle,
struct sockaddr_qrtr *sq,struct qmi_txn *txn, const void *data)+{
- struct slimbus_power_resp_msg_v01 *resp;
- resp = (struct slimbus_power_resp_msg_v01 *)data;
you dont need cast away from void
- if (resp->resp.result != QMI_RESULT_SUCCESS_V01)
pr_err("%s: QMI power request failed 0x%x\n", __func__,resp->resp.result);
cant we use dev_err?
+static int qcom_slim_qmi_send_power_request(struct qcom_slim_ngd_ctrl *ctrl,
struct slimbus_power_req_msg_v01 *req)+{
- struct slimbus_power_resp_msg_v01 resp = { { 0, 0 } };
- struct qmi_txn txn;
- int rc;
- rc = qmi_txn_init(ctrl->qmi.handle, &txn,
slimbus_power_resp_msg_v01_ei, &resp);- rc = qmi_send_request(ctrl->qmi.handle, NULL, &txn,
SLIMBUS_QMI_POWER_REQ_V01,SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN,slimbus_power_req_msg_v01_ei, req);- if (rc < 0) {
dev_err(ctrl->dev, "%s: QMI send req fail %d\n", __func__, rc);qmi_txn_cancel(&txn);- }
- if (rc < 0)
return rc;
why not add this is prev error check?
+static int qcom_slim_qmi_init(struct qcom_slim_ngd_ctrl *ctrl,
bool apps_is_master)+{
- int rc = 0;
superfluous init
+static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
struct completion *comp)+{
- struct qcom_slim_ngd_dma_desc *desc;
- unsigned long flags;
- spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
- if ((ctrl->tx_tail + 1) % QCOM_SLIM_NGD_DESC_NUM == ctrl->tx_head) {
spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);return NULL;- }
- desc = &ctrl->txdesc[ctrl->tx_tail];
- desc->base = (u32 *)((u8 *)ctrl->tx_base +
(ctrl->tx_tail * SLIM_MSGQ_BUF_LEN));
too many casts
+static int qcom_slim_ngd_post_rx_msgq(struct qcom_slim_ngd_ctrl *ctrl) +{
- struct qcom_slim_ngd_dma_desc *desc;
- struct dma_slave_config conf = {
.direction = DMA_DEV_TO_MEM,- };
- int ret, i;
- ret = dmaengine_slave_config(ctrl->dma_rx_channel, &conf);
so you are only setting direction for conf, not any other parameters? If not why bother setting direction
- if (ret)
dev_err(ctrl->dev, "Error Configuring rx dma\n");- for (i = 0; i < QCOM_SLIM_NGD_DESC_NUM; i++) {
desc = &ctrl->rx_desc[i];desc->phys = ctrl->rx_phys_base + i * SLIM_MSGQ_BUF_LEN;desc->ctrl = ctrl;desc->base = ctrl->rx_base + i * SLIM_MSGQ_BUF_LEN;desc->desc = dmaengine_prep_slave_single(ctrl->dma_rx_channel,desc->phys, SLIM_MSGQ_BUF_LEN,DMA_DEV_TO_MEM,DMA_PREP_INTERRUPT);
why issue multiple slave_single to dmaengine, you can bundle them and issue dmaengine_prep_slave_sg()..
+static int qcom_slim_ngd_qmi_svc_event_init(struct qcom_slim_ngd_qmi *qmi) +{
- int ret = 0;
superfluous init here too
+static int qcom_slim_ngd_runtime_idle(struct device *dev) +{
- struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
- if (ctrl->state == QCOM_SLIM_NGD_CTRL_AWAKE)
ctrl->state = QCOM_SLIM_NGD_CTRL_IDLE;- pm_request_autosuspend(dev);
- return -EAGAIN;
+}
double empty lines, here