diff mbox series

[V2,10/18] i2c: imx-lpi2c: add bus recovery feature

Message ID 20210406113306.2633595-11-xiaoning.wang@nxp.com (mailing list archive)
State New, archived
Headers show
Series i2c: imx-lpi2c: New features and bug fixes | expand

Commit Message

Clark Wang April 6, 2021, 11:32 a.m. UTC 
Add bus recovery feature for LPI2C.
Need add gpio pinctrl, scl-gpios and sda-gpios configuration in dts.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
V2 changes:
 - No change. Add dt-bindings in the next patch.
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 83 ++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

Comments

kernel test robot April 7, 2021, 12:34 a.m. UTC | #1 
Hi Clark,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on next-20210406]
[cannot apply to wsa/i2c/for-next robh/for-next v5.12-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Clark-Wang/i2c-imx-lpi2c-New-features-and-bug-fixes/20210406-193539
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: nios2-randconfig-r003-20210406 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5853de64dd40e1cc71f0adede924934aee4c4f0e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Clark-Wang/i2c-imx-lpi2c-New-features-and-bug-fixes/20210406-193539
        git checkout 5853de64dd40e1cc71f0adede924934aee4c4f0e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-imx-lpi2c.c: In function 'lpi2c_imx_init_recovery_info':
>> drivers/i2c/busses/i2c-imx-lpi2c.c:583:21: error: implicit declaration of function 'devm_gpiod_get'; did you mean 'devm_gpio_free'? [-Werror=implicit-function-declaration]
     583 |  rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
         |                     ^~~~~~~~~~~~~~
         |                     devm_gpio_free
>> drivers/i2c/busses/i2c-imx-lpi2c.c:583:55: error: 'GPIOD_IN' undeclared (first use in this function); did you mean 'GPIOF_IN'?
     583 |  rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
         |                                                       ^~~~~~~~
         |                                                       GPIOF_IN
   drivers/i2c/busses/i2c-imx-lpi2c.c:583:55: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/i2c/busses/i2c-imx-lpi2c.c:584:55: error: 'GPIOD_OUT_HIGH_OPEN_DRAIN' undeclared (first use in this function)
     584 |  rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
         |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +583 drivers/i2c/busses/i2c-imx-lpi2c.c

   560	
   561	/*
   562	 * We switch SCL and SDA to their GPIO function and do some bitbanging
   563	 * for bus recovery. These alternative pinmux settings can be
   564	 * described in the device tree by a separate pinctrl state "gpio". If
   565	 * this is missing this is not a big problem, the only implication is
   566	 * that we can't do bus recovery.
   567	 */
   568	static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
   569			struct platform_device *pdev)
   570	{
   571		struct i2c_bus_recovery_info *rinfo = &lpi2c_imx->rinfo;
   572	
   573		lpi2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
   574		if (!lpi2c_imx->pinctrl || IS_ERR(lpi2c_imx->pinctrl)) {
   575			dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
   576			return PTR_ERR(lpi2c_imx->pinctrl);
   577		}
   578	
   579		lpi2c_imx->pinctrl_pins_default = pinctrl_lookup_state(lpi2c_imx->pinctrl,
   580				PINCTRL_STATE_DEFAULT);
   581		lpi2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(lpi2c_imx->pinctrl,
   582				"gpio");
 > 583		rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
 > 584		rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
   585	
   586		if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||
   587		    PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER) {
   588			return -EPROBE_DEFER;
   589		} else if (IS_ERR(rinfo->sda_gpiod) ||
   590			   IS_ERR(rinfo->scl_gpiod) ||
   591			   IS_ERR(lpi2c_imx->pinctrl_pins_default) ||
   592			   IS_ERR(lpi2c_imx->pinctrl_pins_gpio)) {
   593			dev_dbg(&pdev->dev, "recovery information incomplete\n");
   594			return 0;
   595		}
   596	
   597		dev_info(&pdev->dev, "using scl%s for recovery\n",
   598			 rinfo->sda_gpiod ? ",sda" : "");
   599	
   600		rinfo->prepare_recovery = lpi2c_imx_prepare_recovery;
   601		rinfo->unprepare_recovery = lpi2c_imx_unprepare_recovery;
   602		rinfo->recover_bus = i2c_generic_scl_recovery;
   603		lpi2c_imx->adapter.bus_recovery_info = rinfo;
   604	
   605		return 0;
   606	}
   607	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Aisheng Dong May 21, 2021, 6:44 a.m. UTC | #2 
> From: Clark Wang <xiaoning.wang@nxp.com>
> Sent: Tuesday, April 6, 2021 7:33 PM
> 
> Add bus recovery feature for LPI2C.
> Need add gpio pinctrl, scl-gpios and sda-gpios configuration in dts.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> ---
> V2 changes:
>  - No change. Add dt-bindings in the next patch.

Dt binding patch should be sent before driver change

