Re: [PATCH 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer"
On Wed, Nov 18, 2015 at 3:31 PM, Lars-Peter Clausen wrote:
> On 11/17/2015 03:28 PM, Shubhrajyoti Datta wrote:
>> On Tue, Nov 17, 2015 at 1:04 PM, Lars-Peter Clausen wrote:
>>> On 11/17/2015 06:17 AM, Shubhrajyoti Datta wrote:
On Mon, Nov 16, 2015 at 7:12 PM, Lars-Peter Clausen
wrote:
> Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
> transfer") removed the reinitialization of the controller before the start
> of each transfer. Apparently this change is not safe to make and the
> commit
> results in random I2C bus failures.
Which is the platform and the ip version that you saw the issue.
Did you see the issue with read and write as well?
>>>
>>> The IP version is the axi-iic v2.0 Revision 8. I've tested this on a few
>>> platforms, custom ones and standard ones and I could reproduce it on most.
>>> One of them was on the ZED board. The one where I couldn't reproduce it was
>>> the ZC706. But that doesn't necessarily mean it doesn't happen there, just
>>> that it is not triggered by the testcase.
>> All the boards having the same version of the ip is what I have understood.
>>
>> Thanks for the info I will try to reproduce the issue.
>>
>>>
>>> The problem is that it is random corruption,
>> Of registers?
>
> To be honest I don't know if there is corruption during I2C write transfers,
> but there is definitely corruption during read transactions.
Ok
Actually I was thinking it is only an issue with
/* dynamic mode seem to suffer from problems if we just flushes
* fifos and the next message is a TX with len 0 (only addr)
* reset the IP instead of just flush fifos
*/
xiic_reinit(i2c);
<\quote>
I think as of now since read is also impacted we can revert it.
> - Lars
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: imx: make bus recovery through pinctrl optional
On Wed, Nov 18, 2015 at 1:24 AM, Uwe Kleine-König
wrote:
> Hello,
>
> On Tue, Nov 17, 2015 at 06:02:59PM -0600, Li Yang wrote:
>> Since commit 1c4b6c3bcf30 ("i2c: imx: implement bus recovery") the driver
>> starts to use gpio/pinctrl to do i2c bus recovery. But pinctrl is not
>> always available for platforms using this driver such as ls1021a and
>> ls1043a, and the device tree binding also mentioned this gpio based
>> recovery mechanism as optional. The patch make it really optional that
>> the probe function won't bailout when pinctrl is not available and it
>> won't try to register recovery functions if pinctrl is NULL when the
>> PINCTRL is not enabled at all.
>>
>> Signed-off-by: Li Yang
>> Cc: Gao Pan
>> ---
>> drivers/i2c/busses/i2c-imx.c | 13 ++---
>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
>> index 1e4d99d..7813153 100644
>> --- a/drivers/i2c/busses/i2c-imx.c
>> +++ b/drivers/i2c/busses/i2c-imx.c
>> @@ -1086,12 +1086,6 @@ static int i2c_imx_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>> - if (IS_ERR(i2c_imx->pinctrl)) {
>> - ret = PTR_ERR(i2c_imx->pinctrl);
>> - goto clk_disable;
>> - }
>> -
>> /* Request IRQ */
>> ret = devm_request_irq(&pdev->dev, irq, i2c_imx_isr, 0,
>> pdev->name, i2c_imx);
>> @@ -1125,7 +1119,12 @@ static int i2c_imx_probe(struct platform_device *pdev)
>> goto clk_disable;
>> }
>>
>> - i2c_imx_init_recovery_info(i2c_imx, pdev);
>> + /* optional bus recovery feature through pinctrl */
>> + i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>> + if (IS_ERR_OR_NULL(i2c_imx->pinctrl))
>> + dev_info(&pdev->dev, "can't get pinctrl, bus recovery feature
>> disabled\n");
>> + else
>> + i2c_imx_init_recovery_info(i2c_imx, pdev);
>
> I'm pretty sure this is wrong. If pinctrl isn't available
> devm_pinctrl_get returns NULL? But AFAIK you must not ignore an error,
> so the better thing to do is:
If CONFIG_PINCTRL is not enabled, the devm_pinctrl_get() will return
NULL directly as defined in the include/linux/pinctrl/consumer.h.
If CONFIG_PINCTRL is enabled because we are using a multi-platform
image but the actual hardware used doesn't have a pinctrl driver or
pinctrl device tree nodes. It is expected that the devm_pinctrl_get()
will return error. But as the pinctrl is only used for bus recovery
which is just an optional function of this driver. We shouldn't
bailout the probe but keep the driver working without the bus recovery
function. As for generic errors like (!dev) or out-of-memory, the
probe will fail elsewhere anyway.
Regards,
Leo
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
On Wed, Nov 18, 2015 at 2:55 PM, LABBE Corentin
wrote:
> The simple_strtoul function is marked as obsolete.
> This patch replace it by kstrtou8.
>
Only one concern. simple_strto* goes through the string until it has
an invalid character or \0. In your case kstrtou8 will fail the
transfer. So, is there possible cases when HW returns such data?
And just a style nitpicks below.
> if (p[0] == 'x') {
> - data->byte = simple_strtol(p + 1, NULL, 16);
> + /*
> +* voluntarily dropping error code of kstrtou8 since
> all
-> Voluntarily…
> +* error code that it could return are invalid
> according
> +* to Documentation/i2c/fault-codes
-> …codes.
> +*/
> + if (kstrtou8(p + 1, 16, &data->byte))
> + return -EPROTO;
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
The simple_strtoul function is marked as obsolete.
This patch replace it by kstrtou8.
Signed-off-by: LABBE Corentin
---
drivers/i2c/busses/i2c-taos-evm.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-taos-evm.c
b/drivers/i2c/busses/i2c-taos-evm.c
index 4c7fc2d..f673f5d 100644
--- a/drivers/i2c/busses/i2c-taos-evm.c
+++ b/drivers/i2c/busses/i2c-taos-evm.c
@@ -130,7 +130,13 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter,
u16 addr,
return 0;
} else {
if (p[0] == 'x') {
- data->byte = simple_strtol(p + 1, NULL, 16);
+ /*
+* voluntarily dropping error code of kstrtou8 since all
+* error code that it could return are invalid according
+* to Documentation/i2c/fault-codes
+*/
+ if (kstrtou8(p + 1, 16, &data->byte))
+ return -EPROTO;
return 0;
}
}
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
Hello Changes since v2 - removed err variable - fix a spelling issue Changes since v1 - drop the return code of kstrtou8 and return -EPROTO as suggested by Jean Delvare - Added a comment on the return code drop Regards LABBE Corentin (1): i2c: taos-evm: replace simple_strtoul by kstrtou8 drivers/i2c/busses/i2c-taos-evm.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i2c slave support für i.mx6
Hi, > thanks to your comments in the rcar driver, I understand the function > of each register. But I'd like a manual of this board. Where is > accurately entered into the individual registers. As far as I know, there is no public user manual for RCar SoCs. I can ask, though. > Had someone for me a manual by the individual registers are explained. > So that I can understand the rcar driver better. However, if you have a specific question, you could ask and I could try improving the driver's documentation :) Regards, Wolfram signature.asc Description: Digital signature
Re: [PATCH 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer"
On 11/17/2015 03:28 PM, Shubhrajyoti Datta wrote:
> On Tue, Nov 17, 2015 at 1:04 PM, Lars-Peter Clausen wrote:
>> On 11/17/2015 06:17 AM, Shubhrajyoti Datta wrote:
>>> On Mon, Nov 16, 2015 at 7:12 PM, Lars-Peter Clausen wrote:
Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
transfer") removed the reinitialization of the controller before the start
of each transfer. Apparently this change is not safe to make and the commit
results in random I2C bus failures.
>>>
>>> Which is the platform and the ip version that you saw the issue.
>>> Did you see the issue with read and write as well?
>>
>> The IP version is the axi-iic v2.0 Revision 8. I've tested this on a few
>> platforms, custom ones and standard ones and I could reproduce it on most.
>> One of them was on the ZED board. The one where I couldn't reproduce it was
>> the ZC706. But that doesn't necessarily mean it doesn't happen there, just
>> that it is not triggered by the testcase.
> All the boards having the same version of the ip is what I have understood.
>
> Thanks for the info I will try to reproduce the issue.
>
>>
>> The problem is that it is random corruption,
> Of registers?
To be honest I don't know if there is corruption during I2C write transfers,
but there is definitely corruption during read transactions.
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
On Wed, Nov 18, 2015 at 10:29:54AM +0100, Jean Delvare wrote:
> Hi Corentin,
>
> On Wed, 18 Nov 2015 09:00:53 +0100, LABBE Corentin wrote:
> > The simple_strtoul function is marked as obsolete.
> > This patch replace it by kstrtou8.
> >
> > Signed-off-by: LABBE Corentin
> > ---
> > drivers/i2c/busses/i2c-taos-evm.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-taos-evm.c
> > b/drivers/i2c/busses/i2c-taos-evm.c
> > index 4c7fc2d..552f127 100644
> > --- a/drivers/i2c/busses/i2c-taos-evm.c
> > +++ b/drivers/i2c/busses/i2c-taos-evm.c
> > @@ -70,6 +70,7 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter,
> > u16 addr,
> > struct serio *serio = adapter->algo_data;
> > struct taos_data *taos = serio_get_drvdata(serio);
> > char *p;
> > + int err;
> >
> > /* Encode our transaction. "@" is for the device address, "$" for the
> >SMBus command and "#" for the data. */
> > @@ -130,7 +131,14 @@ static int taos_smbus_xfer(struct i2c_adapter
> > *adapter, u16 addr,
> > return 0;
> > } else {
> > if (p[0] == 'x') {
> > - data->byte = simple_strtol(p + 1, NULL, 16);
> > + /*
> > +* voluntary dropping error code of kstrtou8 since all
>
> "Voluntarily" or "Willingly" would be more correct.
>
> > +* error code that it could return are invalid according
> > +* to Documentation/i2c/fault-codes
> > +*/
> > + err = kstrtou8(p + 1, 16, &data->byte);
> > + if (err)
> > + return -EPROTO;
> > return 0;
> > }
> > }
>
> Thanks for the patch. Note that you don't strictly need the "err"
> variable as you never use its value.
>
> Reviewed-by: Jean Delvare
> Tested-by: Jean Delvare
>
Thanks
I will send a v3 without the err and the spell fix.
Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
Hi Corentin,
On Wed, 18 Nov 2015 09:00:53 +0100, LABBE Corentin wrote:
> The simple_strtoul function is marked as obsolete.
> This patch replace it by kstrtou8.
>
> Signed-off-by: LABBE Corentin
> ---
> drivers/i2c/busses/i2c-taos-evm.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-taos-evm.c
> b/drivers/i2c/busses/i2c-taos-evm.c
> index 4c7fc2d..552f127 100644
> --- a/drivers/i2c/busses/i2c-taos-evm.c
> +++ b/drivers/i2c/busses/i2c-taos-evm.c
> @@ -70,6 +70,7 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter, u16
> addr,
> struct serio *serio = adapter->algo_data;
> struct taos_data *taos = serio_get_drvdata(serio);
> char *p;
> + int err;
>
> /* Encode our transaction. "@" is for the device address, "$" for the
> SMBus command and "#" for the data. */
> @@ -130,7 +131,14 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter,
> u16 addr,
> return 0;
> } else {
> if (p[0] == 'x') {
> - data->byte = simple_strtol(p + 1, NULL, 16);
> + /*
> + * voluntary dropping error code of kstrtou8 since all
"Voluntarily" or "Willingly" would be more correct.
> + * error code that it could return are invalid according
> + * to Documentation/i2c/fault-codes
> + */
> + err = kstrtou8(p + 1, 16, &data->byte);
> + if (err)
> + return -EPROTO;
> return 0;
> }
> }
Thanks for the patch. Note that you don't strictly need the "err"
variable as you never use its value.
Reviewed-by: Jean Delvare
Tested-by: Jean Delvare
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/10] i2c: rcar: refactor setup of a msg
Hi Wolfram, On Wed, Nov 18, 2015 at 5:07 PM, Wolfram Sang wrote: > >> On Lager it seems that the GP5_5/GP_6 pins with the I2C2 bus for the >> ADV7511 chip has more flexible configuration, so using either IIC or >> I2C should be possible. You can sample those pins on EXIO_A with the >> ZEBAX break out adapter. > > Yes, that's what I basically did here for developing the whole patch > series and for the recent testing. A simple 's/iic2/i2c2/g' on the Lager > DTS does the trick. Great, thanks! Lager seems to work well, but I wonder why Koelsch is unstable... / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/10] i2c: rcar: refactor setup of a msg
> On Lager it seems that the GP5_5/GP_6 pins with the I2C2 bus for the > ADV7511 chip has more flexible configuration, so using either IIC or > I2C should be possible. You can sample those pins on EXIO_A with the > ZEBAX break out adapter. Yes, that's what I basically did here for developing the whole patch series and for the recent testing. A simple 's/iic2/i2c2/g' on the Lager DTS does the trick. signature.asc Description: Digital signature
[PATCH v2 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
The simple_strtoul function is marked as obsolete.
This patch replace it by kstrtou8.
Signed-off-by: LABBE Corentin
---
drivers/i2c/busses/i2c-taos-evm.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-taos-evm.c
b/drivers/i2c/busses/i2c-taos-evm.c
index 4c7fc2d..552f127 100644
--- a/drivers/i2c/busses/i2c-taos-evm.c
+++ b/drivers/i2c/busses/i2c-taos-evm.c
@@ -70,6 +70,7 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter, u16
addr,
struct serio *serio = adapter->algo_data;
struct taos_data *taos = serio_get_drvdata(serio);
char *p;
+ int err;
/* Encode our transaction. "@" is for the device address, "$" for the
SMBus command and "#" for the data. */
@@ -130,7 +131,14 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter,
u16 addr,
return 0;
} else {
if (p[0] == 'x') {
- data->byte = simple_strtol(p + 1, NULL, 16);
+ /*
+* voluntary dropping error code of kstrtou8 since all
+* error code that it could return are invalid according
+* to Documentation/i2c/fault-codes
+*/
+ err = kstrtou8(p + 1, 16, &data->byte);
+ if (err)
+ return -EPROTO;
return 0;
}
}
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
