drivers/i2c/busses/i2c-designware-master.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Currently if the SoC needs pinctrl to switch the SCL and SDA from the I2C
function to GPIO function, the recovery won't work.
scl-gpio = <>;
sda-gpio = <>;
Are not enough for some SoCs to have a working recovery.
Some need:
scl-gpio = <>;
sda-gpio = <>;
pinctrl-names = "default", "recovery";
pinctrl-0 = <&i2c_pins_hw>;
pinctrl-1 = <&i2c_pins_gpio>;
The driver was not filling rinfo->pinctrl with the device node
pinctrl data which is needed by generic recovery code.
Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V5 -> V6:
* Put a proper commit msg subject
V4 -> V5:
* put back `else if`
* reword the commit msg Subject to add the `i2c: designware: ` tag
* Add the missing `Reviewed-by: Andy Shevchenko` tag
V3 -> V4:
* Replace `else if` by simply `if`.
V2 -> V3:
* put back 'if (!rinfo->pinctrl)' test since devm_pinctrl_get()
can return NULL if CONFIG_PINCTRL is not set.
* print an error msg when devm_pinctrl_get() returns an error that
is not -EPROBE_DEFER.
* print a dbg msg if devm_pinctrl_get() return NULL.
V1 -> V2:
* remove the unnecessary 'if (!rinfo->pinctrl)' test
* test if return is -EPROBE_DEFER, in that case, return it.
* Reword the commit message according to review
drivers/i2c/busses/i2c-designware-master.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index b720fcc5c10a..30b2de829a32 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -17,6 +17,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/reset.h>
@@ -855,6 +856,17 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
return PTR_ERR(gpio);
rinfo->sda_gpiod = gpio;
+ rinfo->pinctrl = devm_pinctrl_get(dev->dev);
+ if (IS_ERR(rinfo->pinctrl)) {
+ if (PTR_ERR(rinfo->pinctrl) == -EPROBE_DEFER)
+ return PTR_ERR(rinfo->pinctrl);
+
+ rinfo->pinctrl = NULL;
+ dev_err(dev->dev, "getting pinctrl info failed: bus recovery might not work\n");
+ } else if (!rinfo->pinctrl) {
+ dev_dbg(dev->dev, "pinctrl is disabled, bus recovery might not work\n");
+ }
+
rinfo->recover_bus = i2c_generic_scl_recovery;
rinfo->prepare_recovery = i2c_dw_prepare_recovery;
rinfo->unprepare_recovery = i2c_dw_unprepare_recovery;
--
2.17.1On Tue, Aug 22, 2023 at 04:34:37PM +0200, Yann Sionneau wrote: > Currently if the SoC needs pinctrl to switch the SCL and SDA from the I2C > function to GPIO function, the recovery won't work. > > scl-gpio = <>; > sda-gpio = <>; > > Are not enough for some SoCs to have a working recovery. > Some need: > > scl-gpio = <>; > sda-gpio = <>; > pinctrl-names = "default", "recovery"; > pinctrl-0 = <&i2c_pins_hw>; > pinctrl-1 = <&i2c_pins_gpio>; > > The driver was not filling rinfo->pinctrl with the device node > pinctrl data which is needed by generic recovery code. > > Signed-off-by: Yann Sionneau <ysionneau@kalray.eu> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Applied to for-next, thanks!
Hi Yann,
[...]
> + rinfo->pinctrl = devm_pinctrl_get(dev->dev);
> + if (IS_ERR(rinfo->pinctrl)) {
> + if (PTR_ERR(rinfo->pinctrl) == -EPROBE_DEFER)
> + return PTR_ERR(rinfo->pinctrl);
> +
> + rinfo->pinctrl = NULL;
> + dev_err(dev->dev, "getting pinctrl info failed: bus recovery might not work\n");
I'd still have preferred a dev_warn() here... but it's OK.
> + } else if (!rinfo->pinctrl) {
> + dev_dbg(dev->dev, "pinctrl is disabled, bus recovery might not work\n");
> + }
thanks for following up,
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Thanks,
Andi
On Tue, Aug 22, 2023 at 04:34:37PM +0200, Yann Sionneau wrote: > Currently if the SoC needs pinctrl to switch the SCL and SDA from the I2C > function to GPIO function, the recovery won't work. > > scl-gpio = <>; > sda-gpio = <>; > > Are not enough for some SoCs to have a working recovery. > Some need: > > scl-gpio = <>; > sda-gpio = <>; > pinctrl-names = "default", "recovery"; > pinctrl-0 = <&i2c_pins_hw>; > pinctrl-1 = <&i2c_pins_gpio>; > > The driver was not filling rinfo->pinctrl with the device node > pinctrl data which is needed by generic recovery code. Now looks pretty much good enough (yet the period is not needed in the Subject, but it's fine for your newbie submission — no need to resend or make a new version). Thank you! -- With Best Regards, Andy Shevchenko
On 8/22/23 17:43, Andy Shevchenko wrote: > On Tue, Aug 22, 2023 at 04:34:37PM +0200, Yann Sionneau wrote: >> Currently if the SoC needs pinctrl to switch the SCL and SDA from the I2C >> function to GPIO function, the recovery won't work. >> >> scl-gpio = <>; >> sda-gpio = <>; >> >> Are not enough for some SoCs to have a working recovery. >> Some need: >> >> scl-gpio = <>; >> sda-gpio = <>; >> pinctrl-names = "default", "recovery"; >> pinctrl-0 = <&i2c_pins_hw>; >> pinctrl-1 = <&i2c_pins_gpio>; >> >> The driver was not filling rinfo->pinctrl with the device node >> pinctrl data which is needed by generic recovery code. > > Now looks pretty much good enough (yet the period is not needed in the Subject, > but it's fine for your newbie submission — no need to resend or make a new > version). Thank you! > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
© 2016 - 2025 Red Hat, Inc.