1. Patchew
  2. linux
  3. View series

[PATCH v4] i2c: busses: Use min() to improve code

Qianfeng Rong posted 1 patch 3 months ago 
drivers/i2c/busses/i2c-st.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
[PATCH v4] i2c: busses: Use min() to improve code
Posted by Qianfeng Rong 3 months ago 
Use min() to reduce the code and improve its readability.

The type of the max parameter in the st_i2c_rd_fill_tx_fifo()
was changed from int to u32, because the max parameter passed
in is always greater than 0.

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3-> v4:
- Modified code alignment.

v2-> v3:
- added the <linux/minmax.h> as suggested by Jonathan.
- used i = min(...) as the loop initializer according
  to Jonathan.

v1-> v2:
- Change the max parameter type in st_i2c_rd_fill_tx_fifo()
  from int to u32
---
 drivers/i2c/busses/i2c-st.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
index 750fff3d2389..c86380fb05fb 100644
--- a/drivers/i2c/busses/i2c-st.c
+++ b/drivers/i2c/busses/i2c-st.c
@@ -19,6 +19,7 @@
 #include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/minmax.h>
 
 /* SSC registers */
 #define SSC_BRG				0x000
@@ -422,12 +423,8 @@ static void st_i2c_wr_fill_tx_fifo(struct st_i2c_dev *i2c_dev)
 	tx_fstat = readl_relaxed(i2c_dev->base + SSC_TX_FSTAT);
 	tx_fstat &= SSC_TX_FSTAT_STATUS;
 
-	if (c->count < (SSC_TXFIFO_SIZE - tx_fstat))
-		i = c->count;
-	else
-		i = SSC_TXFIFO_SIZE - tx_fstat;
-
-	for (; i > 0; i--, c->count--, c->buf++)
+	for (i = min(c->count, SSC_TXFIFO_SIZE - tx_fstat);
+	     i > 0; i--, c->count--, c->buf++)
 		st_i2c_write_tx_fifo(i2c_dev, *c->buf);
 }
 
@@ -439,7 +436,7 @@ static void st_i2c_wr_fill_tx_fifo(struct st_i2c_dev *i2c_dev)
  * This functions fills the Tx FIFO with fixed pattern when
  * in read mode to trigger clock.
  */
-static void st_i2c_rd_fill_tx_fifo(struct st_i2c_dev *i2c_dev, int max)
+static void st_i2c_rd_fill_tx_fifo(struct st_i2c_dev *i2c_dev, u32 max)
 {
 	struct st_i2c_client *c = &i2c_dev->client;
 	u32 tx_fstat, sta;
@@ -452,12 +449,8 @@ static void st_i2c_rd_fill_tx_fifo(struct st_i2c_dev *i2c_dev, int max)
 	tx_fstat = readl_relaxed(i2c_dev->base + SSC_TX_FSTAT);
 	tx_fstat &= SSC_TX_FSTAT_STATUS;
 
-	if (max < (SSC_TXFIFO_SIZE - tx_fstat))
-		i = max;
-	else
-		i = SSC_TXFIFO_SIZE - tx_fstat;
-
-	for (; i > 0; i--, c->xfered++)
+	for (i = min(max, SSC_TXFIFO_SIZE - tx_fstat);
+	     i > 0; i--, c->xfered++)
 		st_i2c_write_tx_fifo(i2c_dev, 0xff);
 }
 
-- 
2.34.1
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Andy Shevchenko 2 months, 3 weeks ago 
On Wed, Jul 09, 2025 at 12:23:46PM +0800, Qianfeng Rong wrote:
> Use min() to reduce the code and improve its readability.
> 
> The type of the max parameter in the st_i2c_rd_fill_tx_fifo()
> was changed from int to u32, because the max parameter passed
> in is always greater than 0.

...

>  #include <linux/of.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
> +#include <linux/minmax.h>

Do not blindly add a new inclusion to the end of the list. The list as you may
notice even on this small context suggests that it's ordered. Please, keep
things in order.

