diff mbox series

[v3] i2c: cadence: Add standard bus recovery support

Message ID 20220728055150.18368-1-shubhrajyoti.datta@xilinx.com
State Accepted
Commit 58b924241d0a23eee8e86dd9e6f5dacd01c82e62
Headers show
Series [v3] i2c: cadence: Add standard bus recovery support | expand

Commit Message

Shubhrajyoti Datta July 28, 2022, 5:51 a.m. UTC
Hook up the standard GPIO/pinctrl-based recovery support.
We are doing the recovery at the beginning on a timeout.

Multiple people have contributed to the series.
Original patch from Cirag and another one from Robert.

Cc: Chirag Parekh <chiragp@xilinx.com>
Cc: Robert Hancock <robert.hancock@calian.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 v2:
Updated the busbusy check on a timeout
v3:
Added pinctrl_get

Did unit testing and probed the scl to see the clock pulses.


 drivers/i2c/busses/i2c-cadence.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Michal Simek July 29, 2022, 10:56 a.m. UTC | #1
On 7/28/22 07:51, Shubhrajyoti Datta wrote:
> Hook up the standard GPIO/pinctrl-based recovery support.
> We are doing the recovery at the beginning on a timeout.
> 
> Multiple people have contributed to the series.
> Original patch from Cirag and another one from Robert.
> 
> Cc: Chirag Parekh <chiragp@xilinx.com>
> Cc: Robert Hancock <robert.hancock@calian.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>   v2:
> Updated the busbusy check on a timeout
> v3:
> Added pinctrl_get
> 
> Did unit testing and probed the scl to see the clock pulses.

Can you please describe testing procedure? What board did you use?
What was the hardware configuration?

Thanks,
Michal
Shubhrajyoti Datta July 29, 2022, 11:05 a.m. UTC | #2
[AMD Official Use Only - General]



> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Friday, July 29, 2022 4:26 PM
> To: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>; linux-
> i2c@vger.kernel.org
> Cc: michal.simek@xilinx.com; git-dev (AMD-Xilinx) <git-dev@amd.com>;
> Chirag Parekh <chiragp@xilinx.com>; Robert Hancock
> <robert.hancock@calian.com>
> Subject: Re: [PATCH v3] i2c: cadence: Add standard bus recovery support
> 
> 
> 
> On 7/28/22 07:51, Shubhrajyoti Datta wrote:
> > Hook up the standard GPIO/pinctrl-based recovery support.
> > We are doing the recovery at the beginning on a timeout.
> >
> > Multiple people have contributed to the series.
> > Original patch from Cirag and another one from Robert.
> >
> > Cc: Chirag Parekh <chiragp@xilinx.com>
> > Cc: Robert Hancock <robert.hancock@calian.com>
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> >   v2:
> > Updated the busbusy check on a timeout
> > v3:
> > Added pinctrl_get
> >
> > Did unit testing and probed the scl to see the clock pulses.
> 
> Can you please describe testing procedure? What board did you use?
> What was the hardware configuration?


Tested on zcu102 .  and then I called the recovery in send  setup . 
Also I had added the prints in the 
To confirm that the core was calling the zynqmp pinctrl to set pin mux. 

The scl  line was Probed. I could see the clock cycles in the scl line.

[   84.286340] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for pin 14
[   84.293779] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for pin 15
[   84.301266] i2c-core: set_scl_gpio_value 164 1
[   84.305807] i2c i2c-0: SCL is set_scl 1 i ia 1
[   84.310331] i2c-core: get_scl_gpio_value 158
[   84.314681] i2c-core: set_scl_gpio_value 164 0
[   84.322519] i2c i2c-0: SCL is set_scl 0
[   84.326439] i2c i2c-0: SCL is set_scl 0 i ia 2
[   84.330965] i2c-core: set_scl_gpio_value 164 1
[   84.335496] i2c i2c-0: SCL is set_scl 1
[   84.339418] i2c i2c-0: SCL is set_scl 1 i ia 3
[   84.343947] i2c-core: get_scl_gpio_value 158
[   84.348302] i2c-core: set_scl_gpio_value 164 0
[   84.356135] i2c i2c-0: SCL is set_scl 0
[   84.360059] i2c i2c-0: SCL is set_scl 0 i ia 4
[   84.364586] i2c-core: set_scl_gpio_value 164 1
[   84.369117] i2c i2c-0: SCL is set_scl 1
[   84.373037] i2c i2c-0: SCL is set_scl 1 i ia 5
[   84.377564] i2c-core: get_scl_gpio_value 158
[   84.381914] i2c-core: set_scl_gpio_value 164 0


