| Message ID | 20180516165118.16551-1-srinivas.kandagatla@linaro.org |
|---|---|
| Headers | show |
| Series | slimbus: Add QCOM SLIMBus NGD driver | expand |
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 -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html