| Message ID | 20210406113306.2633595-11-xiaoning.wang@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: imx-lpi2c: New features and bug fixes | expand |
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
> 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 --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;
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(+)