> 
> Thanks,
> Michal
Michal Simek July 29, 2022, 11:17 a.m. UTC | #3
On 7/29/22 13:05, Datta, Shubhrajyoti wrote:
> [AMD Official Use Only - General]
> 
> 
> 
>> -----Original Message-----
>> From: Simek, Michal <michal.simek@amd.com>
>> Sent: Friday, July 29, 2022 4:26 PM
>> To: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>; linux-
>> i2c@vger.kernel.org
>> Cc: michal.simek@xilinx.com; git-dev (AMD-Xilinx) <git-dev@amd.com>;
>> Chirag Parekh <chiragp@xilinx.com>; Robert Hancock
>> <robert.hancock@calian.com>
>> Subject: Re: [PATCH v3] i2c: cadence: Add standard bus recovery support
>>
>>
>>
>> On 7/28/22 07:51, Shubhrajyoti Datta wrote:
>>> Hook up the standard GPIO/pinctrl-based recovery support.
>>> We are doing the recovery at the beginning on a timeout.
>>>
>>> Multiple people have contributed to the series.
>>> Original patch from Cirag and another one from Robert.
>>>
>>> Cc: Chirag Parekh <chiragp@xilinx.com>
>>> Cc: Robert Hancock <robert.hancock@calian.com>
>>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>>> ---
>>>    v2:
>>> Updated the busbusy check on a timeout
>>> v3:
>>> Added pinctrl_get
>>>
>>> Did unit testing and probed the scl to see the clock pulses.
>>
>> Can you please describe testing procedure? What board did you use?
>> What was the hardware configuration?
> 
> 
> Tested on zcu102 .  and then I called the recovery in send  setup .
> Also I had added the prints in the
> To confirm that the core was calling the zynqmp pinctrl to set pin mux.
> 
> The scl  line was Probed. I could see the clock cycles in the scl line.
> 
> [   84.286340] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for pin 14
> [   84.293779] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for pin 15
> [   84.301266] i2c-core: set_scl_gpio_value 164 1
> [   84.305807] i2c i2c-0: SCL is set_scl 1 i ia 1
> [   84.310331] i2c-core: get_scl_gpio_value 158
> [   84.314681] i2c-core: set_scl_gpio_value 164 0
> [   84.322519] i2c i2c-0: SCL is set_scl 0
> [   84.326439] i2c i2c-0: SCL is set_scl 0 i ia 2
> [   84.330965] i2c-core: set_scl_gpio_value 164 1
> [   84.335496] i2c i2c-0: SCL is set_scl 1
> [   84.339418] i2c i2c-0: SCL is set_scl 1 i ia 3
> [   84.343947] i2c-core: get_scl_gpio_value 158
> [   84.348302] i2c-core: set_scl_gpio_value 164 0
> [   84.356135] i2c i2c-0: SCL is set_scl 0
> [   84.360059] i2c i2c-0: SCL is set_scl 0 i ia 4
> [   84.364586] i2c-core: set_scl_gpio_value 164 1
> [   84.369117] i2c i2c-0: SCL is set_scl 1
> [   84.373037] i2c i2c-0: SCL is set_scl 1 i ia 5
> [   84.377564] i2c-core: get_scl_gpio_value 158
> [   84.381914] i2c-core: set_scl_gpio_value 164 0

ok. Thanks.

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal
Michal Simek Sept. 27, 2022, 7:06 a.m. UTC | #4
Hi Wolfram,

