[PATCH 0/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[PATCH 0/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[Pei Yue Ho]

Enable driver to fetch the optional parameter (fifo-depth)
from device tree. If the parameter is not found in the device
tree, it will use the default value declared in the driver.

Pei Yue Ho (2):
  dt-bindings: i2c: i2c-cdns.txt: Add description for an optional
    parameter, fifo-depth
  i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

 doc/device-tree-bindings/i2c/i2c-cdns.txt |  4 ++++
 drivers/i2c/i2c-cdns.c                    | 26 ++++++++++++++---------
 2 files changed, 20 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH 1/2] dt-bindings: i2c: i2c-cdns.txt: Add description for an optional parameter, fifo-depth

[Pei Yue Ho]

Add description for fifo-depth parameter that can be used
in the device tree.

Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
---
 doc/device-tree-bindings/i2c/i2c-cdns.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/device-tree-bindings/i2c/i2c-cdns.txt b/doc/device-tree-bindings/i2c/i2c-cdns.txt
index 202e0b7626..eaff34a555 100644
--- a/doc/device-tree-bindings/i2c/i2c-cdns.txt
+++ b/doc/device-tree-bindings/i2c/i2c-cdns.txt
@@ -9,6 +9,9 @@ Required properties:
 - interrupt-parent	: Must be core interrupt controller
 - clocks		: Clock phandles (see clock bindings for details).
 
+Optional properties:
+- fifo-depth		: To specify the FIFO depth of the controller.
+
 Example:
 	i2c0: i2c@e0004000 {
 		compatible = "cdns,i2c-r1p10";
@@ -16,5 +19,6 @@ Example:
 		clocks = <&clkc 38>;
 		interrupts = <0 25 4>;
 		interrupt-parent = <&intc>;
+		fifo-depth = <32>;
 		status = "disabled";
 	};
-- 
2.25.1

[PATCH 2/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[Pei Yue Ho]

Enable driver to fetch the optional parameter (fifo-depth)
from device tree. If the parameter is not found in the device
tree, it will use the default value declared in the driver.

Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
---
 drivers/i2c/i2c-cdns.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/i2c-cdns.c b/drivers/i2c/i2c-cdns.c
index 0da9f6f35a..65a0c76a2a 100644
--- a/drivers/i2c/i2c-cdns.c
+++ b/drivers/i2c/i2c-cdns.c
@@ -78,7 +78,7 @@ struct cdns_i2c_regs {
 					CDNS_I2C_INTERRUPT_RXUNF | \
 					CDNS_I2C_INTERRUPT_ARBLOST)
 
-#define CDNS_I2C_FIFO_DEPTH		16
+#define CDNS_I2C_FIFO_DEPTH_DEFAULT	16
 #define CDNS_I2C_TRANSFER_SIZE_MAX	255 /* Controller transfer limit */
 #define CDNS_I2C_TRANSFER_SIZE		(CDNS_I2C_TRANSFER_SIZE_MAX - 3)
 
@@ -135,6 +135,7 @@ struct i2c_cdns_bus {
 
 	int hold_flag;
 	u32 quirks;
+	u32 fifo_depth;
 };
 
 struct cdns_i2c_platform_data {
@@ -277,7 +278,7 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
 			writel(addr, &regs->address);
 			start = 0;
 		}
-		if (len && readl(&regs->transfer_size) == CDNS_I2C_FIFO_DEPTH) {
+		if (len && readl(&regs->transfer_size) == i2c_bus->fifo_depth) {
 			ret = cdns_i2c_wait(regs, CDNS_I2C_INTERRUPT_COMP |
 					    CDNS_I2C_INTERRUPT_ARBLOST);
 			if (ret & CDNS_I2C_INTERRUPT_ARBLOST)
@@ -310,9 +311,10 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
 	return 0;
 }
 
-static inline bool cdns_is_hold_quirk(int hold_quirk, int curr_recv_count)
+static inline bool cdns_is_hold_quirk(struct i2c_cdns_bus *i2c_bus, int hold_quirk,
+				      int curr_recv_count)
 {
-	return hold_quirk && (curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1);
+	return hold_quirk && (curr_recv_count == i2c_bus->fifo_depth + 1);
 }
 
 static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
@@ -327,7 +329,7 @@ static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
 	curr_recv_count = recv_count;
 
 	/* Check for the message size against the FIFO depth */
-	if (recv_count > CDNS_I2C_FIFO_DEPTH)
+	if (recv_count > i2c_bus->fifo_depth)
 		setbits_le32(&regs->control, CDNS_I2C_CONTROL_HOLD);
 
 	setbits_le32(&regs->control, CDNS_I2C_CONTROL_CLR_FIFO |
@@ -349,7 +351,7 @@ static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
 
 	while (recv_count && !is_arbitration_lost(regs)) {
 		while (readl(&regs->status) & CDNS_I2C_STATUS_RXDV) {
-			if (recv_count < CDNS_I2C_FIFO_DEPTH &&
+			if (recv_count < i2c_bus->fifo_depth &&
 			    !i2c_bus->hold_flag) {
 				clrbits_le32(&regs->control,
 					     CDNS_I2C_CONTROL_HOLD);
@@ -365,20 +367,20 @@ static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
 		if (cdns_is_hold_quirk(hold_quirk, curr_recv_count)) {
 			/* wait while fifo is full */
 			while (readl(&regs->transfer_size) !=
-				     (curr_recv_count - CDNS_I2C_FIFO_DEPTH))
+				     (curr_recv_count - i2c_bus->fifo_depth))
 				;
 			/*
 			 * Check number of bytes to be received against maximum
 			 * transfer size and update register accordingly.
 			 */
-			if ((recv_count - CDNS_I2C_FIFO_DEPTH) >
+			if ((recv_count - i2c_bus->fifo_depth) >
 			    CDNS_I2C_TRANSFER_SIZE) {
 				writel(CDNS_I2C_TRANSFER_SIZE,
 				       &regs->transfer_size);
 				curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
-					CDNS_I2C_FIFO_DEPTH;
+					i2c_bus->fifo_depth;
 			} else {
-				writel(recv_count - CDNS_I2C_FIFO_DEPTH,
+				writel(recv_count - i2c_bus->fifo_depth,
 				       &regs->transfer_size);
 				curr_recv_count = recv_count;
 			}
@@ -496,6 +498,10 @@ static int cdns_i2c_of_to_plat(struct udevice *dev)
 		return ret;
 	}
 
+	/* Update FIFO depth based on device tree entry */
+	i2c_bus->fifo_depth = dev_read_u32_default(dev, "fifo-depth",
+						   CDNS_I2C_FIFO_DEPTH_DEFAULT);
+
 	return 0;
 }
 
-- 
2.25.1

RE: [PATCH 0/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[WeiLiang Lim]

Anyone can help review this patch please?

> -----Original Message-----
> From: PeiYue Ho <peiyue.ho@starfivetech.com>
> Sent: Wednesday, January 4, 2023 3:20 PM
> To: u-boot@lists.denx.de
> Cc: PeiYue Ho <peiyue.ho@starfivetech.com>; EngLee Teh
> <englee.teh@starfivetech.com>; WeiLiang Lim <weiliang.lim@starfivetech.com>
> Subject: [PATCH 0/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from
> device tree
> 
> Enable driver to fetch the optional parameter (fifo-depth) from device tree. If
> the parameter is not found in the device tree, it will use the default value
> declared in the driver.
> 
> Pei Yue Ho (2):
>   dt-bindings: i2c: i2c-cdns.txt: Add description for an optional
>     parameter, fifo-depth
>   i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree
> 
>  doc/device-tree-bindings/i2c/i2c-cdns.txt |  4 ++++
>  drivers/i2c/i2c-cdns.c                    | 26 ++++++++++++++---------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> --
> 2.25.1

Re: [PATCH 1/2] dt-bindings: i2c: i2c-cdns.txt: Add description for an optional parameter, fifo-depth

[Heiko Schocher]

Hello Pei Yue Ho

On 04.01.23 08:20, Pei Yue Ho wrote:
> Add description for fifo-depth parameter that can be used
> in the device tree.
> 
> Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
> Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
> Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
> ---
>  doc/device-tree-bindings/i2c/i2c-cdns.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Heiko Schocher <hs@denx.de>

Do you plan to submit this to linux too?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH 2/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[Heiko Schocher]

Hello Pei Yue Ho,

On 04.01.23 08:20, Pei Yue Ho wrote:
> Enable driver to fetch the optional parameter (fifo-depth)
> from device tree. If the parameter is not found in the device
> tree, it will use the default value declared in the driver.
> 
> Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
> Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
> Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
> ---
>  drivers/i2c/i2c-cdns.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH 2/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[Heiko Schocher]

Hello Pei Yue Ho,

On 13.02.23 06:47, Heiko Schocher wrote:
> Hello Pei Yue Ho,
> 
> On 04.01.23 08:20, Pei Yue Ho wrote:
>> Enable driver to fetch the optional parameter (fifo-depth)
>> from device tree. If the parameter is not found in the device
>> tree, it will use the default value declared in the driver.
>>
>> Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
>> Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
>> Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
>> ---
>>  drivers/i2c/i2c-cdns.c | 26 ++++++++++++++++----------
>>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> Reviewed-by: Heiko Schocher <hs@denx.de>

I am sorry, your patch drops error in gitlab Ci, see:

https://dev.azure.com/hs0298/110c3e42-44d5-4db4-9bd5-8a8bbead15f3/_apis/build/builds/100/logs/310

"""
2023-02-13T06:30:04.2539867Z        arm:  +   xilinx_zynq_virt
2023-02-13T06:30:04.2540416Z +drivers/i2c/i2c-cdns.c: In function 'cdns_i2c_read_data':
2023-02-13T06:30:04.2541494Z +drivers/i2c/i2c-cdns.c:363:48: error: passing argument 1 of
'cdns_is_hold_quirk' makes pointer from integer without a cast [-Werror=int-conversion]
2023-02-13T06:30:04.2542415Z +  363 |                         if (cdns_is_hold_quirk(hold_quirk,
curr_recv_count))
2023-02-13T06:30:04.2542799Z +      |                                                ^~~~~~~~~~
2023-02-13T06:30:04.2543066Z +      |                                                |
2023-02-13T06:30:04.2579584Z +      |                                                int
2023-02-13T06:30:04.2580260Z +drivers/i2c/i2c-cdns.c:314:60: note: expected 'struct i2c_cdns_bus *'
but argument is of type 'int'
2023-02-13T06:30:04.2580754Z +  314 | static inline bool cdns_is_hold_quirk(struct i2c_cdns_bus
*i2c_bus, int hold_quirk,
2023-02-13T06:30:04.2581135Z +      |                                       ~~~~~~~~~~~~~~~~~~~~~^~~~~~~
2023-02-13T06:30:04.2581604Z +drivers/i2c/i2c-cdns.c:363:29: error: too few arguments to function
'cdns_is_hold_quirk'
2023-02-13T06:30:04.2581973Z +      |                             ^~~~~~~~~~~~~~~~~~
2023-02-13T06:30:04.2582352Z +drivers/i2c/i2c-cdns.c:314:20: note: declared here
2023-02-13T06:30:04.2582649Z +      |                    ^~~~~~~~~~~~~~~~~~
2023-02-13T06:30:04.2583415Z +drivers/i2c/i2c-cdns.c:367:40: error: passing argument 1 of
'cdns_is_hold_quirk' makes pointer from integer without a cast [-Werror=int-conversion]
2023-02-13T06:30:04.2583944Z +  367 |                 if (cdns_is_hold_quirk(hold_quirk,
curr_recv_count)) {
2023-02-13T06:30:04.2584262Z +      |                                        ^~~~~~~~~~
2023-02-13T06:30:04.2584510Z +      |                                        |
2023-02-13T06:30:04.2584743Z +      |                                        int
2023-02-13T06:30:04.2585511Z +drivers/i2c/i2c-cdns.c:367:21: error: too few arguments to function
'cdns_is_hold_quirk'
2023-02-13T06:30:04.2585984Z +      |                     ^~~~~~~~~~~~~~~~~~
2023-02-13T06:30:04.2586267Z +cc1: all warnings being treated as errors
2023-02-13T06:30:04.2586703Z +make[3]: *** [scripts/Makefile.build:257: drivers/i2c/i2c-cdns.o] Error 1
2023-02-13T06:30:04.2587106Z +make[2]: *** [scripts/Makefile.build:397: drivers/i2c] Error 2
2023-02-13T06:30:04.2587457Z +make[1]: *** [Makefile:1845: drivers] Error 2
2023-02-13T06:30:04.2587832Z +make: *** [Makefile:177: sub-make] Error 2
2023-02-13T06:30:04.2587979Z
2023-02-13T06:30:04.2588213Z     0    0    1 /1              xilinx_zynq_virt
"""

Please fix!

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

RE: [PATCH 1/2] dt-bindings: i2c: i2c-cdns.txt: Add description for an optional parameter, fifo-depth

[PeiYue Ho]

> -----Original Message-----
> From: Heiko Schocher <hs@denx.de>
> Sent: Monday, February 13, 2023 1:44 PM
> To: PeiYue Ho <peiyue.ho@starfivetech.com>; u-boot@lists.denx.de
> Cc: EngLee Teh <englee.teh@starfivetech.com>; WeiLiang Lim
> <weiliang.lim@starfivetech.com>
> Subject: Re: [PATCH 1/2] dt-bindings: i2c: i2c-cdns.txt: Add description for an
> optional parameter, fifo-depth
> 
> Hello Pei Yue Ho
> 
> On 04.01.23 08:20, Pei Yue Ho wrote:
> > Add description for fifo-depth parameter that can be used in the
> > device tree.
> >
> > Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
> > Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
> > Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
> > ---
> >  doc/device-tree-bindings/i2c/i2c-cdns.txt | 4 ++++
> >  1 file changed, 4 insertions(+)
> 
> Reviewed-by: Heiko Schocher <hs@denx.de>
> 
> Do you plan to submit this to linux too?

Yes, we do plan to submit it to linux in the future
> 
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH 1/2] dt-bindings: i2c: i2c-cdns.txt: Add description for an optional parameter, fifo-depth

[Heiko Schocher]

Hello Pei Yue Ho,

On 04.01.23 08:20, Pei Yue Ho wrote:
> Add description for fifo-depth parameter that can be used
> in the device tree.
> 
> Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
> Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
> Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
>  doc/device-tree-bindings/i2c/i2c-cdns.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Applied to u-boot-i2c.git master

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH 2/2] i2c: i2c-cdns.c: Update driver to read fifo-depth from device tree

[Heiko Schocher]

Hello Pei Yue Ho,

On 04.01.23 08:20, Pei Yue Ho wrote:
> Enable driver to fetch the optional parameter (fifo-depth)
> from device tree. If the parameter is not found in the device
> tree, it will use the default value declared in the driver.
> 
> Signed-off-by: Pei Yue Ho <peiyue.ho@starfivetech.com>
> Reviewed-by: Wei Liang Lim <weiliang.lim@starfivetech.com>
> Reviewed-by: Eng Lee Teh <englee.teh@starfivetech.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
>  drivers/i2c/i2c-cdns.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)


Applied to u-boot-i2c.git master

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de