* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
@ 2013-11-22 14:07 ludovic.desroches at atmel.com
2013-11-22 14:33 ` Wolfram Sang
2013-11-22 15:01 ` boris brezillon
0 siblings, 2 replies; 8+ messages in thread
From: ludovic.desroches at atmel.com @ 2013-11-22 14:07 UTC (permalink / raw)
To: linux-arm-kernel
From: Ludovic Desroches <ludovic.desroches@atmel.com>
There are still I2C unexpected behaviors which are solved by reducing TWI
internal frequency.
Cc: <stable@vger.kernel.org> #3.10+
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
arch/arm/mach-at91/sama5d3.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
index 4012797..4ee0de5 100644
--- a/arch/arm/mach-at91/sama5d3.c
+++ b/arch/arm/mach-at91/sama5d3.c
@@ -95,19 +95,19 @@ static struct clk twi0_clk = {
.name = "twi0_clk",
.pid = SAMA5D3_ID_TWI0,
.type = CLK_TYPE_PERIPHERAL,
- .div = AT91_PMC_PCR_DIV2,
+ .div = AT91_PMC_PCR_DIV8,
};
static struct clk twi1_clk = {
.name = "twi1_clk",
.pid = SAMA5D3_ID_TWI1,
.type = CLK_TYPE_PERIPHERAL,
- .div = AT91_PMC_PCR_DIV2,
+ .div = AT91_PMC_PCR_DIV8,
};
static struct clk twi2_clk = {
.name = "twi2_clk",
.pid = SAMA5D3_ID_TWI2,
.type = CLK_TYPE_PERIPHERAL,
- .div = AT91_PMC_PCR_DIV2,
+ .div = AT91_PMC_PCR_DIV8,
};
static struct clk mmc0_clk = {
.name = "mci0_clk",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 14:07 [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency ludovic.desroches at atmel.com
@ 2013-11-22 14:33 ` Wolfram Sang
2013-11-22 14:51 ` Ludovic Desroches
2013-11-22 15:01 ` boris brezillon
1 sibling, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2013-11-22 14:33 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 22, 2013 at 03:07:08PM +0100, ludovic.desroches at atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>
> There are still I2C unexpected behaviors which are solved by reducing TWI
> internal frequency.
>
> Cc: <stable@vger.kernel.org> #3.10+
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
I think the commit message needs more details. Is this a true bugfix
because the real bus frequency was too high because of the wrong
divider? Is this a workaround which makes things work but will make the
bus frequency slower than it should be?
> ---
> arch/arm/mach-at91/sama5d3.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
> index 4012797..4ee0de5 100644
> --- a/arch/arm/mach-at91/sama5d3.c
> +++ b/arch/arm/mach-at91/sama5d3.c
> @@ -95,19 +95,19 @@ static struct clk twi0_clk = {
> .name = "twi0_clk",
> .pid = SAMA5D3_ID_TWI0,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk twi1_clk = {
> .name = "twi1_clk",
> .pid = SAMA5D3_ID_TWI1,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk twi2_clk = {
> .name = "twi2_clk",
> .pid = SAMA5D3_ID_TWI2,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk mmc0_clk = {
> .name = "mci0_clk",
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131122/6a9cebe4/attachment.sig>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 14:33 ` Wolfram Sang
@ 2013-11-22 14:51 ` Ludovic Desroches
2013-11-22 14:58 ` Wolfram Sang
0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Desroches @ 2013-11-22 14:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi Wolfram,
On Fri, Nov 22, 2013 at 03:33:51PM +0100, Wolfram Sang wrote:
> On Fri, Nov 22, 2013 at 03:07:08PM +0100, ludovic.desroches at atmel.com wrote:
> > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> >
> > There are still I2C unexpected behaviors which are solved by reducing TWI
> > internal frequency.
> >
> > Cc: <stable@vger.kernel.org> #3.10+
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>
> I think the commit message needs more details. Is this a true bugfix
> because the real bus frequency was too high because of the wrong
> divider? Is this a workaround which makes things work but will make the
> bus frequency slower than it should be?
This fix doesn't concern the i2c bus frequency, only the internal IP frequency.
TWI has been validated at 66MHz. With some devices, transfer hangs during i2c
frame transmission. This issue disappears when reducing the internal frequency
of the IP. Maybe there is some oversampling on i2c signals.
Unfortunately, I have no clear status about the root cause that's why
the commit message was imprecise.
Regards
Ludovic
>
> > ---
> > arch/arm/mach-at91/sama5d3.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
> > index 4012797..4ee0de5 100644
> > --- a/arch/arm/mach-at91/sama5d3.c
> > +++ b/arch/arm/mach-at91/sama5d3.c
> > @@ -95,19 +95,19 @@ static struct clk twi0_clk = {
> > .name = "twi0_clk",
> > .pid = SAMA5D3_ID_TWI0,
> > .type = CLK_TYPE_PERIPHERAL,
> > - .div = AT91_PMC_PCR_DIV2,
> > + .div = AT91_PMC_PCR_DIV8,
> > };
> > static struct clk twi1_clk = {
> > .name = "twi1_clk",
> > .pid = SAMA5D3_ID_TWI1,
> > .type = CLK_TYPE_PERIPHERAL,
> > - .div = AT91_PMC_PCR_DIV2,
> > + .div = AT91_PMC_PCR_DIV8,
> > };
> > static struct clk twi2_clk = {
> > .name = "twi2_clk",
> > .pid = SAMA5D3_ID_TWI2,
> > .type = CLK_TYPE_PERIPHERAL,
> > - .div = AT91_PMC_PCR_DIV2,
> > + .div = AT91_PMC_PCR_DIV8,
> > };
> > static struct clk mmc0_clk = {
> > .name = "mci0_clk",
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 14:51 ` Ludovic Desroches
@ 2013-11-22 14:58 ` Wolfram Sang
2013-11-22 15:07 ` Ludovic Desroches
0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2013-11-22 14:58 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 22, 2013 at 03:51:41PM +0100, Ludovic Desroches wrote:
> Hi Wolfram,
>
> On Fri, Nov 22, 2013 at 03:33:51PM +0100, Wolfram Sang wrote:
> > On Fri, Nov 22, 2013 at 03:07:08PM +0100, ludovic.desroches at atmel.com wrote:
> > > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > >
> > > There are still I2C unexpected behaviors which are solved by reducing TWI
> > > internal frequency.
> > >
> > > Cc: <stable@vger.kernel.org> #3.10+
> > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> >
> > I think the commit message needs more details. Is this a true bugfix
> > because the real bus frequency was too high because of the wrong
> > divider? Is this a workaround which makes things work but will make the
> > bus frequency slower than it should be?
>
> This fix doesn't concern the i2c bus frequency, only the internal IP frequency.
>
> TWI has been validated at 66MHz. With some devices, transfer hangs during i2c
> frame transmission. This issue disappears when reducing the internal frequency
> of the IP. Maybe there is some oversampling on i2c signals.
> Unfortunately, I have no clear status about the root cause that's why
> the commit message was imprecise.
This paragraph is a way better commit message IMO :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131122/7a20bd79/attachment.sig>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 14:58 ` Wolfram Sang
@ 2013-11-22 15:07 ` Ludovic Desroches
2013-11-22 15:09 ` Nicolas Ferre
0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Desroches @ 2013-11-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 22, 2013 at 03:58:35PM +0100, Wolfram Sang wrote:
> On Fri, Nov 22, 2013 at 03:51:41PM +0100, Ludovic Desroches wrote:
> > Hi Wolfram,
> >
> > On Fri, Nov 22, 2013 at 03:33:51PM +0100, Wolfram Sang wrote:
> > > On Fri, Nov 22, 2013 at 03:07:08PM +0100, ludovic.desroches at atmel.com wrote:
> > > > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > >
> > > > There are still I2C unexpected behaviors which are solved by reducing TWI
> > > > internal frequency.
> > > >
> > > > Cc: <stable@vger.kernel.org> #3.10+
> > > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > >
> > > I think the commit message needs more details. Is this a true bugfix
> > > because the real bus frequency was too high because of the wrong
> > > divider? Is this a workaround which makes things work but will make the
> > > bus frequency slower than it should be?
> >
> > This fix doesn't concern the i2c bus frequency, only the internal IP frequency.
> >
> > TWI has been validated at 66MHz. With some devices, transfer hangs during i2c
> > frame transmission. This issue disappears when reducing the internal frequency
> > of the IP. Maybe there is some oversampling on i2c signals.
> > Unfortunately, I have no clear status about the root cause that's why
> > the commit message was imprecise.
>
> This paragraph is a way better commit message IMO :)
>
Ok I'll update it.
Thanks for the review Wolfram.
Regards
Ludovic
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 15:07 ` Ludovic Desroches
@ 2013-11-22 15:09 ` Nicolas Ferre
0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Ferre @ 2013-11-22 15:09 UTC (permalink / raw)
To: linux-arm-kernel
On 22/11/2013 16:07, Ludovic Desroches :
> On Fri, Nov 22, 2013 at 03:58:35PM +0100, Wolfram Sang wrote:
>> On Fri, Nov 22, 2013 at 03:51:41PM +0100, Ludovic Desroches wrote:
>>> Hi Wolfram,
>>>
>>> On Fri, Nov 22, 2013 at 03:33:51PM +0100, Wolfram Sang wrote:
>>>> On Fri, Nov 22, 2013 at 03:07:08PM +0100, ludovic.desroches at atmel.com wrote:
>>>>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>>
>>>>> There are still I2C unexpected behaviors which are solved by reducing TWI
>>>>> internal frequency.
>>>>>
>>>>> Cc: <stable@vger.kernel.org> #3.10+
>>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>
>>>> I think the commit message needs more details. Is this a true bugfix
>>>> because the real bus frequency was too high because of the wrong
>>>> divider? Is this a workaround which makes things work but will make the
>>>> bus frequency slower than it should be?
>>>
>>> This fix doesn't concern the i2c bus frequency, only the internal IP frequency.
>>>
>>> TWI has been validated at 66MHz. With some devices, transfer hangs during i2c
>>> frame transmission. This issue disappears when reducing the internal frequency
>>> of the IP. Maybe there is some oversampling on i2c signals.
>>> Unfortunately, I have no clear status about the root cause that's why
>>> the commit message was imprecise.
>>
>> This paragraph is a way better commit message IMO :)
>>
>
> Ok I'll update it.
Ludo, you can also add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Bye,
> Thanks for the review Wolfram.
>
> Regards
>
> Ludovic
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 14:07 [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency ludovic.desroches at atmel.com
2013-11-22 14:33 ` Wolfram Sang
@ 2013-11-22 15:01 ` boris brezillon
2013-11-22 15:11 ` Nicolas Ferre
1 sibling, 1 reply; 8+ messages in thread
From: boris brezillon @ 2013-11-22 15:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ludovic,
On 22/11/2013 15:07, ludovic.desroches at atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>
> There are still I2C unexpected behaviors which are solved by reducing TWI
> internal frequency.
I guess I should do the same for the dt version of sama5 clks.
Nicolas, what do you think ?
Best Regards,
Boris
>
> Cc: <stable@vger.kernel.org> #3.10+
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
> arch/arm/mach-at91/sama5d3.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
> index 4012797..4ee0de5 100644
> --- a/arch/arm/mach-at91/sama5d3.c
> +++ b/arch/arm/mach-at91/sama5d3.c
> @@ -95,19 +95,19 @@ static struct clk twi0_clk = {
> .name = "twi0_clk",
> .pid = SAMA5D3_ID_TWI0,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk twi1_clk = {
> .name = "twi1_clk",
> .pid = SAMA5D3_ID_TWI1,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk twi2_clk = {
> .name = "twi2_clk",
> .pid = SAMA5D3_ID_TWI2,
> .type = CLK_TYPE_PERIPHERAL,
> - .div = AT91_PMC_PCR_DIV2,
> + .div = AT91_PMC_PCR_DIV8,
> };
> static struct clk mmc0_clk = {
> .name = "mci0_clk",
>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency
2013-11-22 15:01 ` boris brezillon
@ 2013-11-22 15:11 ` Nicolas Ferre
0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Ferre @ 2013-11-22 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On 22/11/2013 16:01, boris brezillon :
> Hi Ludovic,
>
> On 22/11/2013 15:07, ludovic.desroches at atmel.com wrote:
>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>>
>> There are still I2C unexpected behaviors which are solved by reducing TWI
>> internal frequency.
>
> I guess I should do the same for the dt version of sama5 clks.
> Nicolas, what do you think ?
Yes, you can queue these modification on top of your "CCF related fixes"
branch.
Bye,
>> Cc: <stable@vger.kernel.org> #3.10+
>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>> arch/arm/mach-at91/sama5d3.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
>> index 4012797..4ee0de5 100644
>> --- a/arch/arm/mach-at91/sama5d3.c
>> +++ b/arch/arm/mach-at91/sama5d3.c
>> @@ -95,19 +95,19 @@ static struct clk twi0_clk = {
>> .name = "twi0_clk",
>> .pid = SAMA5D3_ID_TWI0,
>> .type = CLK_TYPE_PERIPHERAL,
>> - .div = AT91_PMC_PCR_DIV2,
>> + .div = AT91_PMC_PCR_DIV8,
>> };
>> static struct clk twi1_clk = {
>> .name = "twi1_clk",
>> .pid = SAMA5D3_ID_TWI1,
>> .type = CLK_TYPE_PERIPHERAL,
>> - .div = AT91_PMC_PCR_DIV2,
>> + .div = AT91_PMC_PCR_DIV8,
>> };
>> static struct clk twi2_clk = {
>> .name = "twi2_clk",
>> .pid = SAMA5D3_ID_TWI2,
>> .type = CLK_TYPE_PERIPHERAL,
>> - .div = AT91_PMC_PCR_DIV2,
>> + .div = AT91_PMC_PCR_DIV8,
>> };
>> static struct clk mmc0_clk = {
>> .name = "mci0_clk",
>>
>
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-11-22 15:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-22 14:07 [PATCH] ARM: at91: sama5d3: reduce TWI internal clock frequency ludovic.desroches at atmel.com
2013-11-22 14:33 ` Wolfram Sang
2013-11-22 14:51 ` Ludovic Desroches
2013-11-22 14:58 ` Wolfram Sang
2013-11-22 15:07 ` Ludovic Desroches
2013-11-22 15:09 ` Nicolas Ferre
2013-11-22 15:01 ` boris brezillon
2013-11-22 15:11 ` Nicolas Ferre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox