| Message ID | 1463694268-9473-2-git-send-email-austinwc@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andy Gross |
| Headers | show |
Hi, >From: Naveen Kaje <nkaje@codeaurora.org> > >I2C QUP driver relies on SMBus emulation support from the framework. >To handle SMBus block reads, the driver should check I2C_M_RECV_LEN >flag and should read the first byte received as the message length. > >The driver configures the QUP hardware to read one byte. Once the >message length is known from this byte, the QUP hardware is configured >to read the rest. > >Signed-off-by: Naveen Kaje <nkaje@codeaurora.org> >Signed-off-by: Austin Christ <austinwc@codeaurora.org> >--- > drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 65 insertions(+), 3 deletions(-) > >Changes: >- v3: > - clean up redundant checks > - use constant instead of variable for smbus length field >- v2: > - rework the smbus block read and break into separate function > >diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c >index ea6ca5f..9fbed83 100644 >--- a/drivers/i2c/busses/i2c-qup.c >+++ b/drivers/i2c/busses/i2c-qup.c >@@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup) > return data_len; > } > >+static bool qup_i2c_check_msg_len(struct i2c_msg *msg) >+{ >+ return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN)); >+} >+ >+static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup, >+ struct i2c_msg *msg) >+{ >+ int len = 0; >+ >+ if (msg->len > 1) { >+ tags[len++] = QUP_TAG_V2_DATARD_STOP; >+ tags[len++] = qup_i2c_get_data_len(qup) - 1; >+ } else { >+ tags[len++] = QUP_TAG_V2_START; >+ tags[len++] = addr & 0xff; >+ >+ if (msg->flags & I2C_M_TEN) >+ tags[len++] = addr >> 8; >+ >+ tags[len++] = QUP_TAG_V2_DATARD; >+ /* Read 1 byte indicating the length of the SMBus message */ >+ tags[len++] = 1; >+ } >+ return len; >+} >+ > static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, > struct i2c_msg *msg, int is_dma) > { >@@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, > > int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last); > >+ /* Handle tags for SMBus block read */ >+ if (qup_i2c_check_msg_len(msg)) >+ return qup_i2c_set_tags_smb(addr, tags, qup, msg); >+ > if (qup->blk.pos == 0) { > tags[len++] = QUP_TAG_V2_START; > tags[len++] = addr & 0xff; >@@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, > struct i2c_msg *msg) > { > u32 val; >- int idx, pos = 0, ret = 0, total; >+ int idx, pos = 0, ret = 0, total, msg_offset = 0; > >+ /* >+ * If the message length is already read in >+ * the first byte of the buffer, account for >+ * that by setting the offset >+ */ >+ if (qup_i2c_check_msg_len(msg) && (msg->len > 1)) >+ msg_offset = 1; > total = qup_i2c_get_data_len(qup); >+ total -= msg_offset; > > /* 2 extra bytes for read tags */ > while (pos < (total + 2)) { >@@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, > > if (pos >= (total + 2)) > goto out; >- >- msg->buf[qup->pos++] = val & 0xff; >+ msg->buf[qup->pos + msg_offset] = val & 0xff; >+ qup->pos++; > } > } > >@@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg) > goto err; > > qup->blk.pos++; >+ >+ /* Handle SMBus block read length */ >+ if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) { >+ if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) { >+ ret = -EPROTO; >+ goto err; >+ } >+ msg->len += msg->buf[0]; >+ qup->pos = 0; >+ qup_i2c_set_read_mode_v2(qup, msg->len); >+ ret = qup_i2c_issue_xfer_v2(qup, msg); >+ if (ret) >+ goto err; >+ ret = qup_i2c_wait_for_complete(qup, msg); >+ if (ret) >+ goto err; >+ qup_i2c_set_blk_data(qup, msg); >+ } > } while (qup->blk.pos < qup->blk.count); > > err: >@@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, > goto out; > } > >+ if (qup_i2c_check_msg_len(&msgs[idx])) { >+ ret = -EOPNOTSUPP; >+ goto out; >+ } >+ > if (msgs[idx].flags & I2C_M_RD) > ret = qup_i2c_read_one(qup, &msgs[idx]); > else Reviewed-by: sricharan@codeaurora.org Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 20, 2016 at 3:14 AM, Austin Christ <austinwc@codeaurora.org> wrote: > From: Naveen Kaje <nkaje@codeaurora.org> > > I2C QUP driver relies on SMBus emulation support from the framework. > To handle SMBus block reads, the driver should check I2C_M_RECV_LEN > flag and should read the first byte received as the message length. > > The driver configures the QUP hardware to read one byte. Once the > message length is known from this byte, the QUP hardware is configured > to read the rest. > > Signed-off-by: Naveen Kaje <nkaje@codeaurora.org> > Signed-off-by: Austin Christ <austinwc@codeaurora.org> > --- > drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 65 insertions(+), 3 deletions(-) > > Changes: > - v3: > - clean up redundant checks > - use constant instead of variable for smbus length field > - v2: > - rework the smbus block read and break into separate function > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index ea6ca5f..9fbed83 100644 > --- a/drivers/i2c/busses/i2c-qup.c > +++ b/drivers/i2c/busses/i2c-qup.c > @@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup) > return data_len; > } > > +static bool qup_i2c_check_msg_len(struct i2c_msg *msg) > +{ > + return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN)); > +} > + > +static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup, > + struct i2c_msg *msg) > +{ > + int len = 0; > + > + if (msg->len > 1) { > + tags[len++] = QUP_TAG_V2_DATARD_STOP; > + tags[len++] = qup_i2c_get_data_len(qup) - 1; > + } else { > + tags[len++] = QUP_TAG_V2_START; > + tags[len++] = addr & 0xff; > + > + if (msg->flags & I2C_M_TEN) > + tags[len++] = addr >> 8; > + > + tags[len++] = QUP_TAG_V2_DATARD; > + /* Read 1 byte indicating the length of the SMBus message */ > + tags[len++] = 1; > + } > + return len; > +} > + > static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, > struct i2c_msg *msg, int is_dma) > { > @@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, > > int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last); > > + /* Handle tags for SMBus block read */ > + if (qup_i2c_check_msg_len(msg)) > + return qup_i2c_set_tags_smb(addr, tags, qup, msg); > + > if (qup->blk.pos == 0) { > tags[len++] = QUP_TAG_V2_START; > tags[len++] = addr & 0xff; > @@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, > struct i2c_msg *msg) > { > u32 val; > - int idx, pos = 0, ret = 0, total; > + int idx, pos = 0, ret = 0, total, msg_offset = 0; > > + /* > + * If the message length is already read in > + * the first byte of the buffer, account for > + * that by setting the offset > + */ > + if (qup_i2c_check_msg_len(msg) && (msg->len > 1)) > + msg_offset = 1; > total = qup_i2c_get_data_len(qup); > + total -= msg_offset; > > /* 2 extra bytes for read tags */ > while (pos < (total + 2)) { > @@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, > > if (pos >= (total + 2)) > goto out; > - > - msg->buf[qup->pos++] = val & 0xff; > + msg->buf[qup->pos + msg_offset] = val & 0xff; > + qup->pos++; > } > } > > @@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg) > goto err; > > qup->blk.pos++; > + > + /* Handle SMBus block read length */ > + if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) { > + if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) { > + ret = -EPROTO; > + goto err; > + } > + msg->len += msg->buf[0]; > + qup->pos = 0; > + qup_i2c_set_read_mode_v2(qup, msg->len); > + ret = qup_i2c_issue_xfer_v2(qup, msg); > + if (ret) > + goto err; > + ret = qup_i2c_wait_for_complete(qup, msg); > + if (ret) > + goto err; > + qup_i2c_set_blk_data(qup, msg); > + } > } while (qup->blk.pos < qup->blk.count); > > err: > @@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, > goto out; > } > > + if (qup_i2c_check_msg_len(&msgs[idx])) { > + ret = -EOPNOTSUPP; Could you please rethink on this return value ? It should not be "EOPNOTSUPP" ~Rajeev > + goto out; > + } > + > if (msgs[idx].flags & I2C_M_RD) > ret = qup_i2c_read_one(qup, &msgs[idx]); > else > -- > Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 5/25/2016 3:31 AM, rajeev kumar wrote: > On Fri, May 20, 2016 at 3:14 AM, Austin Christ <austinwc@codeaurora.org> wrote: >> From: Naveen Kaje <nkaje@codeaurora.org> >> >> I2C QUP driver relies on SMBus emulation support from the framework. >> To handle SMBus block reads, the driver should check I2C_M_RECV_LEN >> flag and should read the first byte received as the message length. >> >> The driver configures the QUP hardware to read one byte. Once the >> message length is known from this byte, the QUP hardware is configured >> to read the rest. >> >> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org> >> Signed-off-by: Austin Christ <austinwc@codeaurora.org> >> --- >> drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 65 insertions(+), 3 deletions(-) >> >> Changes: >> - v3: >> - clean up redundant checks >> - use constant instead of variable for smbus length field >> - v2: >> - rework the smbus block read and break into separate function >> >> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c >> index ea6ca5f..9fbed83 100644 >> --- a/drivers/i2c/busses/i2c-qup.c >> +++ b/drivers/i2c/busses/i2c-qup.c >> @@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup) >> return data_len; >> } >> >> +static bool qup_i2c_check_msg_len(struct i2c_msg *msg) >> +{ >> + return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN)); >> +} >> + >> +static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup, >> + struct i2c_msg *msg) >> +{ >> + int len = 0; >> + >> + if (msg->len > 1) { >> + tags[len++] = QUP_TAG_V2_DATARD_STOP; >> + tags[len++] = qup_i2c_get_data_len(qup) - 1; >> + } else { >> + tags[len++] = QUP_TAG_V2_START; >> + tags[len++] = addr & 0xff; >> + >> + if (msg->flags & I2C_M_TEN) >> + tags[len++] = addr >> 8; >> + >> + tags[len++] = QUP_TAG_V2_DATARD; >> + /* Read 1 byte indicating the length of the SMBus message */ >> + tags[len++] = 1; >> + } >> + return len; >> +} >> + >> static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, >> struct i2c_msg *msg, int is_dma) >> { >> @@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, >> >> int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last); >> >> + /* Handle tags for SMBus block read */ >> + if (qup_i2c_check_msg_len(msg)) >> + return qup_i2c_set_tags_smb(addr, tags, qup, msg); >> + >> if (qup->blk.pos == 0) { >> tags[len++] = QUP_TAG_V2_START; >> tags[len++] = addr & 0xff; >> @@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, >> struct i2c_msg *msg) >> { >> u32 val; >> - int idx, pos = 0, ret = 0, total; >> + int idx, pos = 0, ret = 0, total, msg_offset = 0; >> >> + /* >> + * If the message length is already read in >> + * the first byte of the buffer, account for >> + * that by setting the offset >> + */ >> + if (qup_i2c_check_msg_len(msg) && (msg->len > 1)) >> + msg_offset = 1; >> total = qup_i2c_get_data_len(qup); >> + total -= msg_offset; >> >> /* 2 extra bytes for read tags */ >> while (pos < (total + 2)) { >> @@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, >> >> if (pos >= (total + 2)) >> goto out; >> - >> - msg->buf[qup->pos++] = val & 0xff; >> + msg->buf[qup->pos + msg_offset] = val & 0xff; >> + qup->pos++; >> } >> } >> >> @@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg) >> goto err; >> >> qup->blk.pos++; >> + >> + /* Handle SMBus block read length */ >> + if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) { >> + if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) { >> + ret = -EPROTO; >> + goto err; >> + } >> + msg->len += msg->buf[0]; >> + qup->pos = 0; >> + qup_i2c_set_read_mode_v2(qup, msg->len); >> + ret = qup_i2c_issue_xfer_v2(qup, msg); >> + if (ret) >> + goto err; >> + ret = qup_i2c_wait_for_complete(qup, msg); >> + if (ret) >> + goto err; >> + qup_i2c_set_blk_data(qup, msg); >> + } >> } while (qup->blk.pos < qup->blk.count); >> >> err: >> @@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, >> goto out; >> } >> >> + if (qup_i2c_check_msg_len(&msgs[idx])) { >> + ret = -EOPNOTSUPP; > Could you please rethink on this return value ? It should not be "EOPNOTSUPP" > > ~Rajeev This will be replaced with "EINVAL" in v4 patches. > >> + goto out; >> + } >> + >> if (msgs[idx].flags & I2C_M_RD) >> ret = qup_i2c_read_one(qup, &msgs[idx]); >> else >> -- >> Qualcomm Innovation Center, Inc. >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >> a Linux Foundation Collaborative Project. >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Thanks, Austin -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index ea6ca5f..9fbed83 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup) return data_len; } +static bool qup_i2c_check_msg_len(struct i2c_msg *msg) +{ + return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN)); +} + +static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup, + struct i2c_msg *msg) +{ + int len = 0; + + if (msg->len > 1) { + tags[len++] = QUP_TAG_V2_DATARD_STOP; + tags[len++] = qup_i2c_get_data_len(qup) - 1; + } else { + tags[len++] = QUP_TAG_V2_START; + tags[len++] = addr & 0xff; + + if (msg->flags & I2C_M_TEN) + tags[len++] = addr >> 8; + + tags[len++] = QUP_TAG_V2_DATARD; + /* Read 1 byte indicating the length of the SMBus message */ + tags[len++] = 1; + } + return len; +} + static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, struct i2c_msg *msg, int is_dma) { @@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup, int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last); + /* Handle tags for SMBus block read */ + if (qup_i2c_check_msg_len(msg)) + return qup_i2c_set_tags_smb(addr, tags, qup, msg); + if (qup->blk.pos == 0) { tags[len++] = QUP_TAG_V2_START; tags[len++] = addr & 0xff; @@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg) { u32 val; - int idx, pos = 0, ret = 0, total; + int idx, pos = 0, ret = 0, total, msg_offset = 0; + /* + * If the message length is already read in + * the first byte of the buffer, account for + * that by setting the offset + */ + if (qup_i2c_check_msg_len(msg) && (msg->len > 1)) + msg_offset = 1; total = qup_i2c_get_data_len(qup); + total -= msg_offset; /* 2 extra bytes for read tags */ while (pos < (total + 2)) { @@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup, if (pos >= (total + 2)) goto out; - - msg->buf[qup->pos++] = val & 0xff; + msg->buf[qup->pos + msg_offset] = val & 0xff; + qup->pos++; } } @@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg) goto err; qup->blk.pos++; + + /* Handle SMBus block read length */ + if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) { + if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) { + ret = -EPROTO; + goto err; + } + msg->len += msg->buf[0]; + qup->pos = 0; + qup_i2c_set_read_mode_v2(qup, msg->len); + ret = qup_i2c_issue_xfer_v2(qup, msg); + if (ret) + goto err; + ret = qup_i2c_wait_for_complete(qup, msg); + if (ret) + goto err; + qup_i2c_set_blk_data(qup, msg); + } } while (qup->blk.pos < qup->blk.count); err: @@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, goto out; } + if (qup_i2c_check_msg_len(&msgs[idx])) { + ret = -EOPNOTSUPP; + goto out; + } + if (msgs[idx].flags & I2C_M_RD) ret = qup_i2c_read_one(qup, &msgs[idx]); else