On 7/29/22 13:17, Michal Simek wrote:
> 
> 
> On 7/29/22 13:05, Datta, Shubhrajyoti wrote:
>> [AMD Official Use Only - General]
>>
>>
>>
>>> -----Original Message-----
>>> From: Simek, Michal <michal.simek@amd.com>
>>> Sent: Friday, July 29, 2022 4:26 PM
>>> To: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>; linux-
>>> i2c@vger.kernel.org
>>> Cc: michal.simek@xilinx.com; git-dev (AMD-Xilinx) <git-dev@amd.com>;
>>> Chirag Parekh <chiragp@xilinx.com>; Robert Hancock
>>> <robert.hancock@calian.com>
>>> Subject: Re: [PATCH v3] i2c: cadence: Add standard bus recovery support
>>>
>>>
>>>
>>> On 7/28/22 07:51, Shubhrajyoti Datta wrote:
>>>> Hook up the standard GPIO/pinctrl-based recovery support.
>>>> We are doing the recovery at the beginning on a timeout.
>>>>
>>>> Multiple people have contributed to the series.
>>>> Original patch from Cirag and another one from Robert.
>>>>
>>>> Cc: Chirag Parekh <chiragp@xilinx.com>
>>>> Cc: Robert Hancock <robert.hancock@calian.com>
>>>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>>>> ---
>>>>    v2:
>>>> Updated the busbusy check on a timeout
>>>> v3:
>>>> Added pinctrl_get
>>>>
>>>> Did unit testing and probed the scl to see the clock pulses.
>>>
>>> Can you please describe testing procedure? What board did you use?
>>> What was the hardware configuration?
>>
>>
>> Tested on zcu102 .  and then I called the recovery in send  setup .
>> Also I had added the prints in the
>> To confirm that the core was calling the zynqmp pinctrl to set pin mux.
>>
>> The scl  line was Probed. I could see the clock cycles in the scl line.
>>
>> [   84.286340] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for 
>> pin 14
>> [   84.293779] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: set mux  for 
>> pin 15
>> [   84.301266] i2c-core: set_scl_gpio_value 164 1
>> [   84.305807] i2c i2c-0: SCL is set_scl 1 i ia 1
>> [   84.310331] i2c-core: get_scl_gpio_value 158
>> [   84.314681] i2c-core: set_scl_gpio_value 164 0
>> [   84.322519] i2c i2c-0: SCL is set_scl 0
>> [   84.326439] i2c i2c-0: SCL is set_scl 0 i ia 2
>> [   84.330965] i2c-core: set_scl_gpio_value 164 1
>> [   84.335496] i2c i2c-0: SCL is set_scl 1
>> [   84.339418] i2c i2c-0: SCL is set_scl 1 i ia 3
>> [   84.343947] i2c-core: get_scl_gpio_value 158
>> [   84.348302] i2c-core: set_scl_gpio_value 164 0
>> [   84.356135] i2c i2c-0: SCL is set_scl 0
>> [   84.360059] i2c i2c-0: SCL is set_scl 0 i ia 4
>> [   84.364586] i2c-core: set_scl_gpio_value 164 1
>> [   84.369117] i2c i2c-0: SCL is set_scl 1
>> [   84.373037] i2c i2c-0: SCL is set_scl 1 i ia 5
>> [   84.377564] i2c-core: get_scl_gpio_value 158
>> [   84.381914] i2c-core: set_scl_gpio_value 164 0
> 
> ok. Thanks.
> 
> Acked-by: Michal Simek <michal.simek@amd.com>
> 

Do you see any issue with this version?

Thanks,
Michal
Wolfram Sang Sept. 27, 2022, 8:32 p.m. UTC | #5
On Thu, Jul 28, 2022 at 11:21:50AM +0530, Shubhrajyoti Datta wrote:
> Hook up the standard GPIO/pinctrl-based recovery support.
> We are doing the recovery at the beginning on a timeout.
> 
> Multiple people have contributed to the series.
> Original patch from Cirag and another one from Robert.
> 
> Cc: Chirag Parekh <chiragp@xilinx.com>
> Cc: Robert Hancock <robert.hancock@calian.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Applied to for-next, thanks!
Wolfram Sang Sept. 27, 2022, 8:35 p.m. UTC | #6
> Applied to for-next, thanks!