> ---
>  drivers/i2c/busses/i2c-imx-lpi2c.c | 83 ++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 77ceb743b282..77dd6ee5a4a8 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> @@ -108,6 +109,11 @@ struct lpi2c_imx_struct {
>  	unsigned int		txfifosize;
>  	unsigned int		rxfifosize;
>  	enum lpi2c_imx_mode	mode;
> +
> +	struct i2c_bus_recovery_info rinfo;
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_pins_default;
> +	struct pinctrl_state *pinctrl_pins_gpio;
>  };
> 
>  static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx, @@ -135,6
> +141,8 @@ static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx)
> 
>  		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
>  			dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> +			if (lpi2c_imx->adapter.bus_recovery_info)
> +				i2c_recover_bus(&lpi2c_imx->adapter);
>  			return -ETIMEDOUT;
>  		}
>  		schedule();
> @@ -192,6 +200,8 @@ static void lpi2c_imx_stop(struct lpi2c_imx_struct
> *lpi2c_imx)
> 
>  		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
>  			dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> +			if (lpi2c_imx->adapter.bus_recovery_info)
> +				i2c_recover_bus(&lpi2c_imx->adapter);
>  			break;
>  		}
>  		schedule();
> @@ -329,6 +339,8 @@ static int lpi2c_imx_txfifo_empty(struct
> lpi2c_imx_struct *lpi2c_imx)
> 
>  		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
>  			dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty timeout\n");
> +			if (lpi2c_imx->adapter.bus_recovery_info)
> +				i2c_recover_bus(&lpi2c_imx->adapter);
>  			return -ETIMEDOUT;
>  		}
>  		schedule();
> @@ -528,6 +540,71 @@ static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
> 
> +static void lpi2c_imx_prepare_recovery(struct i2c_adapter *adap) {
> +	struct lpi2c_imx_struct *lpi2c_imx;
> +
> +	lpi2c_imx = container_of(adap, struct lpi2c_imx_struct, adapter);
> +
> +	pinctrl_select_state(lpi2c_imx->pinctrl,
> +lpi2c_imx->pinctrl_pins_gpio); }
> +
> +static void lpi2c_imx_unprepare_recovery(struct i2c_adapter *adap) {
> +	struct lpi2c_imx_struct *lpi2c_imx;
> +
> +	lpi2c_imx = container_of(adap, struct lpi2c_imx_struct, adapter);
> +
> +	pinctrl_select_state(lpi2c_imx->pinctrl,
> +lpi2c_imx->pinctrl_pins_default); }
> +
> +/*
> + * We switch SCL and SDA to their GPIO function and do some bitbanging
> + * for bus recovery. These alternative pinmux settings can be
> + * described in the device tree by a separate pinctrl state "gpio". If
> + * this is missing this is not a big problem, the only implication is
> + * that we can't do bus recovery.
> + */
> +static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
> +		struct platform_device *pdev)
> +{
> +	struct i2c_bus_recovery_info *rinfo = &lpi2c_imx->rinfo;
> +
> +	lpi2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
> +	if (!lpi2c_imx->pinctrl || IS_ERR(lpi2c_imx->pinctrl)) {
> +		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not
> supported\n");
> +		return PTR_ERR(lpi2c_imx->pinctrl);
> +	}
> +
> +	lpi2c_imx->pinctrl_pins_default = pinctrl_lookup_state(lpi2c_imx->pinctrl,
> +			PINCTRL_STATE_DEFAULT);
> +	lpi2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(lpi2c_imx->pinctrl,
> +			"gpio");
> +	rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
> +	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl",
> +GPIOD_OUT_HIGH_OPEN_DRAIN);
> +
> +	if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||
> +	    PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER) {
> +		return -EPROBE_DEFER;
> +	} else if (IS_ERR(rinfo->sda_gpiod) ||
> +		   IS_ERR(rinfo->scl_gpiod) ||
> +		   IS_ERR(lpi2c_imx->pinctrl_pins_default) ||
> +		   IS_ERR(lpi2c_imx->pinctrl_pins_gpio)) {
> +		dev_dbg(&pdev->dev, "recovery information incomplete\n");
> +		return 0;
> +	}
> +
> +	dev_info(&pdev->dev, "using scl%s for recovery\n",
> +		 rinfo->sda_gpiod ? ",sda" : "");
> +
> +	rinfo->prepare_recovery = lpi2c_imx_prepare_recovery;
> +	rinfo->unprepare_recovery = lpi2c_imx_unprepare_recovery;
> +	rinfo->recover_bus = i2c_generic_scl_recovery;
> +	lpi2c_imx->adapter.bus_recovery_info = rinfo;
> +
> +	return 0;
> +}
> +
>  static u32 lpi2c_imx_func(struct i2c_adapter *adapter)  {
>  	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | @@ -632,6 +709,12
> @@ static int lpi2c_imx_probe(struct platform_device *pdev)
>  	lpi2c_imx->txfifosize = 1 << (temp & 0x0f);
>  	lpi2c_imx->rxfifosize = 1 << ((temp >> 8) & 0x0f);
> 
> +	/* Init optional bus recovery function */
> +	ret = lpi2c_imx_init_recovery_info(lpi2c_imx, pdev);
> +	/* Give it another chance if pinctrl used is not ready yet */
> +	if (ret == -EPROBE_DEFER)
> +		goto rpm_disable;
> +
>  	ret = i2c_add_adapter(&lpi2c_imx->adapter);
>  	if (ret)
>  		goto rpm_disable;
> --
> 2.25.1
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 77ceb743b282..77dd6ee5a4a8 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -18,6 +18,7 @@ 
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -108,6 +109,11 @@  struct lpi2c_imx_struct {
 	unsigned int		txfifosize;
 	unsigned int		rxfifosize;
 	enum lpi2c_imx_mode	mode;
+
+	struct i2c_bus_recovery_info rinfo;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_pins_default;
+	struct pinctrl_state *pinctrl_pins_gpio;
 };
 
 static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx,
@@ -135,6 +141,8 @@  static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx)
 
 		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
 			dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
