* [PATCH v2 0/3] i2c: mv64xxx: Support for I2C unstuck
@ 2023-10-06 0:33 Chris Packham
2023-10-06 0:33 ` [PATCH v2 1/3] dt-bindings: i2c: mv64xxx: update bindings for unstuck register Chris Packham
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Chris Packham @ 2023-10-06 0:33 UTC (permalink / raw)
To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt,
conor+dt, pierre.gondois
Cc: linux-i2c, devicetree, linux-kernel, Chris Packham
Some newer Marvell SoCs natively support a recovery mechanisim. This can be
used as an alternative to the generic pinctrl/GPIO recovery mechanism used on
the older SoCs.
Chris Packham (3):
dt-bindings: i2c: mv64xxx: update bindings for unstuck register
arm64: dts: marvell: AC5: use I2C unstuck function
i2c: mv64xxx: add support for FSM based recovery
.../bindings/i2c/marvell,mv64xxx-i2c.yaml | 5 +-
arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++--
drivers/i2c/busses/i2c-mv64xxx.c | 68 ++++++++++++++++++-
3 files changed, 73 insertions(+), 14 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/3] dt-bindings: i2c: mv64xxx: update bindings for unstuck register 2023-10-06 0:33 [PATCH v2 0/3] i2c: mv64xxx: Support for I2C unstuck Chris Packham @ 2023-10-06 0:33 ` Chris Packham 2023-10-06 0:33 ` [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function Chris Packham 2023-10-06 0:33 ` [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery Chris Packham 2 siblings, 0 replies; 10+ messages in thread From: Chris Packham @ 2023-10-06 0:33 UTC (permalink / raw) To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois Cc: linux-i2c, devicetree, linux-kernel, Chris Packham Some newer Marvell SoCs support an "unstuck" function for I2C bus recovery. This is an alternative to the generic GPIO based recovery that the older SoCs use. The unstuck register falls outside of the usual address block for the I2C controller so requires an additional cell in the register property. This is optional and does not need to be supplied. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Acked-by: Conor Dooley <conor.dooley@microchip.com> --- Notes: Changes in v2: - Collect ack from Conor .../devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml b/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml index 984fc1ed3ec6..461d1c9ee3f7 100644 --- a/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml @@ -45,7 +45,10 @@ properties: auto-detects this and sets it appropriately. reg: - maxItems: 1 + minItems: 1 + items: + - description: I2C controller registers + - description: I2C unstuck register interrupts: maxItems: 1 -- 2.42.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function 2023-10-06 0:33 [PATCH v2 0/3] i2c: mv64xxx: Support for I2C unstuck Chris Packham 2023-10-06 0:33 ` [PATCH v2 1/3] dt-bindings: i2c: mv64xxx: update bindings for unstuck register Chris Packham @ 2023-10-06 0:33 ` Chris Packham 2023-10-19 14:40 ` Gregory CLEMENT 2023-10-06 0:33 ` [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery Chris Packham 2 siblings, 1 reply; 10+ messages in thread From: Chris Packham @ 2023-10-06 0:33 UTC (permalink / raw) To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois Cc: linux-i2c, devicetree, linux-kernel, Chris Packham The AC5 SoC supports using a controller based I2C unstuck function for recovery. Use this instead of the generic GPIO recovery. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi index c9ce1010c415..e52d3c3496d5 100644 --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi @@ -137,7 +137,7 @@ mdio: mdio@22004 { i2c0: i2c@11000{ compatible = "marvell,mv78230-i2c"; - reg = <0x11000 0x20>; + reg = <0x11000 0x20>, <0x110a0 0x4>; #address-cells = <1>; #size-cells = <0>; @@ -146,17 +146,14 @@ i2c0: i2c@11000{ interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>; clock-frequency=<100000>; - pinctrl-names = "default", "gpio"; + pinctrl-names = "default"; pinctrl-0 = <&i2c0_pins>; - pinctrl-1 = <&i2c0_gpio>; - scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; - sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; status = "disabled"; }; i2c1: i2c@11100{ compatible = "marvell,mv78230-i2c"; - reg = <0x11100 0x20>; + reg = <0x11100 0x20>, <0x110a4 0x4>; #address-cells = <1>; #size-cells = <0>; @@ -165,11 +162,8 @@ i2c1: i2c@11100{ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>; clock-frequency=<100000>; - pinctrl-names = "default", "gpio"; + pinctrl-names = "default"; pinctrl-0 = <&i2c1_pins>; - pinctrl-1 = <&i2c1_gpio>; - scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; - sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; status = "disabled"; }; -- 2.42.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function 2023-10-06 0:33 ` [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function Chris Packham @ 2023-10-19 14:40 ` Gregory CLEMENT 2023-10-19 19:41 ` Chris Packham 0 siblings, 1 reply; 10+ messages in thread From: Gregory CLEMENT @ 2023-10-19 14:40 UTC (permalink / raw) To: Chris Packham, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois Cc: linux-i2c, devicetree, linux-kernel, Chris Packham Hello Chris, > The AC5 SoC supports using a controller based I2C unstuck function for > recovery. Use this instead of the generic GPIO recovery. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi > index c9ce1010c415..e52d3c3496d5 100644 > --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi > +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi > @@ -137,7 +137,7 @@ mdio: mdio@22004 { > > i2c0: i2c@11000{ > compatible = "marvell,mv78230-i2c"; > - reg = <0x11000 0x20>; > + reg = <0x11000 0x20>, <0x110a0 0x4>; > #address-cells = <1>; > #size-cells = <0>; > > @@ -146,17 +146,14 @@ i2c0: i2c@11000{ > interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>; > clock-frequency=<100000>; > > - pinctrl-names = "default", "gpio"; > + pinctrl-names = "default"; > pinctrl-0 = <&i2c0_pins>; > - pinctrl-1 = <&i2c0_gpio>; > - scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; > - sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; By doing this then older kernel won't be able to do recovery, while if you keep it, the new kernels will still use new way to support recovery thanks to the new reg filed added and old kernels will continue to work. However, what we try to maintain is running new kernel on old dtb not the opposite which is just a nice to have. At the end it is up to you, if you really want to remove this chunk I will apply it once the driver part of the series will be accepted. Gregory > status = "disabled"; > }; > > i2c1: i2c@11100{ > compatible = "marvell,mv78230-i2c"; > - reg = <0x11100 0x20>; > + reg = <0x11100 0x20>, <0x110a4 0x4>; > #address-cells = <1>; > #size-cells = <0>; > > @@ -165,11 +162,8 @@ i2c1: i2c@11100{ > interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>; > clock-frequency=<100000>; > > - pinctrl-names = "default", "gpio"; > + pinctrl-names = "default"; > pinctrl-0 = <&i2c1_pins>; > - pinctrl-1 = <&i2c1_gpio>; > - scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; > - sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; > status = "disabled"; > }; > > -- > 2.42.0 > -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function 2023-10-19 14:40 ` Gregory CLEMENT @ 2023-10-19 19:41 ` Chris Packham 0 siblings, 0 replies; 10+ messages in thread From: Chris Packham @ 2023-10-19 19:41 UTC (permalink / raw) To: Gregory CLEMENT, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois Cc: linux-i2c, devicetree, linux-kernel On 20/10/23 03:40, Gregory CLEMENT wrote: > Hello Chris, > >> The AC5 SoC supports using a controller based I2C unstuck function for >> recovery. Use this instead of the generic GPIO recovery. >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++---------- >> 1 file changed, 4 insertions(+), 10 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi >> index c9ce1010c415..e52d3c3496d5 100644 >> --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi >> +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi >> @@ -137,7 +137,7 @@ mdio: mdio@22004 { >> >> i2c0: i2c@11000{ >> compatible = "marvell,mv78230-i2c"; >> - reg = <0x11000 0x20>; >> + reg = <0x11000 0x20>, <0x110a0 0x4>; >> #address-cells = <1>; >> #size-cells = <0>; >> >> @@ -146,17 +146,14 @@ i2c0: i2c@11000{ >> interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>; >> clock-frequency=<100000>; >> >> - pinctrl-names = "default", "gpio"; >> + pinctrl-names = "default"; >> pinctrl-0 = <&i2c0_pins>; >> - pinctrl-1 = <&i2c0_gpio>; >> - scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> - sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; > By doing this then older kernel won't be able to do recovery, while if > you keep it, the new kernels will still use new way to support recovery > thanks to the new reg filed added and old kernels will continue to work. > > However, what we try to maintain is running new kernel on old dtb not > the opposite which is just a nice to have. At the end it is up to you, > if you really want to remove this chunk I will apply it once the driver > part of the series will be accepted. The GPIO recovery triggers an Erratum where the SoC locks up so I'd prefer to see it gone (basically a version of that offload Erratum from the early Armada-XPs). I think it's all academic because I'm pretty sure I'm the only one actually running an upstream kernel on the AC5X. Marvell still ship a horribly out of date fork in their official SDK. > > Gregory > > >> status = "disabled"; >> }; >> >> i2c1: i2c@11100{ >> compatible = "marvell,mv78230-i2c"; >> - reg = <0x11100 0x20>; >> + reg = <0x11100 0x20>, <0x110a4 0x4>; >> #address-cells = <1>; >> #size-cells = <0>; >> >> @@ -165,11 +162,8 @@ i2c1: i2c@11100{ >> interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>; >> clock-frequency=<100000>; >> >> - pinctrl-names = "default", "gpio"; >> + pinctrl-names = "default"; >> pinctrl-0 = <&i2c1_pins>; >> - pinctrl-1 = <&i2c1_gpio>; >> - scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> - sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> status = "disabled"; >> }; >> >> -- >> 2.42.0 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery 2023-10-06 0:33 [PATCH v2 0/3] i2c: mv64xxx: Support for I2C unstuck Chris Packham 2023-10-06 0:33 ` [PATCH v2 1/3] dt-bindings: i2c: mv64xxx: update bindings for unstuck register Chris Packham 2023-10-06 0:33 ` [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function Chris Packham @ 2023-10-06 0:33 ` Chris Packham 2023-10-12 20:15 ` Andi Shyti 2 siblings, 1 reply; 10+ messages in thread From: Chris Packham @ 2023-10-06 0:33 UTC (permalink / raw) To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois Cc: linux-i2c, devicetree, linux-kernel, Chris Packham Some newer Marvell SoCs (AC5 and CN9130, possibly more) support a I2C unstuck function. This provides a recovery function as part of the FSM as an alternative to changing pinctrl modes and using the generic GPIO based recovery. Allow for using this by adding an optional resource to the platform data which contains the address of the I2C unstuck register for the I2C controller. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v2: - shorted delay and timeout used with read_poll_timeout_atomic() - make use dev_dbg() for added messages - remove reference to "marvell,armada-8k-i2c" - I've elected to keep the behaviour that failing to ioremap the unstuck register (if supplied) is an error drivers/i2c/busses/i2c-mv64xxx.c | 68 ++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index fd8403b07fa6..efd28bbecf61 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -21,6 +21,7 @@ #include <linux/pm_runtime.h> #include <linux/reset.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_irq.h> @@ -82,6 +83,13 @@ /* Bridge Status values */ #define MV64XXX_I2C_BRIDGE_STATUS_ERROR BIT(0) +/* Unstuck Register values */ +#define MV64XXX_I2C_UNSTUCK_TRIGGER BIT(0) +#define MV64XXX_I2C_UNSTUCK_ON_GOING BIT(1) +#define MV64XXX_I2C_UNSTUCK_ERROR BIT(2) +#define MV64XXX_I2C_UNSTUCK_COUNT(val) ((val & 0xf0) >> 4) +#define MV64XXX_I2C_UNSTUCK_INPROGRESS (MV64XXX_I2C_UNSTUCK_TRIGGER|MV64XXX_I2C_UNSTUCK_ON_GOING) + /* Driver states */ enum { MV64XXX_I2C_STATE_INVALID, @@ -126,6 +134,7 @@ struct mv64xxx_i2c_data { u32 aborting; u32 cntl_bits; void __iomem *reg_base; + void __iomem *unstuck_reg; struct mv64xxx_i2c_regs reg_offsets; u32 addr1; u32 addr2; @@ -735,6 +744,33 @@ mv64xxx_i2c_can_offload(struct mv64xxx_i2c_data *drv_data) return false; } +static int +mv64xxx_i2c_recover_bus(struct i2c_adapter *adap) +{ + struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); + int ret; + u32 val; + + dev_dbg(&adap->dev, "Trying i2c bus recovery\n"); + writel(MV64XXX_I2C_UNSTUCK_TRIGGER, drv_data->unstuck_reg); + ret = readl_poll_timeout_atomic(drv_data->unstuck_reg, val, + !(val & MV64XXX_I2C_UNSTUCK_INPROGRESS), + 10, 1000); + if (ret) { + dev_err(&adap->dev, "recovery timeout\n"); + return ret; + } + + if (val & MV64XXX_I2C_UNSTUCK_ERROR) { + dev_err(&adap->dev, "recovery failed\n"); + return -EBUSY; + } + + dev_dbg(&adap->dev, "recovery complete after %d pulses\n", MV64XXX_I2C_UNSTUCK_COUNT(val)); + + return 0; +} + /* ***************************************************************************** * @@ -936,8 +972,21 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, } #endif /* CONFIG_OF */ -static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data, - struct device *dev) +static int mv64xxx_i2c_init_fsm_recovery_info(struct mv64xxx_i2c_data *drv_data, + struct device *dev) +{ + struct i2c_bus_recovery_info *rinfo = &drv_data->rinfo; + + dev_dbg(dev, "using FSM for recovery\n"); + rinfo->recover_bus = mv64xxx_i2c_recover_bus; + drv_data->adapter.bus_recovery_info = rinfo; + + return 0; + +} + +static int mv64xxx_i2c_init_gpio_recovery_info(struct mv64xxx_i2c_data *drv_data, + struct device *dev) { struct i2c_bus_recovery_info *rinfo = &drv_data->rinfo; @@ -986,6 +1035,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) { struct mv64xxx_i2c_data *drv_data; struct mv64xxx_i2c_pdata *pdata = dev_get_platdata(&pd->dev); + struct resource *res; int rc; if ((!pdata && !pd->dev.of_node)) @@ -1000,6 +1050,14 @@ mv64xxx_i2c_probe(struct platform_device *pd) if (IS_ERR(drv_data->reg_base)) return PTR_ERR(drv_data->reg_base); + /* optional unstuck support */ + res = platform_get_resource(pd, IORESOURCE_MEM, 1); + if (res) { + drv_data->unstuck_reg = devm_ioremap_resource(&pd->dev, res); + if (IS_ERR(drv_data->unstuck_reg)) + return PTR_ERR(drv_data->unstuck_reg); + } + strscpy(drv_data->adapter.name, MV64XXX_I2C_CTLR_NAME " adapter", sizeof(drv_data->adapter.name)); @@ -1037,7 +1095,11 @@ mv64xxx_i2c_probe(struct platform_device *pd) return rc; } - rc = mv64xxx_i2c_init_recovery_info(drv_data, &pd->dev); + if (drv_data->unstuck_reg) + rc = mv64xxx_i2c_init_fsm_recovery_info(drv_data, &pd->dev); + else + rc = mv64xxx_i2c_init_gpio_recovery_info(drv_data, &pd->dev); + if (rc == -EPROBE_DEFER) return rc; -- 2.42.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery 2023-10-06 0:33 ` [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery Chris Packham @ 2023-10-12 20:15 ` Andi Shyti 2023-10-12 20:25 ` Chris Packham 2023-10-13 7:11 ` Wolfram Sang 0 siblings, 2 replies; 10+ messages in thread From: Andi Shyti @ 2023-10-12 20:15 UTC (permalink / raw) To: Chris Packham Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois, linux-i2c, devicetree, linux-kernel Hi Chris, ... > +static int > +mv64xxx_i2c_recover_bus(struct i2c_adapter *adap) > +{ > + struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); > + int ret; > + u32 val; > + > + dev_dbg(&adap->dev, "Trying i2c bus recovery\n"); > + writel(MV64XXX_I2C_UNSTUCK_TRIGGER, drv_data->unstuck_reg); > + ret = readl_poll_timeout_atomic(drv_data->unstuck_reg, val, > + !(val & MV64XXX_I2C_UNSTUCK_INPROGRESS), > + 10, 1000); mmmhhh... still a bit skeptical about waiting 100 times 10us in atomic. I'm still of the opinion that this should run in a separate thread. Any different opinion from the network? BTW, first question, considering that you decreased the time considerably... does it work? Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery 2023-10-12 20:15 ` Andi Shyti @ 2023-10-12 20:25 ` Chris Packham 2023-10-13 7:11 ` Wolfram Sang 1 sibling, 0 replies; 10+ messages in thread From: Chris Packham @ 2023-10-12 20:25 UTC (permalink / raw) To: Andi Shyti Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois, linux-i2c, devicetree, linux-kernel On 13/10/23 09:15, Andi Shyti wrote: > Hi Chris, > > ... > >> +static int >> +mv64xxx_i2c_recover_bus(struct i2c_adapter *adap) >> +{ >> + struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); >> + int ret; >> + u32 val; >> + >> + dev_dbg(&adap->dev, "Trying i2c bus recovery\n"); >> + writel(MV64XXX_I2C_UNSTUCK_TRIGGER, drv_data->unstuck_reg); >> + ret = readl_poll_timeout_atomic(drv_data->unstuck_reg, val, >> + !(val & MV64XXX_I2C_UNSTUCK_INPROGRESS), >> + 10, 1000); > mmmhhh... still a bit skeptical about waiting 100 times 10us in > atomic. > > I'm still of the opinion that this should run in a separate > thread. Any different opinion from the network? > > BTW, first question, considering that you decreased the time > considerably... does it work? Yes it still works. It did stop working with a really low timeout (10, 100) but I didn't look hard for anything in-between. > > Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery 2023-10-12 20:15 ` Andi Shyti 2023-10-12 20:25 ` Chris Packham @ 2023-10-13 7:11 ` Wolfram Sang 2023-10-15 20:12 ` Chris Packham 1 sibling, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2023-10-13 7:11 UTC (permalink / raw) To: Andi Shyti Cc: Chris Packham, gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois, linux-i2c, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 172 bytes --] > mmmhhh... still a bit skeptical about waiting 100 times 10us in > atomic. Has it been discussed already why the non-atomic version of read_poll_timeout is not enough? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery 2023-10-13 7:11 ` Wolfram Sang @ 2023-10-15 20:12 ` Chris Packham 0 siblings, 0 replies; 10+ messages in thread From: Chris Packham @ 2023-10-15 20:12 UTC (permalink / raw) To: Wolfram Sang, Andi Shyti, gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt, pierre.gondois, linux-i2c, devicetree, linux-kernel On 13/10/23 20:11, Wolfram Sang wrote: >> mmmhhh... still a bit skeptical about waiting 100 times 10us in >> atomic. > Has it been discussed already why the non-atomic version of > read_poll_timeout is not enough? > For mv64xxx i2c_recovery() is called from two places. One would be fine with read_poll_timeout() but the other is in an interrupt handler so needs the atomic version (or something else that doesn't schedule). ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-19 19:41 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-10-06 0:33 [PATCH v2 0/3] i2c: mv64xxx: Support for I2C unstuck Chris Packham 2023-10-06 0:33 ` [PATCH v2 1/3] dt-bindings: i2c: mv64xxx: update bindings for unstuck register Chris Packham 2023-10-06 0:33 ` [PATCH v2 2/3] arm64: dts: marvell: AC5: use I2C unstuck function Chris Packham 2023-10-19 14:40 ` Gregory CLEMENT 2023-10-19 19:41 ` Chris Packham 2023-10-06 0:33 ` [PATCH v2 3/3] i2c: mv64xxx: add support for FSM based recovery Chris Packham 2023-10-12 20:15 ` Andi Shyti 2023-10-12 20:25 ` Chris Packham 2023-10-13 7:11 ` Wolfram Sang 2023-10-15 20:12 ` Chris Packham
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).