[PATCH] i2c-designware: increase timeout of semaphore transfer

[PATCH] i2c-designware: increase timeout of semaphore transfer

[Oliver Neukum]

Our testing shows the semaphore failing to be transferred on CherryTrail
in about 0.5% of all cases. The existing timeout needs to be lengthened
to accomodate the worst cases.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 1590ad0..2d238e3 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -21,7 +21,7 @@
 
 #include "i2c-designware-core.h"
 
-#define SEMAPHORE_TIMEOUT	100
+#define SEMAPHORE_TIMEOUT	500
 #define PUNIT_SEMAPHORE		0x7
 #define PUNIT_SEMAPHORE_BIT	BIT(0)
 #define PUNIT_SEMAPHORE_ACQUIRE	BIT(1)
-- 
2.10.2

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Andy Shevchenko]

On Thu, 2017-03-16 at 17:24 +0100, Oliver Neukum wrote:
> Our testing shows the semaphore failing to be transferred on
> CherryTrail
> in about 0.5% of all cases. The existing timeout needs to be
> lengthened
> to accomodate the worst cases.

I think it's just a semi-hiding of real issue.

Hans?

> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> b/drivers/i2c/busses/i2c-designware-baytrail.c
> index 1590ad0..2d238e3 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -21,7 +21,7 @@
>  
>  #include "i2c-designware-core.h"
>  
> -#define SEMAPHORE_TIMEOUT	100
> +#define SEMAPHORE_TIMEOUT	500
>  #define PUNIT_SEMAPHORE		0x7
>  #define PUNIT_SEMAPHORE_BIT	BIT(0)
>  #define PUNIT_SEMAPHORE_ACQUIRE	BIT(1)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Hans de Goede]

Hi,

On 16-03-17 17:35, Andy Shevchenko wrote:
> On Thu, 2017-03-16 at 17:24 +0100, Oliver Neukum wrote:
>> Our testing shows the semaphore failing to be transferred on
>> CherryTrail
>> in about 0.5% of all cases. The existing timeout needs to be
>> lengthened
>> to accomodate the worst cases.
>
> I think it's just a semi-hiding of real issue.

I would not know what real issue, I've seen the timeout being
hit sometimes too, it seems that sometimes the punit just is
busy with other stuff for a while before acking the semaphore
request, so I think this patch is fine:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Oliver, can you rebase this on top of:


https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail

Which is in -next now, please ? I'm sorta surprised you didn't
use this is a base since your commit msg mentions cherrytrail
and with that series the semaphore code is broken on
cherrytrail.

Regards,

Hans



>
> Hans?
>
>>
>> Signed-off-by: Oliver Neukum <oneukum@suse.com>
>> ---
>>  drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
>> b/drivers/i2c/busses/i2c-designware-baytrail.c
>> index 1590ad0..2d238e3 100644
>> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
>> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
>> @@ -21,7 +21,7 @@
>>
>>  #include "i2c-designware-core.h"
>>
>> -#define SEMAPHORE_TIMEOUT	100
>> +#define SEMAPHORE_TIMEOUT	500
>>  #define PUNIT_SEMAPHORE		0x7
>>  #define PUNIT_SEMAPHORE_BIT	BIT(0)
>>  #define PUNIT_SEMAPHORE_ACQUIRE	BIT(1)
>

[PATCH] i2c-designware: increase timeout of semaphore transfer

[Oliver Neukum]

Our testing shows the semaphore failing to be transferred on CherryTrail
in about 0.5% of all cases. The existing timeout needs to be lengthened
to accomodate the worst cases.

V2: Rebased on https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 1749a0f..c0e7c88 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -22,7 +22,7 @@
 
 #include "i2c-designware-core.h"
 
-#define SEMAPHORE_TIMEOUT	100
+#define SEMAPHORE_TIMEOUT	500
 #define PUNIT_SEMAPHORE		0x7
 #define PUNIT_SEMAPHORE_CHT	0x10e
 #define PUNIT_SEMAPHORE_BIT	BIT(0)
-- 
2.10.2

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Oliver Neukum]

Am Freitag, den 17.03.2017, 08:35 +0100 schrieb Hans de Goede:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Oliver, can you rebase this on top of:
> 
> 
> https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail
> 
> Which is in -next now, please ? I'm sorta surprised you didn't
> use this is a base since your commit msg mentions cherrytrail
> and with that series the semaphore code is broken on
> cherrytrail.

Done. I used Linus' tree specifically because it is kind of canonical.
The patch is taken from an internal tree which had your patches.

	Regards
		Oliver

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Hans de Goede]

Hi,

On 20-03-17 14:37, Oliver Neukum wrote:
> Our testing shows the semaphore failing to be transferred on CherryTrail
> in about 0.5% of all cases. The existing timeout needs to be lengthened
> to accomodate the worst cases.
>
> V2: Rebased on https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>

Looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
> index 1749a0f..c0e7c88 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -22,7 +22,7 @@
>
>  #include "i2c-designware-core.h"
>
> -#define SEMAPHORE_TIMEOUT	100
> +#define SEMAPHORE_TIMEOUT	500
>  #define PUNIT_SEMAPHORE		0x7
>  #define PUNIT_SEMAPHORE_CHT	0x10e
>  #define PUNIT_SEMAPHORE_BIT	BIT(0)
>

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Jarkko Nikula]

On 03/20/17 15:39, Hans de Goede wrote:
> Hi,
>
> On 20-03-17 14:37, Oliver Neukum wrote:
>> Our testing shows the semaphore failing to be transferred on CherryTrail
>> in about 0.5% of all cases. The existing timeout needs to be lengthened
>> to accomodate the worst cases.
>>
>> V2: Rebased on
>> https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail
>>
>>
>> Signed-off-by: Oliver Neukum <oneukum@suse.com>
>
> Looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH] i2c-designware: increase timeout of semaphore transfer

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On Mon, Mar 20, 2017 at 02:37:30PM +0100, Oliver Neukum wrote:
> Our testing shows the semaphore failing to be transferred on CherryTrail
> in about 0.5% of all cases. The existing timeout needs to be lengthened
> to accomodate the worst cases.
> 
> V2: Rebased on https://cgit.freedesktop.org/drm-intel/commit/?h=topic/designware-baytrail
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>

Fixed this checkpatch issue:

WARNING: 'accomodate' may be misspelled - perhaps 'accommodate'?

and applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]