Andi, if you don't mind, please fix this.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Qianfeng Rong 2 months, 3 weeks ago 
>>   #include <linux/of.h>
>>   #include <linux/pinctrl/consumer.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/minmax.h>
> Do not blindly add a new inclusion to the end of the list. The list as you may
> notice even on this small context suggests that it's ordered. Please, keep
> things in order.
Thanks for your reminder. Best regards, Qianfeng
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Andi Shyti 2 months, 3 weeks ago 
Hi,

On Fri, Jul 11, 2025 at 09:57:24PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 09, 2025 at 12:23:46PM +0800, Qianfeng Rong wrote:
> > Use min() to reduce the code and improve its readability.
> > 
> > The type of the max parameter in the st_i2c_rd_fill_tx_fifo()
> > was changed from int to u32, because the max parameter passed
> > in is always greater than 0.
> 
> ...
> 
> >  #include <linux/of.h>
> >  #include <linux/pinctrl/consumer.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/minmax.h>
> 
> Do not blindly add a new inclusion to the end of the list. The list as you may
> notice even on this small context suggests that it's ordered. Please, keep
> things in order.
> 
> Andi, if you don't mind, please fix this.

Yes, I actually had this fixed but I had forgotten to push. It's
updated now.

Andi
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Andi Shyti 2 months, 4 weeks ago 
Hi Qianfeng,

On Wed, Jul 09, 2025 at 12:23:46PM +0800, Qianfeng Rong wrote:
> Use min() to reduce the code and improve its readability.
> 
> The type of the max parameter in the st_i2c_rd_fill_tx_fifo()
> was changed from int to u32, because the max parameter passed
> in is always greater than 0.
> 
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The 'Suggested-by' tag implies that the patch was suggested by
Jonathan, which is not the case here. Jonathan reviewed the patch
and proposed improvements, but the patch itself comes from you.

For this reason, I will remove the 'Suggested-by' tag. If
Jonathan wants to add his Acked-by or Reviewed-by, I will gladly
include it.

Thanks, Jonathan, for the helpful reviews, and thanks to Qianfeng
for promptly following up on the feedback.

Merged to i2c/i2c-host.

Thanks,
Andi
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Qianfeng Rong 2 months, 3 weeks ago 
>> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
>> Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
>> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> The 'Suggested-by' tag implies that the patch was suggested by
> Jonathan, which is not the case here. Jonathan reviewed the patch
> and proposed improvements, but the patch itself comes from you.
>
Thank you, Andi, for your patient explanation.

Best regards,
Qianfeng
Re: [PATCH v4] i2c: busses: Use min() to improve code
Posted by Jonathan Cameron 2 months, 3 weeks ago 
On Thu, 10 Jul 2025 22:42:26 +0200
Andi Shyti <andi.shyti@kernel.org> wrote:

> Hi Qianfeng,
> 
> On Wed, Jul 09, 2025 at 12:23:46PM +0800, Qianfeng Rong wrote:
> > Use min() to reduce the code and improve its readability.
> > 
> > The type of the max parameter in the st_i2c_rd_fill_tx_fifo()
> > was changed from int to u32, because the max parameter passed
> > in is always greater than 0.
> > 
> > Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> > Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
> > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> The 'Suggested-by' tag implies that the patch was suggested by
> Jonathan, which is not the case here. Jonathan reviewed the patch
> and proposed improvements, but the patch itself comes from you.
> 
> For this reason, I will remove the 'Suggested-by' tag. If
> Jonathan wants to add his Acked-by or Reviewed-by, I will gladly
> include it.

Trivial enough that I don't care ;)  Was a drive by review at
best.

Thanks for asking though.

J

> 
> Thanks, Jonathan, for the helpful reviews, and thanks to Qianfeng
> for promptly following up on the feedback.
> 
> Merged to i2c/i2c-host.
> 
> Thanks,
> Andi
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.