[PATCH]i2c: hisi: Clear the interrupt status and optimize writing limitation

[PATCH]i2c: hisi: Clear the interrupt status and optimize writing limitation

[Devyn Liu]

Devyn Liu (2):
  i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth
    - 1
  i2c: hisi: Add clearing tx aempty interrupt operation

 drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.30.0

[PATCH 1/2] i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth - 1

[Devyn Liu]

The driver finishs a write cycle by read the fifo tx full status
or write limit decrease to 0. The driver starts to write data to
the FIFO after the I2C FIFO almost empty interrupt is reported.
The threshold for FIFO empty interrupt is that the amount of data in
the FIFO is less than or equal to 1.
Reduce write maxwrite to the fifo depth - 1. Limiting the number of
data to be written at a time to remaining fifo capacity.

Signed-off-by: Devyn Liu <liudingyuan@huawei.com>
---
 drivers/i2c/busses/i2c-hisi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
index dfad5bad5075..82a0c739aae4 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -266,7 +266,7 @@ static int hisi_i2c_read_rx_fifo(struct hisi_i2c_controller *ctlr)
 
 static void hisi_i2c_xfer_msg(struct hisi_i2c_controller *ctlr)
 {
-	int max_write = HISI_I2C_TX_FIFO_DEPTH;
+	int max_write = HISI_I2C_TX_FIFO_DEPTH - 1;
 	bool need_restart = false, last_msg;
 	struct i2c_msg *cur_msg;
 	u32 cmd, fifo_state;
-- 
2.30.0

[PATCH 2/2] i2c: hisi: Add clearing tx aempty interrupt operation

[Devyn Liu]

The driver receives the tx fifo almost empty(aempty) interrupt and
reads the tx_aempty_int_mstat to start a round of data transfer.
The operation of clearing the TX aempty interrupt after completing
a write cycle is added to ensure that the FIFO is truly at almost
empty status when an aempty interrupt is received.
The threshold for fifo almost empty interrupt is defined as 1.

Signed-off-by: Devyn Liu <liudingyuan@huawei.com>
---
 drivers/i2c/busses/i2c-hisi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
index 82a0c739aae4..08f6f97722a8 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -57,6 +57,8 @@
 #define   HISI_I2C_FS_SPK_LEN_CNT	GENMASK(7, 0)
 #define HISI_I2C_HS_SPK_LEN		0x003c
 #define   HISI_I2C_HS_SPK_LEN_CNT	GENMASK(7, 0)
+#define HISI_I2C_TX_INT_CLR		0x0040
+#define   HISI_I2C_TX_AEMPTY_INT		BIT(0)
 #define HISI_I2C_INT_MSTAT		0x0044
 #define HISI_I2C_INT_CLR		0x0048
 #define HISI_I2C_INT_MASK		0x004C
@@ -124,6 +126,11 @@ static void hisi_i2c_clear_int(struct hisi_i2c_controller *ctlr, u32 mask)
 	writel_relaxed(mask, ctlr->iobase + HISI_I2C_INT_CLR);
 }
 
+static void hisi_i2c_clear_tx_int(struct hisi_i2c_controller *ctlr, u32 mask)
+{
+	writel_relaxed(mask, ctlr->iobase + HISI_I2C_TX_INT_CLR);
+}
+
 static void hisi_i2c_handle_errors(struct hisi_i2c_controller *ctlr)
 {
 	u32 int_err = ctlr->xfer_err, reg;
@@ -168,6 +175,7 @@ static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
 	writel(reg, ctlr->iobase + HISI_I2C_FIFO_CTRL);
 
 	hisi_i2c_clear_int(ctlr, HISI_I2C_INT_ALL);
+	hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
 	hisi_i2c_enable_int(ctlr, HISI_I2C_INT_ALL);
 
 	return 0;
@@ -323,6 +331,8 @@ static void hisi_i2c_xfer_msg(struct hisi_i2c_controller *ctlr)
 	 */
 	if (ctlr->msg_tx_idx == ctlr->msg_num)
 		hisi_i2c_disable_int(ctlr, HISI_I2C_INT_TX_EMPTY);
+
+	hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
 }
 
 static irqreturn_t hisi_i2c_irq(int irq, void *context)
