Re: [PATCH 11/12] i2c: qup: reorganization of driver code to remove polling for qup v1
Hi Abhishek,
On 2/19/2018 6:51 PM, Abhishek Sahu wrote:
> On 2018-02-16 13:14, Sricharan R wrote:
>> Hi Abhishek,
>>
>> On 2/3/2018 1:28 PM, Abhishek Sahu wrote:
>>> Following are the major issues in current driver code
>>>
>>> 1. The current driver simply assumes the transfer completion
>>> whenever its gets any non-error interrupts and then simply do the
>>> polling of available/free bytes in FIFO.
>>> 2. The block mode is not working properly since no handling in
>>> being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ.
>>>
>>> Because of above, i2c v1 transfers of size greater than 32 are failing
>>> with following error message
>>>
>>> i2c_qup 78b6000.i2c: timeout for fifo out full
>>>
>>> To make block mode working properly and move to use the interrupts
>>> instead of polling, major code reorganization is required. Following
>>> are the major changes done in this patch
>>>
>>> 1. Remove the polling of TX FIFO free space and RX FIFO available
>>> bytes and move to interrupts completely. QUP has QUP_MX_OUTPUT_DONE,
>>> QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
>>> interrupts to handle FIFO’s properly so check all these interrupts.
>>> 2. During write, For FIFO mode, TX FIFO can be directly written
>>> without checking for FIFO space. For block mode, the QUP will generate
>>> OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of available
>>> space.
>>> 3. During read, both TX and RX FIFO will be used. TX will be used
>>> for writing tags and RX will be used for receiving the data. In QUP,
>>> TX and RX can operate in separate mode so configure modes accordingly.
>>> 4. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
>>> will be generated after all the bytes have been copied in RX FIFO. For
>>> read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
>>> whenever it has block size of available data.
>>>
>>> Signed-off-by: Abhishek Sahu
>>> ---
>>> drivers/i2c/busses/i2c-qup.c | 399
>>> ---
>>> 1 file changed, 257 insertions(+), 142 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>>> index edea3b9..af6c21a 100644
>>> --- a/drivers/i2c/busses/i2c-qup.c
>>> +++ b/drivers/i2c/busses/i2c-qup.c
>>> @@ -73,8 +73,11 @@
>>> #define QUP_IN_SVC_FLAG BIT(9)
>>> #define QUP_MX_OUTPUT_DONE BIT(10)
>>> #define QUP_MX_INPUT_DONE BIT(11)
>>> +#define OUT_BLOCK_WRITE_REQ BIT(12)
>>> +#define IN_BLOCK_READ_REQ BIT(13)
>>>
>>> /* I2C mini core related values */
>>> +#define QUP_NO_INPUT BIT(7)
>>> #define QUP_CLOCK_AUTO_GATE BIT(13)
>>> #define I2C_MINI_CORE (2 << 8)
>>> #define I2C_N_VAL 15
>>> @@ -138,13 +141,51 @@
>>> #define DEFAULT_CLK_FREQ 10
>>> #define DEFAULT_SRC_CLK 2000
>>>
>>> +/* MAX_OUTPUT_DONE_FLAG has been received */
>>> +#define QUP_BLK_EVENT_TX_IRQ_DONE BIT(0)
>>> +/* MAX_INPUT_DONE_FLAG has been received */
>>> +#define QUP_BLK_EVENT_RX_IRQ_DONE BIT(1)
>>> +/* All the TX bytes have been written in TX FIFO */
>>> +#define QUP_BLK_EVENT_TX_DATA_DONE BIT(2)
>>> +/* All the RX bytes have been read from RX FIFO */
>>> +#define QUP_BLK_EVENT_RX_DATA_DONE BIT(3)
>>> +
>>> +/* All the required events to mark a QUP I2C TX transfer completed */
>>> +#define QUP_BLK_EVENT_TX_DONE (QUP_BLK_EVENT_TX_IRQ_DONE | \
>>> + QUP_BLK_EVENT_TX_DATA_DONE)
>>> +/* All the required events to mark a QUP I2C RX transfer completed */
>>> +#define QUP_BLK_EVENT_RX_DONE (QUP_BLK_EVENT_TX_DONE | \
>>> + QUP_BLK_EVENT_RX_IRQ_DONE | \
>>> + QUP_BLK_EVENT_RX_DATA_DONE)
>>> +
>>> +/*
>>> + * count: no of blocks
>>> + * pos: current block number
>>> + * tx_tag_len: tx tag length for current block
>>> + * rx_tag_len: rx tag length for current block
>>> + * data_len: remaining data length for current message
>>> + * total_tx_len: total tx length including tag bytes for current QUP
>>> transfer
>>> + * total_rx_len: total rx length including tag bytes for current QUP
>>> transfer
>>> + * tx_fifo_free: number of free bytes in current QUP block write.
>>> + * fifo_available: number of available bytes in RX FIFO for current
>>> + * QUP block read
>>> + * is_tx_blk_mode: whether tx uses block or FIFO mode in case of non BAM
>>> xfer.
>>> + * is_rx_blk_mode: whether rx uses block or FIFO mode in case of non BAM
>>> xfer.
>>> + * tags: contains tx tag bytes for current QUP transfer
>>> + */
>>> struct qup_i2c_block {
>>> - int count;
>>> - int pos;
>>> - int tx_tag_len;
>>> - int rx_tag_len;
>>> - int data_len;
>>> - u8 tags[6];
>>> + int count;
>>> + int pos;
>>> + int tx_tag_len;
>>> + int rx_tag_len;
>>> + int data_len;
>>> + int total_tx_len;
>>> + int total_rx_len;
>>> + int
Re: [PATCH 11/12] i2c: qup: reorganization of driver code to remove polling for qup v1
On 2018-02-16 13:14, Sricharan R wrote:
Hi Abhishek,
On 2/3/2018 1:28 PM, Abhishek Sahu wrote:
Following are the major issues in current driver code
1. The current driver simply assumes the transfer completion
whenever its gets any non-error interrupts and then simply do the
polling of available/free bytes in FIFO.
2. The block mode is not working properly since no handling in
being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ.
Because of above, i2c v1 transfers of size greater than 32 are failing
with following error message
i2c_qup 78b6000.i2c: timeout for fifo out full
To make block mode working properly and move to use the interrupts
instead of polling, major code reorganization is required. Following
are the major changes done in this patch
1. Remove the polling of TX FIFO free space and RX FIFO available
bytes and move to interrupts completely. QUP has
QUP_MX_OUTPUT_DONE,
QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
interrupts to handle FIFO’s properly so check all these interrupts.
2. During write, For FIFO mode, TX FIFO can be directly written
without checking for FIFO space. For block mode, the QUP will
generate
OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of
available
space.
3. During read, both TX and RX FIFO will be used. TX will be used
for writing tags and RX will be used for receiving the data. In
QUP,
TX and RX can operate in separate mode so configure modes
accordingly.
4. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
will be generated after all the bytes have been copied in RX FIFO.
For
read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
whenever it has block size of available data.
Signed-off-by: Abhishek Sahu
---
drivers/i2c/busses/i2c-qup.c | 399
---
1 file changed, 257 insertions(+), 142 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qup.c
b/drivers/i2c/busses/i2c-qup.c
index edea3b9..af6c21a 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -73,8 +73,11 @@
#define QUP_IN_SVC_FLAGBIT(9)
#define QUP_MX_OUTPUT_DONE BIT(10)
#define QUP_MX_INPUT_DONE BIT(11)
+#define OUT_BLOCK_WRITE_REQBIT(12)
+#define IN_BLOCK_READ_REQ BIT(13)
/* I2C mini core related values */
+#define QUP_NO_INPUT BIT(7)
#define QUP_CLOCK_AUTO_GATEBIT(13)
#define I2C_MINI_CORE (2 << 8)
#define I2C_N_VAL 15
@@ -138,13 +141,51 @@
#define DEFAULT_CLK_FREQ 10
#define DEFAULT_SRC_CLK 2000
+/* MAX_OUTPUT_DONE_FLAG has been received */
+#define QUP_BLK_EVENT_TX_IRQ_DONE BIT(0)
+/* MAX_INPUT_DONE_FLAG has been received */
+#define QUP_BLK_EVENT_RX_IRQ_DONE BIT(1)
+/* All the TX bytes have been written in TX FIFO */
+#define QUP_BLK_EVENT_TX_DATA_DONE BIT(2)
+/* All the RX bytes have been read from RX FIFO */
+#define QUP_BLK_EVENT_RX_DATA_DONE BIT(3)
+
+/* All the required events to mark a QUP I2C TX transfer completed */
+#define QUP_BLK_EVENT_TX_DONE (QUP_BLK_EVENT_TX_IRQ_DONE | \
+QUP_BLK_EVENT_TX_DATA_DONE)
+/* All the required events to mark a QUP I2C RX transfer completed */
+#define QUP_BLK_EVENT_RX_DONE (QUP_BLK_EVENT_TX_DONE | \
+QUP_BLK_EVENT_RX_IRQ_DONE | \
+QUP_BLK_EVENT_RX_DATA_DONE)
+
+/*
+ * count: no of blocks
+ * pos: current block number
+ * tx_tag_len: tx tag length for current block
+ * rx_tag_len: rx tag length for current block
+ * data_len: remaining data length for current message
+ * total_tx_len: total tx length including tag bytes for current QUP
transfer
+ * total_rx_len: total rx length including tag bytes for current QUP
transfer
+ * tx_fifo_free: number of free bytes in current QUP block write.
+ * fifo_available: number of available bytes in RX FIFO for current
+ *QUP block read
+ * is_tx_blk_mode: whether tx uses block or FIFO mode in case of non
BAM xfer.
+ * is_rx_blk_mode: whether rx uses block or FIFO mode in case of non
BAM xfer.
+ * tags: contains tx tag bytes for current QUP transfer
+ */
struct qup_i2c_block {
- int count;
- int pos;
- int tx_tag_len;
- int rx_tag_len;
- int data_len;
- u8 tags[6];
+ int count;
+ int pos;
+ int tx_tag_len;
+ int rx_tag_len;
+ int data_len;
+ int total_tx_len;
+ int total_rx_len;
+ int tx_fifo_free;
+ int fifo_available;
+ boolis_tx_blk_mode;
+ boolis_rx_blk_mode;
+ u8 tags[6];
};
struct qup_i2c_tag {
@@ -187,6 +228,7 @@ struct qup_i2c_dev {
/* To check if this is th
Re: [PATCH 11/12] i2c: qup: reorganization of driver code to remove polling for qup v1
Hi Abhishek,
On 2/3/2018 1:28 PM, Abhishek Sahu wrote:
> Following are the major issues in current driver code
>
> 1. The current driver simply assumes the transfer completion
>whenever its gets any non-error interrupts and then simply do the
>polling of available/free bytes in FIFO.
> 2. The block mode is not working properly since no handling in
>being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ.
>
> Because of above, i2c v1 transfers of size greater than 32 are failing
> with following error message
>
> i2c_qup 78b6000.i2c: timeout for fifo out full
>
> To make block mode working properly and move to use the interrupts
> instead of polling, major code reorganization is required. Following
> are the major changes done in this patch
>
> 1. Remove the polling of TX FIFO free space and RX FIFO available
>bytes and move to interrupts completely. QUP has QUP_MX_OUTPUT_DONE,
>QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
>interrupts to handle FIFO’s properly so check all these interrupts.
> 2. During write, For FIFO mode, TX FIFO can be directly written
>without checking for FIFO space. For block mode, the QUP will generate
>OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of available
>space.
> 3. During read, both TX and RX FIFO will be used. TX will be used
>for writing tags and RX will be used for receiving the data. In QUP,
>TX and RX can operate in separate mode so configure modes accordingly.
> 4. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
>will be generated after all the bytes have been copied in RX FIFO. For
>read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
>whenever it has block size of available data.
>
> Signed-off-by: Abhishek Sahu
> ---
> drivers/i2c/busses/i2c-qup.c | 399
> ---
> 1 file changed, 257 insertions(+), 142 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index edea3b9..af6c21a 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -73,8 +73,11 @@
> #define QUP_IN_SVC_FLAG BIT(9)
> #define QUP_MX_OUTPUT_DONE BIT(10)
> #define QUP_MX_INPUT_DONEBIT(11)
> +#define OUT_BLOCK_WRITE_REQ BIT(12)
> +#define IN_BLOCK_READ_REQBIT(13)
>
> /* I2C mini core related values */
> +#define QUP_NO_INPUT BIT(7)
> #define QUP_CLOCK_AUTO_GATE BIT(13)
> #define I2C_MINI_CORE(2 << 8)
> #define I2C_N_VAL15
> @@ -138,13 +141,51 @@
> #define DEFAULT_CLK_FREQ 10
> #define DEFAULT_SRC_CLK 2000
>
> +/* MAX_OUTPUT_DONE_FLAG has been received */
> +#define QUP_BLK_EVENT_TX_IRQ_DONEBIT(0)
> +/* MAX_INPUT_DONE_FLAG has been received */
> +#define QUP_BLK_EVENT_RX_IRQ_DONEBIT(1)
> +/* All the TX bytes have been written in TX FIFO */
> +#define QUP_BLK_EVENT_TX_DATA_DONE BIT(2)
> +/* All the RX bytes have been read from RX FIFO */
> +#define QUP_BLK_EVENT_RX_DATA_DONE BIT(3)
> +
> +/* All the required events to mark a QUP I2C TX transfer completed */
> +#define QUP_BLK_EVENT_TX_DONE(QUP_BLK_EVENT_TX_IRQ_DONE | \
> + QUP_BLK_EVENT_TX_DATA_DONE)
> +/* All the required events to mark a QUP I2C RX transfer completed */
> +#define QUP_BLK_EVENT_RX_DONE(QUP_BLK_EVENT_TX_DONE | \
> + QUP_BLK_EVENT_RX_IRQ_DONE | \
> + QUP_BLK_EVENT_RX_DATA_DONE)
> +
> +/*
> + * count: no of blocks
> + * pos: current block number
> + * tx_tag_len: tx tag length for current block
> + * rx_tag_len: rx tag length for current block
> + * data_len: remaining data length for current message
> + * total_tx_len: total tx length including tag bytes for current QUP transfer
> + * total_rx_len: total rx length including tag bytes for current QUP transfer
> + * tx_fifo_free: number of free bytes in current QUP block write.
> + * fifo_available: number of available bytes in RX FIFO for current
> + * QUP block read
> + * is_tx_blk_mode: whether tx uses block or FIFO mode in case of non BAM
> xfer.
> + * is_rx_blk_mode: whether rx uses block or FIFO mode in case of non BAM
> xfer.
> + * tags: contains tx tag bytes for current QUP transfer
> + */
> struct qup_i2c_block {
> - int count;
> - int pos;
> - int tx_tag_len;
> - int rx_tag_len;
> - int data_len;
> - u8 tags[6];
> + int count;
> + int pos;
> + int tx_tag_len;
> + int rx_tag_len;
> + int data_len;
> + int total_tx_len;
> + int total_rx_len;
> + int tx_fifo_free;
> + int fifo_available;
> + boolis_tx_blk_mode;
> + boolis_rx_blk_mode;
> + u8
Re: [PATCH 11/12] i2c: qup: reorganization of driver code to remove polling for qup v1
Hi Abhishek,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.15 next-20180205]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Abhishek-Sahu/Major-code-reorganization-to-make-all-i2c-transfers-working/20180206-035527
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
i2c/for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-qup.c: In function 'qup_i2c_read_rx_fifo_v1':
>> drivers/i2c/busses/i2c-qup.c:1139:12: warning: 'idx' may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
if ((idx & 1) == 0) {
~^~~~
vim +/idx +1139 drivers/i2c/busses/i2c-qup.c
191424bb Sricharan R 2016-01-19 1130
3a487e36 Abhishek Sahu 2018-02-03 1131 static void
qup_i2c_read_rx_fifo_v1(struct qup_i2c_dev *qup)
10c5a842 Bjorn Andersson 2014-03-13 1132 {
3a487e36 Abhishek Sahu 2018-02-03 1133 struct qup_i2c_block *blk =
&qup->blk;
3a487e36 Abhishek Sahu 2018-02-03 1134 struct i2c_msg *msg = qup->msg;
10c5a842 Bjorn Andersson 2014-03-13 1135 u32 val = 0;
10c5a842 Bjorn Andersson 2014-03-13 1136 int idx;
10c5a842 Bjorn Andersson 2014-03-13 1137
3a487e36 Abhishek Sahu 2018-02-03 1138 while (blk->fifo_available &&
qup->pos < msg->len) {
10c5a842 Bjorn Andersson 2014-03-13 @1139 if ((idx & 1) == 0) {
10c5a842 Bjorn Andersson 2014-03-13 1140 /* Reading 2
words at time */
10c5a842 Bjorn Andersson 2014-03-13 1141 val =
readl(qup->base + QUP_IN_FIFO_BASE);
10c5a842 Bjorn Andersson 2014-03-13 1142
msg->buf[qup->pos++] = val & 0xFF;
10c5a842 Bjorn Andersson 2014-03-13 1143 } else {
10c5a842 Bjorn Andersson 2014-03-13 1144
msg->buf[qup->pos++] = val >> QUP_MSW_SHIFT;
10c5a842 Bjorn Andersson 2014-03-13 1145 }
3a487e36 Abhishek Sahu 2018-02-03 1146 idx++;
3a487e36 Abhishek Sahu 2018-02-03 1147 blk->fifo_available--;
10c5a842 Bjorn Andersson 2014-03-13 1148 }
c4f0c5fb Sricharan R 2016-01-19 1149
3a487e36 Abhishek Sahu 2018-02-03 1150 if (qup->pos == msg->len)
3a487e36 Abhishek Sahu 2018-02-03 1151 qup->cur_blk_events |=
QUP_BLK_EVENT_RX_DATA_DONE;
10c5a842 Bjorn Andersson 2014-03-13 1152 }
10c5a842 Bjorn Andersson 2014-03-13 1153
:: The code at line 1139 was first introduced by commit
:: 10c5a8425968f8a43b7039ce6261367fc992289f i2c: qup: New bus driver for
the Qualcomm QUP I2C controller
:: TO: Bjorn Andersson
:: CC: Wolfram Sang
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
