* [PATCH 1/4] dt-bindings: i2c: at91: document optional bus recovery properties
2019-10-02 14:46 [PATCH 0/4] i2c bus recovery for Microchip SoCs Kamel Bouhara
@ 2019-10-02 14:46 ` Kamel Bouhara
2019-10-02 14:46 ` [PATCH 2/4] i2c: at91: implement i2c bus recovery Kamel Bouhara
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Kamel Bouhara @ 2019-10-02 14:46 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel
Cc: devicetree, Thomas Petazzoni, Kamel Bouhara
The at91 I2C controller can support bus recovery by re-assigning SCL
and SDA to gpios. Add the optional pinctrl and gpio properties to do
so.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
Documentation/devicetree/bindings/i2c/i2c-at91.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index b7cec17c3daf..8ea2ce5d8610 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -19,8 +19,13 @@ Optional properties:
capable I2C controllers.
- i2c-sda-hold-time-ns: TWD hold time, only available for "atmel,sama5d4-i2c"
and "atmel,sama5d2-i2c".
+- scl-gpios: specify the gpio related to SCL pin
+- sda-gpios: specify the gpio related to SDA pin
+- pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
+ bus recovery, call it "gpio" state
- Child nodes conforming to i2c bus binding
+
Examples :
i2c0: i2c@fff84000 {
@@ -55,6 +60,11 @@ i2c0: i2c@f8034600 {
clocks = <&flx0>;
atmel,fifo-size = <16>;
i2c-sda-hold-time-ns = <336>;
+ pinctrl-names = "default", "gpio";
+ pinctrl-0 = <&pinctrl_i2c0>;
+ pinctrl-1 = <&pinctrl_i2c0_gpio>;
+ sda-gpios = <&pioA 30 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioA 31 GPIO_ACTIVE_HIGH>;
wm8731: wm8731@1a {
compatible = "wm8731";
--
2.23.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-02 14:46 [PATCH 0/4] i2c bus recovery for Microchip SoCs Kamel Bouhara
2019-10-02 14:46 ` [PATCH 1/4] dt-bindings: i2c: at91: document optional bus recovery properties Kamel Bouhara
@ 2019-10-02 14:46 ` Kamel Bouhara
2019-10-04 9:35 ` Claudiu.Beznea
` (2 more replies)
2019-10-02 14:46 ` [PATCH 3/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Kamel Bouhara
` (2 subsequent siblings)
4 siblings, 3 replies; 18+ messages in thread
From: Kamel Bouhara @ 2019-10-02 14:46 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel
Cc: devicetree, Thomas Petazzoni, Kamel Bouhara
Implement i2c bus recovery when slaves devices might hold SDA low.
In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
until the slave release SDA.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
drivers/i2c/busses/i2c-at91-master.c | 63 ++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-at91.h | 8 ++++
2 files changed, 71 insertions(+)
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index a3fcc35ffd3b..df5bb93f952d 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -18,11 +18,13 @@
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
#include <linux/err.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dma-atmel.h>
#include <linux/pm_runtime.h>
@@ -768,6 +770,63 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
return ret;
}
+static void at91_prepare_twi_recovery(struct i2c_adapter *adap)
+{
+ struct at91_twi_dev *dev = i2c_get_adapdata(adap);
+
+ pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_gpio);
+}
+
+static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
+{
+ struct at91_twi_dev *dev = i2c_get_adapdata(adap);
+
+ pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
+}
+
+static int at91_init_twi_recovery_info(struct platform_device *pdev,
+ struct at91_twi_dev *dev)
+{
+ struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+
+ dev->pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
+ dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
+ return PTR_ERR(dev->pinctrl);
+ }
+
+ dev->pinctrl_pins_default = pinctrl_lookup_state(dev->pinctrl,
+ PINCTRL_STATE_DEFAULT);
+ dev->pinctrl_pins_gpio = pinctrl_lookup_state(dev->pinctrl,
+ "gpio");
+ rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
+ if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl",
+ GPIOD_OUT_HIGH_OPEN_DRAIN);
+ if (PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ if (IS_ERR(rinfo->sda_gpiod) ||
+ IS_ERR(rinfo->scl_gpiod) ||
+ IS_ERR(dev->pinctrl_pins_default) ||
+ IS_ERR(dev->pinctrl_pins_gpio)) {
+ dev_info(&pdev->dev, "recovery information incomplete\n");
+ return -EINVAL;
+ }
+
+ dev_info(&pdev->dev, "using scl%s for recovery\n",
+ rinfo->sda_gpiod ? ",sda" : "");
+
+ rinfo->prepare_recovery = at91_prepare_twi_recovery;
+ rinfo->unprepare_recovery = at91_unprepare_twi_recovery;
+ rinfo->recover_bus = i2c_generic_scl_recovery;
+ dev->adapter.bus_recovery_info = rinfo;
+
+ return 0;
+}
+
int at91_twi_probe_master(struct platform_device *pdev,
u32 phy_addr, struct at91_twi_dev *dev)
{
@@ -795,6 +854,10 @@ int at91_twi_probe_master(struct platform_device *pdev,
at91_calc_twi_clock(dev);
+ rc = at91_init_twi_recovery_info(pdev, dev);
+ if (rc == -EPROBE_DEFER)
+ return rc;
+
dev->adapter.algo = &at91_twi_algorithm;
dev->adapter.quirks = &at91_twi_quirks;
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index 499b506f6128..b89dab55e776 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -141,6 +141,10 @@ struct at91_twi_dev {
u32 fifo_size;
struct at91_twi_dma dma;
bool slave_detected;
+ struct i2c_bus_recovery_info rinfo;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *pinctrl_pins_default;
+ struct pinctrl_state *pinctrl_pins_gpio;
#ifdef CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL
unsigned smr;
struct i2c_client *slave;
@@ -158,6 +162,10 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev);
int at91_twi_probe_master(struct platform_device *pdev, u32 phy_addr,
struct at91_twi_dev *dev);
+void at91_twi_prepare_recovery(struct i2c_adapter *adap);
+void at91_twi_unprepare_recovery(struct i2c_adapter *adap);
+void at91_twi_init_recovery_info(struct at91_twi_dev *dev);
+
#ifdef CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL
void at91_init_twi_bus_slave(struct at91_twi_dev *dev);
int at91_twi_probe_slave(struct platform_device *pdev, u32 phy_addr,
--
2.23.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-02 14:46 ` [PATCH 2/4] i2c: at91: implement i2c bus recovery Kamel Bouhara
@ 2019-10-04 9:35 ` Claudiu.Beznea
2019-10-04 20:39 ` Uwe Kleine-König
2019-10-09 13:55 ` Ludovic Desroches
2019-10-21 20:20 ` Wolfram Sang
2 siblings, 1 reply; 18+ messages in thread
From: Claudiu.Beznea @ 2019-10-04 9:35 UTC (permalink / raw)
To: kamel.bouhara, wsa, linux-i2c, linux-kernel, Nicolas.Ferre,
alexandre.belloni, Ludovic.Desroches, linux-arm-kernel
Cc: devicetree, thomas.petazzoni
Hi Kamel,
On 02.10.2019 17:46, Kamel Bouhara wrote:
> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
> + struct at91_twi_dev *dev)
> +{
> + struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
> +
> + dev->pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
You may use IS_ERR_OR_NULL() here.
> + dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
> + return PTR_ERR(dev->pinctrl);
> + }
> +
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-04 9:35 ` Claudiu.Beznea
@ 2019-10-04 20:39 ` Uwe Kleine-König
2019-10-07 10:17 ` Claudiu.Beznea
0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2019-10-04 20:39 UTC (permalink / raw)
To: Claudiu.Beznea
Cc: kamel.bouhara, wsa, linux-i2c, linux-kernel, Nicolas.Ferre,
alexandre.belloni, Ludovic.Desroches, linux-arm-kernel,
devicetree, thomas.petazzoni
On Fri, Oct 04, 2019 at 09:35:23AM +0000, Claudiu.Beznea@microchip.com wrote:
> Hi Kamel,
>
> On 02.10.2019 17:46, Kamel Bouhara wrote:
> > +static int at91_init_twi_recovery_info(struct platform_device *pdev,
> > + struct at91_twi_dev *dev)
> > +{
> > + struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
> > +
> > + dev->pinctrl = devm_pinctrl_get(&pdev->dev);
> > + if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
>
> You may use IS_ERR_OR_NULL() here.
Can devm_pinctrl_get return NULL? From a quick look, it cannot.
rule of thumb: IS_ERR_OR_NULL is wrong as it is a sign of poor return
value semantics.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-04 20:39 ` Uwe Kleine-König
@ 2019-10-07 10:17 ` Claudiu.Beznea
0 siblings, 0 replies; 18+ messages in thread
From: Claudiu.Beznea @ 2019-10-07 10:17 UTC (permalink / raw)
To: u.kleine-koenig
Cc: kamel.bouhara, wsa, linux-i2c, linux-kernel, Nicolas.Ferre,
alexandre.belloni, Ludovic.Desroches, linux-arm-kernel,
devicetree, thomas.petazzoni
On 04.10.2019 23:39, Uwe Kleine-König wrote:
> External E-Mail
>
>
> On Fri, Oct 04, 2019 at 09:35:23AM +0000, Claudiu.Beznea@microchip.com wrote:
>> Hi Kamel,
>>
>> On 02.10.2019 17:46, Kamel Bouhara wrote:
>>> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
>>> + struct at91_twi_dev *dev)
>>> +{
>>> + struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
>>> +
>>> + dev->pinctrl = devm_pinctrl_get(&pdev->dev);
>>> + if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
>>
>> You may use IS_ERR_OR_NULL() here.
>
> Can devm_pinctrl_get return NULL? From a quick look, it cannot.
Looking quickly though it, yes, it seems it can't.
>
> rule of thumb: IS_ERR_OR_NULL is wrong as it is a sign of poor return
> value semantics.
>
> Best regards
> Uwe
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-02 14:46 ` [PATCH 2/4] i2c: at91: implement i2c bus recovery Kamel Bouhara
2019-10-04 9:35 ` Claudiu.Beznea
@ 2019-10-09 13:55 ` Ludovic Desroches
2019-10-09 14:01 ` Alexandre Belloni
2019-10-21 20:20 ` Wolfram Sang
2 siblings, 1 reply; 18+ messages in thread
From: Ludovic Desroches @ 2019-10-09 13:55 UTC (permalink / raw)
To: Kamel Bouhara
Cc: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, linux-arm-kernel, devicetree,
Thomas Petazzoni
On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> External E-Mail
>
>
> Implement i2c bus recovery when slaves devices might hold SDA low.
> In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> until the slave release SDA.
>
Hi Kamel,
Thanks for adding this new feature. As I see patches only for sama5d3 and
sama5d4, I assume it has not been tested with a sama5d2, isn't it?
I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it can
work if we add .strict = true to pinmux_ops which is something plan for the
future...
Are you able to test these points? It would be nice to be aware of
possible side effects.
Regards
Ludovic
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
> drivers/i2c/busses/i2c-at91-master.c | 63 ++++++++++++++++++++++++++++
> drivers/i2c/busses/i2c-at91.h | 8 ++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index a3fcc35ffd3b..df5bb93f952d 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -18,11 +18,13 @@
> #include <linux/dma-mapping.h>
> #include <linux/dmaengine.h>
> #include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/dma-atmel.h>
> #include <linux/pm_runtime.h>
> @@ -768,6 +770,63 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
> return ret;
> }
>
> +static void at91_prepare_twi_recovery(struct i2c_adapter *adap)
> +{
> + struct at91_twi_dev *dev = i2c_get_adapdata(adap);
> +
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_gpio);
> +}
> +
> +static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
> +{
> + struct at91_twi_dev *dev = i2c_get_adapdata(adap);
> +
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
> +}
> +
> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
> + struct at91_twi_dev *dev)
> +{
> + struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
> +
> + dev->pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
> + dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
> + return PTR_ERR(dev->pinctrl);
> + }
> +
> + dev->pinctrl_pins_default = pinctrl_lookup_state(dev->pinctrl,
> + PINCTRL_STATE_DEFAULT);
> + dev->pinctrl_pins_gpio = pinctrl_lookup_state(dev->pinctrl,
> + "gpio");
> + rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
> + if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl",
> + GPIOD_OUT_HIGH_OPEN_DRAIN);
> + if (PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (IS_ERR(rinfo->sda_gpiod) ||
> + IS_ERR(rinfo->scl_gpiod) ||
> + IS_ERR(dev->pinctrl_pins_default) ||
> + IS_ERR(dev->pinctrl_pins_gpio)) {
> + dev_info(&pdev->dev, "recovery information incomplete\n");
> + return -EINVAL;
> + }
> +
> + dev_info(&pdev->dev, "using scl%s for recovery\n",
> + rinfo->sda_gpiod ? ",sda" : "");
> +
> + rinfo->prepare_recovery = at91_prepare_twi_recovery;
> + rinfo->unprepare_recovery = at91_unprepare_twi_recovery;
> + rinfo->recover_bus = i2c_generic_scl_recovery;
> + dev->adapter.bus_recovery_info = rinfo;
> +
> + return 0;
> +}
> +
> int at91_twi_probe_master(struct platform_device *pdev,
> u32 phy_addr, struct at91_twi_dev *dev)
> {
> @@ -795,6 +854,10 @@ int at91_twi_probe_master(struct platform_device *pdev,
>
> at91_calc_twi_clock(dev);
>
> + rc = at91_init_twi_recovery_info(pdev, dev);
> + if (rc == -EPROBE_DEFER)
> + return rc;
> +
> dev->adapter.algo = &at91_twi_algorithm;
> dev->adapter.quirks = &at91_twi_quirks;
>
> diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
> index 499b506f6128..b89dab55e776 100644
> --- a/drivers/i2c/busses/i2c-at91.h
> +++ b/drivers/i2c/busses/i2c-at91.h
> @@ -141,6 +141,10 @@ struct at91_twi_dev {
> u32 fifo_size;
> struct at91_twi_dma dma;
> bool slave_detected;
> + struct i2c_bus_recovery_info rinfo;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pinctrl_pins_default;
> + struct pinctrl_state *pinctrl_pins_gpio;
> #ifdef CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL
> unsigned smr;
> struct i2c_client *slave;
> @@ -158,6 +162,10 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev);
> int at91_twi_probe_master(struct platform_device *pdev, u32 phy_addr,
> struct at91_twi_dev *dev);
>
> +void at91_twi_prepare_recovery(struct i2c_adapter *adap);
> +void at91_twi_unprepare_recovery(struct i2c_adapter *adap);
> +void at91_twi_init_recovery_info(struct at91_twi_dev *dev);
> +
> #ifdef CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL
> void at91_init_twi_bus_slave(struct at91_twi_dev *dev);
> int at91_twi_probe_slave(struct platform_device *pdev, u32 phy_addr,
> --
> 2.23.0
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-09 13:55 ` Ludovic Desroches
@ 2019-10-09 14:01 ` Alexandre Belloni
2019-10-10 6:54 ` Ludovic Desroches
0 siblings, 1 reply; 18+ messages in thread
From: Alexandre Belloni @ 2019-10-09 14:01 UTC (permalink / raw)
To: Kamel Bouhara, Wolfram Sang, linux-i2c, linux-kernel,
Nicolas Ferre, linux-arm-kernel, devicetree, Thomas Petazzoni
On 09/10/2019 15:55:00+0200, Ludovic Desroches wrote:
> On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> > External E-Mail
> >
> >
> > Implement i2c bus recovery when slaves devices might hold SDA low.
> > In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> > until the slave release SDA.
> >
>
> Hi Kamel,
>
> Thanks for adding this new feature. As I see patches only for sama5d3 and
> sama5d4, I assume it has not been tested with a sama5d2, isn't it?
>
I there a point having it on sama5d2 as the controller already supports
this feature?
> I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it can
> work if we add .strict = true to pinmux_ops which is something plan for the
> future...
>
I don't see why it wouldn't work with strict as this is switching muxing
properly instead of using the pins for two functions at the same time.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-09 14:01 ` Alexandre Belloni
@ 2019-10-10 6:54 ` Ludovic Desroches
0 siblings, 0 replies; 18+ messages in thread
From: Ludovic Desroches @ 2019-10-10 6:54 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Kamel Bouhara, Wolfram Sang, linux-i2c, linux-kernel,
Nicolas Ferre, linux-arm-kernel, devicetree, Thomas Petazzoni
On Wed, Oct 09, 2019 at 04:01:47PM +0200, Alexandre Belloni wrote:
>
> On 09/10/2019 15:55:00+0200, Ludovic Desroches wrote:
> > On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> > > External E-Mail
> > >
> > >
> > > Implement i2c bus recovery when slaves devices might hold SDA low.
> > > In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> > > until the slave release SDA.
> > >
> >
> > Hi Kamel,
> >
> > Thanks for adding this new feature. As I see patches only for sama5d3 and
> > sama5d4, I assume it has not been tested with a sama5d2, isn't it?
> >
>
> I there a point having it on sama5d2 as the controller already supports
> this feature?
>
Right, I was focused on pinctrl and forget we have this feature
supported by the IP.
> > I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it can
> > work if we add .strict = true to pinmux_ops which is something plan for the
> > future...
> >
>
> I don't see why it wouldn't work with strict as this is switching muxing
> properly instead of using the pins for two functions at the same time.
>
Not sure devm_gpiod_get won't fail with strict.
Ludovic
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
2019-10-02 14:46 ` [PATCH 2/4] i2c: at91: implement i2c bus recovery Kamel Bouhara
2019-10-04 9:35 ` Claudiu.Beznea
2019-10-09 13:55 ` Ludovic Desroches
@ 2019-10-21 20:20 ` Wolfram Sang
[not found] ` <724d3470-0561-1b3f-c826-bc16c74a8c0a@bootlin.com>
2 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2019-10-21 20:20 UTC (permalink / raw)
To: Kamel Bouhara
Cc: linux-i2c, linux-kernel, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches, linux-arm-kernel, devicetree,
Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 894 bytes --]
On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> Implement i2c bus recovery when slaves devices might hold SDA low.
> In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> until the slave release SDA.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Setting up the bus_recovery looks OK. However, I don't see any call to
i2c_recover_bus(), so the bus_recovery is never used. Did you test this
and see an effect?
Also, I think we should merge this patch "[PATCH v3] i2c: at91: Send bus
clear command if SCL or SDA is down" into this series. The crucial thing
for both is when to apply the recovery (at the beginning of a
transfer!). The rest is "just" that some HW needs a bus_recovery_info
for pinctrl/GPIO handling (from this patch), while other HW needs a
bus_recovery_info with a custom recover_bus callback.
Opinions?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl
2019-10-02 14:46 [PATCH 0/4] i2c bus recovery for Microchip SoCs Kamel Bouhara
2019-10-02 14:46 ` [PATCH 1/4] dt-bindings: i2c: at91: document optional bus recovery properties Kamel Bouhara
2019-10-02 14:46 ` [PATCH 2/4] i2c: at91: implement i2c bus recovery Kamel Bouhara
@ 2019-10-02 14:46 ` Kamel Bouhara
2019-10-02 14:46 ` [PATCH 4/4] ARM: at91/dt: sama5d4: " Kamel Bouhara
2019-10-15 19:10 ` [PATCH 0/4] i2c bus recovery for Microchip SoCs Rob Herring
4 siblings, 0 replies; 18+ messages in thread
From: Kamel Bouhara @ 2019-10-02 14:46 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel
Cc: devicetree, Thomas Petazzoni, Kamel Bouhara
Add the i2c gpio pinctrls to support the i2c bus recovery
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
arch/arm/boot/dts/sama5d3.dtsi | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index f770aace0efd..faf8907d8d7d 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -159,8 +159,11 @@
dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(7)>,
<&dma0 2 AT91_DMA_CFG_PER_ID(8)>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c0>;
+ pinctrl-1 = <&pinctrl_i2c0_gpio>;
+ sda-gpios = <&pioA 30 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioA 31 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&twi0_clk>;
@@ -174,8 +177,11 @@
dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(9)>,
<&dma0 2 AT91_DMA_CFG_PER_ID(10)>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c1>;
+ pinctrl-1 = <&pinctrl_i2c1_gpio>;
+ sda-gpios = <&pioC 26 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioC 27 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&twi1_clk>;
@@ -357,8 +363,11 @@
dmas = <&dma1 2 AT91_DMA_CFG_PER_ID(11)>,
<&dma1 2 AT91_DMA_CFG_PER_ID(12)>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c2>;
+ pinctrl-1 = <&pinctrl_i2c2_gpio>;
+ sda-gpios = <&pioA 18 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioA 19 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&twi2_clk>;
@@ -639,6 +648,12 @@
<AT91_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_NONE /* PA30 periph A TWD0 pin, conflicts with URXD1, ISI_VSYNC */
AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* PA31 periph A TWCK0 pin, conflicts with UTXD1, ISI_HSYNC */
};
+
+ pinctrl_i2c0_gpio: i2c0-gpio {
+ atmel,pins =
+ <AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
i2c1 {
@@ -647,6 +662,12 @@
<AT91_PIOC 26 AT91_PERIPH_B AT91_PINCTRL_NONE /* PC26 periph B TWD1 pin, conflicts with SPI1_NPCS1, ISI_D11 */
AT91_PIOC 27 AT91_PERIPH_B AT91_PINCTRL_NONE>; /* PC27 periph B TWCK1 pin, conflicts with SPI1_NPCS2, ISI_D10 */
};
+
+ pinctrl_i2c1_gpio: i2c1-gpio {
+ atmel,pins =
+ <AT91_PIOC 26 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOC 27 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
i2c2 {
@@ -655,6 +676,12 @@
<AT91_PIOA 18 AT91_PERIPH_B AT91_PINCTRL_NONE /* TWD2 pin, conflicts with LCDDAT18, ISI_D2 */
AT91_PIOA 19 AT91_PERIPH_B AT91_PINCTRL_NONE>; /* TWCK2 pin, conflicts with LCDDAT19, ISI_D3 */
};
+
+ pinctrl_i2c2_gpio: i2c2-gpio {
+ atmel,pins =
+ <AT91_PIOA 18 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOA 19 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
isi {
--
2.23.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] ARM: at91/dt: sama5d4: add i2c gpio pinctrl
2019-10-02 14:46 [PATCH 0/4] i2c bus recovery for Microchip SoCs Kamel Bouhara
` (2 preceding siblings ...)
2019-10-02 14:46 ` [PATCH 3/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Kamel Bouhara
@ 2019-10-02 14:46 ` Kamel Bouhara
2019-10-15 19:10 ` [PATCH 0/4] i2c bus recovery for Microchip SoCs Rob Herring
4 siblings, 0 replies; 18+ messages in thread
From: Kamel Bouhara @ 2019-10-02 14:46 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel
Cc: devicetree, Thomas Petazzoni, Kamel Bouhara
Add the i2c gpio pinctrls so the i2c bus recovery option can be enabled
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
arch/arm/boot/dts/sama5d4.dtsi | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 6ab27a7b388d..34351baab985 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -458,8 +458,11 @@
(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
| AT91_XDMAC_DT_PERID(3))>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c0>;
+ pinctrl-1 = <&pinctrl_i2c0_gpio>;
+ sda-gpios = <&pioA 30 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioA 31 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&pmc PMC_TYPE_PERIPHERAL 32>;
@@ -477,8 +480,11 @@
(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
| AT91_XDMAC_DT_PERID(5))>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c1>;
+ pinctrl-1 = <&pinctrl_i2c1_gpio>;
+ sda-gpios = <&pioE 29 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioE 30 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&pmc PMC_TYPE_PERIPHERAL 33>;
@@ -519,8 +525,11 @@
(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
| AT91_XDMAC_DT_PERID(7))>;
dma-names = "tx", "rx";
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c2>;
+ pinctrl-1 = <&pinctrl_i2c2_gpio>;
+ sda-gpios = <&pioB 29 GPIO_ACTIVE_HIGH>;
+ scl-gpios = <&pioB 30 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
clocks = <&pmc PMC_TYPE_PERIPHERAL 34>;
@@ -1122,6 +1131,12 @@
<AT91_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_NONE
AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_NONE>;
};
+
+ pinctrl_i2c0_gpio: i2c0-gpio {
+ atmel,pins =
+ <AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
i2c1 {
@@ -1130,6 +1145,12 @@
<AT91_PIOE 29 AT91_PERIPH_C AT91_PINCTRL_NONE /* TWD1, conflicts with UART0 RX and DIBP */
AT91_PIOE 30 AT91_PERIPH_C AT91_PINCTRL_NONE>; /* TWCK1, conflicts with UART0 TX and DIBN */
};
+
+ pinctrl_i2c1_gpio: i2c1-gpio {
+ atmel,pins =
+ <AT91_PIOE 29 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOE 30 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
i2c2 {
@@ -1138,6 +1159,12 @@
<AT91_PIOB 29 AT91_PERIPH_A AT91_PINCTRL_NONE /* TWD2, conflicts with RD0 and PWML1 */
AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* TWCK2, conflicts with RF0 */
};
+
+ pinctrl_i2c2_gpio: i2c2-gpio {
+ atmel,pins =
+ <AT91_PIOB 29 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
+ AT91_PIOB 30 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
+ };
};
isi {
--
2.23.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] i2c bus recovery for Microchip SoCs.
2019-10-02 14:46 [PATCH 0/4] i2c bus recovery for Microchip SoCs Kamel Bouhara
` (3 preceding siblings ...)
2019-10-02 14:46 ` [PATCH 4/4] ARM: at91/dt: sama5d4: " Kamel Bouhara
@ 2019-10-15 19:10 ` Rob Herring
4 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2019-10-15 19:10 UTC (permalink / raw)
To: Kamel Bouhara
Cc: Wolfram Sang, linux-i2c, linux-kernel, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel,
devicetree, Thomas Petazzoni
On Wed, Oct 02, 2019 at 04:46:54PM +0200, Kamel Bouhara wrote:
> This patch series introduce the kernel i2c-gpio bus recovery mechanism
> for the Microchip SoCs. Updated the corresponding dts to add i2c
> gpio pinctrl. The bus recovery is configured for the sama5d3/4 xplained
> boards in dts.
Now we have 2 drivers with the same binding and code for using GPIO for
bus recovery. Perhaps all this should be common.
Rob
^ permalink raw reply [flat|nested] 18+ messages in thread