@@ -363,6 +373,7 @@ static irqreturn_t hisi_i2c_irq(int irq, void *context)
 	if (int_stat & HISI_I2C_INT_TRANS_CPLT) {
 		hisi_i2c_disable_int(ctlr, HISI_I2C_INT_ALL);
 		hisi_i2c_clear_int(ctlr, HISI_I2C_INT_ALL);
+		hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
 		complete(ctlr->completion);
 	}
 
-- 
2.30.0

Re: [PATCH]i2c: hisi: Clear the interrupt status and optimize writing limitation

[Andi Shyti]

On Tue, Jan 23, 2024 at 04:02:20PM +0800, Devyn Liu wrote:
> Devyn Liu (2):
>   i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth
>     - 1
>   i2c: hisi: Add clearing tx aempty interrupt operation

The series looks good to me.

Yicong, any thought?

Thanks,
Andi

> 
>  drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> -- 
> 2.30.0
> 

Re: [PATCH 1/2] i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth - 1

[Yicong Yang]

On 2024/1/23 16:02, Devyn Liu wrote:
> The driver finishs a write cycle by read the fifo tx full status
> or write limit decrease to 0. The driver starts to write data to
> the FIFO after the I2C FIFO almost empty interrupt is reported.
> The threshold for FIFO empty interrupt is that the amount of data in
> the FIFO is less than or equal to 1.
> Reduce write maxwrite to the fifo depth - 1. Limiting the number of
> data to be written at a time to remaining fifo capacity.
> 
> Signed-off-by: Devyn Liu <liudingyuan@huawei.com>

Looks good to me. Just one nit below. so,

Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>