Unrelated to this patch, but cppcheck found this issue:

drivers/i2c/busses/i2c-cadence.c:1038:33: warning: Condition 'actual_fscl>fscl' is always false [knownConditionTrueFalse]
  current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) :
                                ^
drivers/i2c/busses/i2c-cadence.c:1035:19: note: Assuming that condition 'actual_fscl>fscl' is not redundant
  if (actual_fscl > fscl)
                  ^
drivers/i2c/busses/i2c-cadence.c:1038:33: note: Condition 'actual_fscl>fscl' is always false
  current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) :

I had a glimpse and I think the checker is correct...
Michal Simek Sept. 29, 2022, 7:29 a.m. UTC | #7
Hi Wolfram,

On 9/27/22 22:35, Wolfram Sang wrote:
> 
>> Applied to for-next, thanks!
> 
> Unrelated to this patch, but cppcheck found this issue:
> 
> drivers/i2c/busses/i2c-cadence.c:1038:33: warning: Condition 'actual_fscl>fscl' is always false [knownConditionTrueFalse]
>    current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) :
>                                  ^
> drivers/i2c/busses/i2c-cadence.c:1035:19: note: Assuming that condition 'actual_fscl>fscl' is not redundant
>    if (actual_fscl > fscl)
>                    ^
> drivers/i2c/busses/i2c-cadence.c:1038:33: note: Condition 'actual_fscl>fscl' is always false
>    current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) :
> 
> I had a glimpse and I think the checker is correct...

Are you still using your ninja-check script?
Can you please share your latest version?

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 630cfa4ddd46..c9a10d4297a9 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -10,10 +10,12 @@ 
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 
 /* Register offsets for the I2C device. */
 #define CDNS_I2C_CR_OFFSET		0x00 /* Control Register, RW */
@@ -127,6 +129,8 @@ 
 #define CDNS_I2C_TIMEOUT_MAX	0xFF
 
 #define CDNS_I2C_BROKEN_HOLD_BIT	BIT(0)
+#define CDNS_I2C_POLL_US	100000
+#define CDNS_I2C_TIMEOUT_US	500000
 
 #define cdns_i2c_readreg(offset)       readl_relaxed(id->membase + offset)
 #define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + offset)
@@ -204,6 +208,7 @@  struct cdns_i2c {
 	struct notifier_block clk_rate_change_nb;
 	u32 quirks;
 	u32 ctrl_reg;
+	struct i2c_bus_recovery_info rinfo;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	u16 ctrl_reg_diva_divb;
 	struct i2c_client *slave;
@@ -832,8 +837,14 @@  static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 #endif
 
 	/* Check if the bus is free */
-	if (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) & CDNS_I2C_SR_BA) {
+
+	ret = readl_relaxed_poll_timeout(id->membase + CDNS_I2C_SR_OFFSET,
+					 reg,
+					 !(reg & CDNS_I2C_SR_BA),
+					 CDNS_I2C_POLL_US, CDNS_I2C_TIMEOUT_US);
+	if (ret) {
 		ret = -EAGAIN;
+		i2c_recover_bus(adap);
 		goto out;
 	}
 
@@ -1242,6 +1253,12 @@  static int cdns_i2c_probe(struct platform_device *pdev)
 		id->quirks = data->quirks;
 	}
 
+	id->rinfo.pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(id->rinfo.pinctrl)) {
+		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
+		return PTR_ERR(id->rinfo.pinctrl);
+	}
+
 	id->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r_mem);
 	if (IS_ERR(id->membase))
 		return PTR_ERR(id->membase);
@@ -1258,6 +1275,7 @@  static int cdns_i2c_probe(struct platform_device *pdev)
 	id->adap.retries = 3;		/* Default retry value. */
 	id->adap.algo_data = id;
 	id->adap.dev.parent = &pdev->dev;
+	id->adap.bus_recovery_info = &id->rinfo;
 	init_completion(&id->xfer_done);
 	snprintf(id->adap.name, sizeof(id->adap.name),
 		 "Cadence I2C at %08lx", (unsigned long)r_mem->start);