+			if (lpi2c_imx->adapter.bus_recovery_info)
+				i2c_recover_bus(&lpi2c_imx->adapter);
 			return -ETIMEDOUT;
 		}
 		schedule();
@@ -192,6 +200,8 @@  static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx)
 
 		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
 			dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
+			if (lpi2c_imx->adapter.bus_recovery_info)
+				i2c_recover_bus(&lpi2c_imx->adapter);
 			break;
 		}
 		schedule();
@@ -329,6 +339,8 @@  static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
 
 		if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
 			dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty timeout\n");
+			if (lpi2c_imx->adapter.bus_recovery_info)
+				i2c_recover_bus(&lpi2c_imx->adapter);
 			return -ETIMEDOUT;
 		}
 		schedule();
@@ -528,6 +540,71 @@  static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void lpi2c_imx_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct lpi2c_imx_struct *lpi2c_imx;
+
+	lpi2c_imx = container_of(adap, struct lpi2c_imx_struct, adapter);
+
+	pinctrl_select_state(lpi2c_imx->pinctrl, lpi2c_imx->pinctrl_pins_gpio);
+}
+
+static void lpi2c_imx_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct lpi2c_imx_struct *lpi2c_imx;
+
+	lpi2c_imx = container_of(adap, struct lpi2c_imx_struct, adapter);
+
+	pinctrl_select_state(lpi2c_imx->pinctrl, lpi2c_imx->pinctrl_pins_default);
+}
+
+/*
+ * We switch SCL and SDA to their GPIO function and do some bitbanging
+ * for bus recovery. These alternative pinmux settings can be
+ * described in the device tree by a separate pinctrl state "gpio". If
+ * this is missing this is not a big problem, the only implication is
+ * that we can't do bus recovery.
+ */
+static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
+		struct platform_device *pdev)
+{
+	struct i2c_bus_recovery_info *rinfo = &lpi2c_imx->rinfo;
+
+	lpi2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!lpi2c_imx->pinctrl || IS_ERR(lpi2c_imx->pinctrl)) {
+		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
+		return PTR_ERR(lpi2c_imx->pinctrl);
+	}
+
+	lpi2c_imx->pinctrl_pins_default = pinctrl_lookup_state(lpi2c_imx->pinctrl,
+			PINCTRL_STATE_DEFAULT);
+	lpi2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(lpi2c_imx->pinctrl,
+			"gpio");
+	rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
+	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
+
+	if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||
+	    PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(rinfo->sda_gpiod) ||
+		   IS_ERR(rinfo->scl_gpiod) ||
+		   IS_ERR(lpi2c_imx->pinctrl_pins_default) ||
+		   IS_ERR(lpi2c_imx->pinctrl_pins_gpio)) {
+		dev_dbg(&pdev->dev, "recovery information incomplete\n");
+		return 0;
+	}
+
+	dev_info(&pdev->dev, "using scl%s for recovery\n",
+		 rinfo->sda_gpiod ? ",sda" : "");
+
+	rinfo->prepare_recovery = lpi2c_imx_prepare_recovery;
+	rinfo->unprepare_recovery = lpi2c_imx_unprepare_recovery;
+	rinfo->recover_bus = i2c_generic_scl_recovery;
+	lpi2c_imx->adapter.bus_recovery_info = rinfo;
+
+	return 0;
+}
+
 static u32 lpi2c_imx_func(struct i2c_adapter *adapter)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
@@ -632,6 +709,12 @@  static int lpi2c_imx_probe(struct platform_device *pdev)
 	lpi2c_imx->txfifosize = 1 << (temp & 0x0f);
 	lpi2c_imx->rxfifosize = 1 << ((temp >> 8) & 0x0f);
 
+	/* Init optional bus recovery function */
+	ret = lpi2c_imx_init_recovery_info(lpi2c_imx, pdev);
+	/* Give it another chance if pinctrl used is not ready yet */
+	if (ret == -EPROBE_DEFER)
+		goto rpm_disable;
+
 	ret = i2c_add_adapter(&lpi2c_imx->adapter);
 	if (ret)
 		goto rpm_disable;