> ---
>  drivers/i2c/busses/i2c-hisi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
> index dfad5bad5075..82a0c739aae4 100644
> --- a/drivers/i2c/busses/i2c-hisi.c
> +++ b/drivers/i2c/busses/i2c-hisi.c
> @@ -266,7 +266,7 @@ static int hisi_i2c_read_rx_fifo(struct hisi_i2c_controller *ctlr)
>  
>  static void hisi_i2c_xfer_msg(struct hisi_i2c_controller *ctlr)
>  {
> -	int max_write = HISI_I2C_TX_FIFO_DEPTH;
> +	int max_write = HISI_I2C_TX_FIFO_DEPTH - 1;

Would it be more readable to use HISI_I2C_TX_F_AE_THRESH instead of '1'?

>  	bool need_restart = false, last_msg;
>  	struct i2c_msg *cur_msg;
>  	u32 cmd, fifo_state;
> 

Re: [PATCH 2/2] i2c: hisi: Add clearing tx aempty interrupt operation

[Yicong Yang]

On 2024/1/23 16:02, Devyn Liu wrote:
> The driver receives the tx fifo almost empty(aempty) interrupt and
> reads the tx_aempty_int_mstat to start a round of data transfer.
> The operation of clearing the TX aempty interrupt after completing
> a write cycle is added to ensure that the FIFO is truly at almost
> empty status when an aempty interrupt is received.
> The threshold for fifo almost empty interrupt is defined as 1.
> 
> Signed-off-by: Devyn Liu <liudingyuan@huawei.com>

Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>

> ---
>  drivers/i2c/busses/i2c-hisi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
> index 82a0c739aae4..08f6f97722a8 100644
> --- a/drivers/i2c/busses/i2c-hisi.c
> +++ b/drivers/i2c/busses/i2c-hisi.c
> @@ -57,6 +57,8 @@
>  #define   HISI_I2C_FS_SPK_LEN_CNT	GENMASK(7, 0)
>  #define HISI_I2C_HS_SPK_LEN		0x003c
>  #define   HISI_I2C_HS_SPK_LEN_CNT	GENMASK(7, 0)
> +#define HISI_I2C_TX_INT_CLR		0x0040
> +#define   HISI_I2C_TX_AEMPTY_INT		BIT(0)
>  #define HISI_I2C_INT_MSTAT		0x0044
>  #define HISI_I2C_INT_CLR		0x0048
>  #define HISI_I2C_INT_MASK		0x004C
> @@ -124,6 +126,11 @@ static void hisi_i2c_clear_int(struct hisi_i2c_controller *ctlr, u32 mask)
>  	writel_relaxed(mask, ctlr->iobase + HISI_I2C_INT_CLR);
>  }
>  
> +static void hisi_i2c_clear_tx_int(struct hisi_i2c_controller *ctlr, u32 mask)
> +{
> +	writel_relaxed(mask, ctlr->iobase + HISI_I2C_TX_INT_CLR);
> +}
> +
>  static void hisi_i2c_handle_errors(struct hisi_i2c_controller *ctlr)
>  {
>  	u32 int_err = ctlr->xfer_err, reg;
> @@ -168,6 +175,7 @@ static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
>  	writel(reg, ctlr->iobase + HISI_I2C_FIFO_CTRL);
>  
>  	hisi_i2c_clear_int(ctlr, HISI_I2C_INT_ALL);
> +	hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
>  	hisi_i2c_enable_int(ctlr, HISI_I2C_INT_ALL);
>  
>  	return 0;
> @@ -323,6 +331,8 @@ static void hisi_i2c_xfer_msg(struct hisi_i2c_controller *ctlr)
>  	 */
>  	if (ctlr->msg_tx_idx == ctlr->msg_num)
>  		hisi_i2c_disable_int(ctlr, HISI_I2C_INT_TX_EMPTY);
> +
> +	hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
>  }
>  
>  static irqreturn_t hisi_i2c_irq(int irq, void *context)
> @@ -363,6 +373,7 @@ static irqreturn_t hisi_i2c_irq(int irq, void *context)
>  	if (int_stat & HISI_I2C_INT_TRANS_CPLT) {
>  		hisi_i2c_disable_int(ctlr, HISI_I2C_INT_ALL);
>  		hisi_i2c_clear_int(ctlr, HISI_I2C_INT_ALL);
> +		hisi_i2c_clear_tx_int(ctlr, HISI_I2C_TX_AEMPTY_INT);
>  		complete(ctlr->completion);
>  	}
>  
> 

Re: [PATCH]i2c: hisi: Clear the interrupt status and optimize writing limitation

[Yicong Yang]

On 2024/2/1 6:47, Andi Shyti wrote:
> On Tue, Jan 23, 2024 at 04:02:20PM +0800, Devyn Liu wrote:
>> Devyn Liu (2):
>>   i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth
>>     - 1
>>   i2c: hisi: Add clearing tx aempty interrupt operation
> 
> The series looks good to me.
> 
> Yicong, any thought?
> 

Sorry for finding this late. Generally looks good to me as well and just replied.

> Thanks,
> Andi
> 
>>
>>  drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> -- 
>> 2.30.0
>>
> .
> 

Re: [PATCH]i2c: hisi: Clear the interrupt status and optimize writing limitation

[Andi Shyti]

On Thu, Feb 01, 2024 at 10:12:09AM +0800, Yicong Yang wrote:
> On 2024/2/1 6:47, Andi Shyti wrote:
> > On Tue, Jan 23, 2024 at 04:02:20PM +0800, Devyn Liu wrote:
> >> Devyn Liu (2):
> >>   i2c: hisi: Optimized the value setting of maxwrite limit to fifo depth
> >>     - 1
> >>   i2c: hisi: Add clearing tx aempty interrupt operation
> > 
> > The series looks good to me.
> > 
> > Yicong, any thought?
> > 
> 
> Sorry for finding this late. Generally looks good to me as well and just replied.

Thanks!

> > Thanks,
> > Andi
> > 
> >>
> >>  drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> -- 
> >> 2.30.0
> >>
> > .
> >