make *_defconfig does not handle modified *_defconfig files

[Nuno Gonçalves]

Hi,

Considering i have arch/arm/configs/abc_defconfig, I can run "make
abc_defconfig" repeatedly, and always get output

# configuration written to .config

or

# No change to .config

But if I now change arch/arm/configs/abc_defconfig, even just the
modification date, then "make abc_defconfig" no longer works. I need
to make clean or delete a file to trigger it (eg
scripts/kconfig/symbol.o).

Thanks,
Nuno

Target bindeb-pkg no longer install DTBs

[Nuno Gonçalves]

Hi,

Since 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 [kbuild: consolidate
Devicetree dtb build rules], the target bindeb-pkg no longer installs
DTBs in the .deb package.

Thanks,
Nuno

Target bindeb-pkg no longer install DTBs

[Nuno Gonçalves]

Hi,

Since 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 [kbuild: consolidate
Devicetree dtb build rules], the target bindeb-pkg no longer installs
DTBs in the .deb package.

Thanks,
Nuno

Re: [PATCH] serial: 8250_dw: Avoid overflow in dw8250_set_termios

[Nuno Gonçalves]

On Mon, Jan 15, 2018 at 9:30 PM, Andy Shevchenko
 wrote:
> It seems the change broke Bluetooth on some Intel platforms. I'm not
> sure yet, but
> see here:
>
> https://github.com/Dunedan/mbp-2016-linux/issues/29#issuecomment-357583782
>

Hi Andy,

I guess you mean de9e33b [1] and not this patch (which is not in
mainline). Can you double check?

Thanks,
Nuno

[1] 
https://github.com/torvalds/linux/commit/de9e33bdfa22e607a88494ff21e9196d00bf4550

Re: [PATCH] serial: 8250_dw: Avoid overflow in dw8250_set_termios

[Nuno Gonçalves]

On Mon, Jan 15, 2018 at 9:30 PM, Andy Shevchenko
 wrote:
> It seems the change broke Bluetooth on some Intel platforms. I'm not
> sure yet, but
> see here:
>
> https://github.com/Dunedan/mbp-2016-linux/issues/29#issuecomment-357583782
>

Hi Andy,

I guess you mean de9e33b [1] and not this patch (which is not in
mainline). Can you double check?

Thanks,
Nuno

[1] 
https://github.com/torvalds/linux/commit/de9e33bdfa22e607a88494ff21e9196d00bf4550

Re: [PATCH] serial: 8250_dw: Avoid overflow in dw8250_set_termios

[Nuno Gonçalves]

Dear Ed,

Thanks.

Tested-by: Nuno Goncalves 

I just would like to report a aditional issue I find, which I am not
sure if it is intend behaviour or not. If I set bauds 1152000,
150, 200, 250, 300, I always get a actually set baud
of 150, because it appears to be the closest baud by my hardware,
but if I set 350, then the port gets disabled.

This is because up to 300 the closest integer divider is 1, but
then it becomes 0, which is invalid.

If we still should return the closest baud, then we must return a
minimum of 1, and never 0, at uart_get_divisor, or
serial8250_get_divisor.

Thanks,
Nuno

On Fri, Jan 12, 2018 at 2:45 PM, Ed Blake  wrote:
> When searching for an achievable input clock rate that is within
> +/-1.6% of an integer multiple of the target baudx16 rate, there is the
> potential to overflow the i * rate calculations.
>
> For example, on a 32-bit system with a baud rate of 400, the
> i * max_rate calculation will overflow if i reaches 67 without finding
> an acceptable rate.
>
> Fix this by setting the upper boundary of the loop appropriately to
> avoid overflow.
>
> Reported-by: Nuno Goncalves 
> Signed-off-by: Ed Blake 
> ---
>  drivers/tty/serial/8250/8250_dw.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c 
> b/drivers/tty/serial/8250/8250_dw.c
> index 5bb0c42c88dd..04b44829f0e3 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -252,7 +252,7 @@ static void dw8250_set_termios(struct uart_port *p, 
> struct ktermios *termios,
>struct ktermios *old)
>  {
> unsigned int baud = tty_termios_baud_rate(termios);
> -   unsigned int target_rate, min_rate, max_rate;
> +   unsigned int target_rate, min_rate, max_rate, div_max;
> struct dw8250_data *d = p->private_data;
> long rate;
> int i, ret;
> @@ -265,12 +265,14 @@ static void dw8250_set_termios(struct uart_port *p, 
> struct ktermios *termios,
> min_rate = target_rate - (target_rate >> 6);
> max_rate = target_rate + (target_rate >> 6);
>
> -   for (i = 1; i <= UART_DIV_MAX; i++) {
> +   /* Avoid overflow */
> +   div_max = min(UINT_MAX / max_rate, (unsigned int)UART_DIV_MAX);
> +   for (i = 1; i <= div_max; i++) {
> rate = clk_round_rate(d->clk, i * target_rate);
> if (rate >= i * min_rate && rate <= i * max_rate)
> break;
> }
> -   if (i <= UART_DIV_MAX) {
> +   if (i <= div_max) {
> clk_disable_unprepare(d->clk);
> ret = clk_set_rate(d->clk, rate);
> clk_prepare_enable(d->clk);
> --
> 2.11.0
>

Re: [PATCH] serial: 8250_dw: Avoid overflow in dw8250_set_termios

[Nuno Gonçalves]

Dear Ed,

Thanks.

Tested-by: Nuno Goncalves 

I just would like to report a aditional issue I find, which I am not
sure if it is intend behaviour or not. If I set bauds 1152000,
150, 200, 250, 300, I always get a actually set baud
of 150, because it appears to be the closest baud by my hardware,
but if I set 350, then the port gets disabled.

This is because up to 300 the closest integer divider is 1, but
then it becomes 0, which is invalid.

If we still should return the closest baud, then we must return a
minimum of 1, and never 0, at uart_get_divisor, or
serial8250_get_divisor.

Thanks,
Nuno

On Fri, Jan 12, 2018 at 2:45 PM, Ed Blake  wrote:
> When searching for an achievable input clock rate that is within
> +/-1.6% of an integer multiple of the target baudx16 rate, there is the
> potential to overflow the i * rate calculations.
>
> For example, on a 32-bit system with a baud rate of 400, the
> i * max_rate calculation will overflow if i reaches 67 without finding
> an acceptable rate.
>
> Fix this by setting the upper boundary of the loop appropriately to
> avoid overflow.
>
> Reported-by: Nuno Goncalves 
> Signed-off-by: Ed Blake 
> ---
>  drivers/tty/serial/8250/8250_dw.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c 
> b/drivers/tty/serial/8250/8250_dw.c
> index 5bb0c42c88dd..04b44829f0e3 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -252,7 +252,7 @@ static void dw8250_set_termios(struct uart_port *p, 
> struct ktermios *termios,
>struct ktermios *old)
>  {
> unsigned int baud = tty_termios_baud_rate(termios);
> -   unsigned int target_rate, min_rate, max_rate;
> +   unsigned int target_rate, min_rate, max_rate, div_max;
> struct dw8250_data *d = p->private_data;
> long rate;
> int i, ret;
> @@ -265,12 +265,14 @@ static void dw8250_set_termios(struct uart_port *p, 
> struct ktermios *termios,
> min_rate = target_rate - (target_rate >> 6);
> max_rate = target_rate + (target_rate >> 6);
>
> -   for (i = 1; i <= UART_DIV_MAX; i++) {
> +   /* Avoid overflow */
> +   div_max = min(UINT_MAX / max_rate, (unsigned int)UART_DIV_MAX);
> +   for (i = 1; i <= div_max; i++) {
> rate = clk_round_rate(d->clk, i * target_rate);
> if (rate >= i * min_rate && rate <= i * max_rate)
> break;
> }
> -   if (i <= UART_DIV_MAX) {
> +   if (i <= div_max) {
> clk_disable_unprepare(d->clk);
> ret = clk_set_rate(d->clk, rate);
> clk_prepare_enable(d->clk);
> --
> 2.11.0
>

Re: [PATCH] 8250_dw: do not int overflow when rate can not be aplied

[Nuno Gonçalves]

So, for me clk_round_rate() always returns 2400, and only the loop
variable i changes, so the search is monotonic, from the highest baud
to the lowest (increasing divider).

I am using a Allwiner H2+, with the serial port configuration from
sunxi-h3-h5.dtsi.

Are you sure that clk_round_rate can return differet values? Is that
because some boards might have several clock options beside the
adjustable divider?

I really need to understand what is the problem, to be able to suggest
a solution to the integer overflow that is being allowed to happen.

Thanks,
Nuno



On Thu, Jan 11, 2018 at 6:48 PM, Ed Blake <ed.bl...@sondrel.com> wrote:
> On 11/01/18 17:28, Nuno Gonçalves wrote:
>> I have to disagree :)
>>
>> if (rate < i * min_rate) is true to i=a, then
>>
>> (rate >= i * min_rate && rate <= i * max_rate) will always be false
>> for any i=b, where b>a.
>
> No, because 'rate' is assigned from clk_round_rate() each iteration.
>
> The idea of this code is to iterate through integer multiples of baud *
> 16 until you find an achievable rate that is within the +/- 1.6% range.
> Until then, the rate returned from clk_round_rate() could be lower than
> i * min_rate or higher than i * max_rate, in which case you keep going.
>
>> If this condition is true, it means the old condition would be always
>> false for the remaining of the iteration.
>>
>> My patch "only" avoids integer overflow and terminates the search as
>> soon as possible, since no need to search for bigger dividers if the
>> current one would already mean a rate below min_rate (that it, this is
>> the closer).
>
> It terminates the search as soon as the rate returned from
> clk_round_rate() is lower than i * min_rate, which is too soon.
>
>> So in fact we also break when the closer divider have been found.
>>
>> Thanks,
>> Nuno
>>
>> On Thu, Jan 11, 2018 at 6:18 PM, Ed Blake <ed.bl...@sondrel.com> wrote:
>>> Hi Nuno,
>>>
>>> Thanks for reporting this and the patch.
>>>
>>> On 11/01/18 13:38, Nuno Goncalves wrote:
>>>> When target_rate is big enough and not permitted in hardware,
>>>> then i is looped to UART_DIV_MAX (0x), and i * max_rate will overflow
>>>> (32b signed).
>>>>
>>>> A fix is to quit the loop early enough, as soon as rate < i * min_rate as 
>>>> it
>>>> means the rate is not permitted.
>>> 'rate < i * min_rate' does not mean the rate is not permitted.  
>>> clk_round_rate() gives the nearest achievable rate to the one requested, 
>>> which may be lower than i * min_rate.  This is not an error and in this 
>>> case we want to continue the loop searching for an acceptable rate.
>>>
>>>
>>>> This avoids arbitraty rates to be applied. Still in my hardware the max
>>>> allowed rate (150) is aplied when a higher is requested. This seems a
>>>> artifact of clk_round_rate which is not understood by me and independent of
>>>> this fix. Might or might not be another bug.
>>>>
>>>> Signed-off-by: Nuno Goncalves <nuno...@gmail.com>
>>>> ---
>>>>  drivers/tty/serial/8250/8250_dw.c | 8 +++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/8250/8250_dw.c 
>>>> b/drivers/tty/serial/8250/8250_dw.c
>>>> index 5bb0c42c88dd..a27ea916abbf 100644
>>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>>> @@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port *p, 
>>>> struct ktermios *termios,
>>>>
>>>>   for (i = 1; i <= UART_DIV_MAX; i++) {
>>>>   rate = clk_round_rate(d->clk, i * target_rate);
>>>> - if (rate >= i * min_rate && rate <= i * max_rate)
>>>> +
>>>> + if (rate < i * min_rate) {
>>>> + i = UART_DIV_MAX + 1;
>>>> + break;
>>>> + }
>>>> +
>>>> + if (rate <= i * max_rate)
>>>>   break;
>>>>   }
>>>>   if (i <= UART_DIV_MAX) {
>>> --
>>> Ed
>>>
>
> --
> Ed
>

Re: [PATCH] 8250_dw: do not int overflow when rate can not be aplied

[Nuno Gonçalves]

So, for me clk_round_rate() always returns 2400, and only the loop
variable i changes, so the search is monotonic, from the highest baud
to the lowest (increasing divider).

I am using a Allwiner H2+, with the serial port configuration from
sunxi-h3-h5.dtsi.

Are you sure that clk_round_rate can return differet values? Is that
because some boards might have several clock options beside the
adjustable divider?

I really need to understand what is the problem, to be able to suggest
a solution to the integer overflow that is being allowed to happen.

Thanks,
Nuno



On Thu, Jan 11, 2018 at 6:48 PM, Ed Blake  wrote:
> On 11/01/18 17:28, Nuno Gonçalves wrote:
>> I have to disagree :)
>>
>> if (rate < i * min_rate) is true to i=a, then
>>
>> (rate >= i * min_rate && rate <= i * max_rate) will always be false
>> for any i=b, where b>a.
>
> No, because 'rate' is assigned from clk_round_rate() each iteration.
>
> The idea of this code is to iterate through integer multiples of baud *
> 16 until you find an achievable rate that is within the +/- 1.6% range.
> Until then, the rate returned from clk_round_rate() could be lower than
> i * min_rate or higher than i * max_rate, in which case you keep going.
>
>> If this condition is true, it means the old condition would be always
>> false for the remaining of the iteration.
>>
>> My patch "only" avoids integer overflow and terminates the search as
>> soon as possible, since no need to search for bigger dividers if the
>> current one would already mean a rate below min_rate (that it, this is
>> the closer).
>
> It terminates the search as soon as the rate returned from
> clk_round_rate() is lower than i * min_rate, which is too soon.
>
>> So in fact we also break when the closer divider have been found.
>>
>> Thanks,
>> Nuno
>>
>> On Thu, Jan 11, 2018 at 6:18 PM, Ed Blake  wrote:
>>> Hi Nuno,
>>>
>>> Thanks for reporting this and the patch.
>>>
>>> On 11/01/18 13:38, Nuno Goncalves wrote:
>>>> When target_rate is big enough and not permitted in hardware,
>>>> then i is looped to UART_DIV_MAX (0x), and i * max_rate will overflow
>>>> (32b signed).
>>>>
>>>> A fix is to quit the loop early enough, as soon as rate < i * min_rate as 
>>>> it
>>>> means the rate is not permitted.
>>> 'rate < i * min_rate' does not mean the rate is not permitted.  
>>> clk_round_rate() gives the nearest achievable rate to the one requested, 
>>> which may be lower than i * min_rate.  This is not an error and in this 
>>> case we want to continue the loop searching for an acceptable rate.
>>>
>>>
>>>> This avoids arbitraty rates to be applied. Still in my hardware the max
>>>> allowed rate (150) is aplied when a higher is requested. This seems a
>>>> artifact of clk_round_rate which is not understood by me and independent of
>>>> this fix. Might or might not be another bug.
>>>>
>>>> Signed-off-by: Nuno Goncalves 
>>>> ---
>>>>  drivers/tty/serial/8250/8250_dw.c | 8 +++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/8250/8250_dw.c 
>>>> b/drivers/tty/serial/8250/8250_dw.c
>>>> index 5bb0c42c88dd..a27ea916abbf 100644
>>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>>> @@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port *p, 
>>>> struct ktermios *termios,
>>>>
>>>>   for (i = 1; i <= UART_DIV_MAX; i++) {
>>>>   rate = clk_round_rate(d->clk, i * target_rate);
>>>> - if (rate >= i * min_rate && rate <= i * max_rate)
>>>> +
>>>> + if (rate < i * min_rate) {
>>>> + i = UART_DIV_MAX + 1;
>>>> + break;
>>>> + }
>>>> +
>>>> + if (rate <= i * max_rate)
>>>>   break;
>>>>   }
>>>>   if (i <= UART_DIV_MAX) {
>>> --
>>> Ed
>>>
>
> --
> Ed
>

Re: [PATCH] 8250_dw: do not int overflow when rate can not be aplied

[Nuno Gonçalves]

I have to disagree :)

if (rate < i * min_rate) is true to i=a, then

(rate >= i * min_rate && rate <= i * max_rate) will always be false
for any i=b, where b>a.

If this condition is true, it means the old condition would be always
false for the remaining of the iteration.

My patch "only" avoids integer overflow and terminates the search as
soon as possible, since no need to search for bigger dividers if the
current one would already mean a rate below min_rate (that it, this is
the closer).

So in fact we also break when the closer divider have been found.

Thanks,
Nuno

On Thu, Jan 11, 2018 at 6:18 PM, Ed Blake  wrote:
> Hi Nuno,
>
> Thanks for reporting this and the patch.
>
> On 11/01/18 13:38, Nuno Goncalves wrote:
>> When target_rate is big enough and not permitted in hardware,
>> then i is looped to UART_DIV_MAX (0x), and i * max_rate will overflow
>> (32b signed).
>>
>> A fix is to quit the loop early enough, as soon as rate < i * min_rate as it
>> means the rate is not permitted.
>
> 'rate < i * min_rate' does not mean the rate is not permitted.  
> clk_round_rate() gives the nearest achievable rate to the one requested, 
> which may be lower than i * min_rate.  This is not an error and in this case 
> we want to continue the loop searching for an acceptable rate.
>
>
>>
>> This avoids arbitraty rates to be applied. Still in my hardware the max
>> allowed rate (150) is aplied when a higher is requested. This seems a
>> artifact of clk_round_rate which is not understood by me and independent of
>> this fix. Might or might not be another bug.
>>
>> Signed-off-by: Nuno Goncalves 
>> ---
>>  drivers/tty/serial/8250/8250_dw.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c 
>> b/drivers/tty/serial/8250/8250_dw.c
>> index 5bb0c42c88dd..a27ea916abbf 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port *p, 
>> struct ktermios *termios,
>>
>>   for (i = 1; i <= UART_DIV_MAX; i++) {
>>   rate = clk_round_rate(d->clk, i * target_rate);
>> - if (rate >= i * min_rate && rate <= i * max_rate)
>> +
>> + if (rate < i * min_rate) {
>> + i = UART_DIV_MAX + 1;
>> + break;
>> + }
>> +
>> + if (rate <= i * max_rate)
>>   break;
>>   }
>>   if (i <= UART_DIV_MAX) {
>
> --
> Ed
>

Re: [PATCH] 8250_dw: do not int overflow when rate can not be aplied

[Nuno Gonçalves]

I have to disagree :)

if (rate < i * min_rate) is true to i=a, then

(rate >= i * min_rate && rate <= i * max_rate) will always be false
for any i=b, where b>a.

If this condition is true, it means the old condition would be always
false for the remaining of the iteration.

My patch "only" avoids integer overflow and terminates the search as
soon as possible, since no need to search for bigger dividers if the
current one would already mean a rate below min_rate (that it, this is
the closer).

So in fact we also break when the closer divider have been found.

Thanks,
Nuno

On Thu, Jan 11, 2018 at 6:18 PM, Ed Blake  wrote:
> Hi Nuno,
>
> Thanks for reporting this and the patch.
>
> On 11/01/18 13:38, Nuno Goncalves wrote:
>> When target_rate is big enough and not permitted in hardware,
>> then i is looped to UART_DIV_MAX (0x), and i * max_rate will overflow
>> (32b signed).
>>
>> A fix is to quit the loop early enough, as soon as rate < i * min_rate as it
>> means the rate is not permitted.
>
> 'rate < i * min_rate' does not mean the rate is not permitted.  
> clk_round_rate() gives the nearest achievable rate to the one requested, 
> which may be lower than i * min_rate.  This is not an error and in this case 
> we want to continue the loop searching for an acceptable rate.
>
>
>>
>> This avoids arbitraty rates to be applied. Still in my hardware the max
>> allowed rate (150) is aplied when a higher is requested. This seems a
>> artifact of clk_round_rate which is not understood by me and independent of
>> this fix. Might or might not be another bug.
>>
>> Signed-off-by: Nuno Goncalves 
>> ---
>>  drivers/tty/serial/8250/8250_dw.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c 
>> b/drivers/tty/serial/8250/8250_dw.c
>> index 5bb0c42c88dd..a27ea916abbf 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port *p, 
>> struct ktermios *termios,
>>
>>   for (i = 1; i <= UART_DIV_MAX; i++) {
>>   rate = clk_round_rate(d->clk, i * target_rate);
>> - if (rate >= i * min_rate && rate <= i * max_rate)
>> +
>> + if (rate < i * min_rate) {
>> + i = UART_DIV_MAX + 1;
>> + break;
>> + }
>> +
>> + if (rate <= i * max_rate)
>>   break;
>>   }
>>   if (i <= UART_DIV_MAX) {
>
> --
> Ed
>

8250_dw bug

[Nuno Gonçalves]

Dear Ed and Greg,

There is a small bug on de9e33bdfa22e607a88494ff21e9196d00bf4550, at
least on 32bit devices.

Line 274 if (rate >= i * min_rate && rate <= i * max_rate)

This will overflow when min_rate/max_rate is large and can not be
achieved in the hardware.

Eg.

stty -F /dev/ttyS2 raw 350 (not achievable on my board).

target_rate=5600 (for 350baud)
min_rate=55125000
max_rate=56875000
rate=2400 (clk_round_rate for my board)

Since my board can only do 150baud, this loop will keep
incrementing i until i=77*56875000 will overflow, and there are
unexpected results.

I suggest this patch:

--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port
*p, struct ktermios *termios,

for (i = 1; i <= UART_DIV_MAX; i++) {
rate = clk_round_rate(d->clk, i * target_rate);
-   if (rate >= i * min_rate && rate <= i * max_rate)
+
+   if (rate < i * min_rate) {
+   i = UART_DIV_MAX;
+   break;
+   }
+
+   if (rate <= i * max_rate)
break;
}
if (i <= UART_DIV_MAX) {


Let me know if you want me to submit the formal patch.

Thanks,
Nuno

8250_dw bug

[Nuno Gonçalves]

Dear Ed and Greg,

There is a small bug on de9e33bdfa22e607a88494ff21e9196d00bf4550, at
least on 32bit devices.

Line 274 if (rate >= i * min_rate && rate <= i * max_rate)

This will overflow when min_rate/max_rate is large and can not be
achieved in the hardware.

Eg.

stty -F /dev/ttyS2 raw 350 (not achievable on my board).

target_rate=5600 (for 350baud)
min_rate=55125000
max_rate=56875000
rate=2400 (clk_round_rate for my board)

Since my board can only do 150baud, this loop will keep
incrementing i until i=77*56875000 will overflow, and there are
unexpected results.

I suggest this patch:

--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -267,7 +267,13 @@ static void dw8250_set_termios(struct uart_port
*p, struct ktermios *termios,

for (i = 1; i <= UART_DIV_MAX; i++) {
rate = clk_round_rate(d->clk, i * target_rate);
-   if (rate >= i * min_rate && rate <= i * max_rate)
+
+   if (rate < i * min_rate) {
+   i = UART_DIV_MAX;
+   break;
+   }
+
+   if (rate <= i * max_rate)
break;
}
if (i <= UART_DIV_MAX) {


Let me know if you want me to submit the formal patch.

Thanks,
Nuno

Re: [tip:timers/urgent] time: Fix timekeeping_freqadjust()' s incorrect use of abs() instead of abs64()

[Nuno Gonçalves]

On Sun, Sep 13, 2015 at 12:07 PM, tip-bot for John Stultz
 wrote:
> Commit-ID:  2619d7e9c92d524cb155ec89fd72875321512e5b
> Gitweb: http://git.kernel.org/tip/2619d7e9c92d524cb155ec89fd72875321512e5b
> Author: John Stultz 
> AuthorDate: Wed, 9 Sep 2015 16:07:30 -0700
> Committer:  Ingo Molnar 
> CommitDate: Sun, 13 Sep 2015 10:30:47 +0200
>
> time: Fix timekeeping_freqadjust()'s incorrect use of abs() instead of abs64()
>
> The internal clocksteering done for fine-grained error
> correction uses a logarithmic approximation, so any time
> adjtimex() adjusts the clock steering, timekeeping_freqadjust()
> quickly approximates the correct clock frequency over a series
> of ticks.
>
> Unfortunately, the logic in timekeeping_freqadjust(), introduced
> in commit:
>
>   dc491596f639 ("timekeeping: Rework frequency adjustments to work better w/ 
> nohz")
>
> used the abs() function with a s64 error value to calculate the
> size of the approximated adjustment to be made.
>
> Per include/linux/kernel.h:
>
>   "abs() should not be used for 64-bit types (s64, u64, long long) - use 
> abs64()".
>
> Thus on 32-bit platforms, this resulted in the clocksteering to
> take a quite dampended random walk trying to converge on the
> proper frequency, which caused the adjustments to be made much
> slower then intended (most easily observed when large
> adjustments are made).
>
> This patch fixes the issue by using abs64() instead.
>
> Reported-by: Nuno Gonçalves 
> Tested-by: Nuno Goncalves 
> Signed-off-by: John Stultz 
> Cc:  # v3.17+
> Cc: Linus Torvalds 
> Cc: Miroslav Lichvar 
> Cc: Peter Zijlstra 
> Cc: Prarit Bhargava 
> Cc: Richard Cochran 
> Cc: Thomas Gleixner 
> Link: 
> http://lkml.kernel.org/r/1441840051-20244-1-git-send-email-john.stu...@linaro.org
> Signed-off-by: Ingo Molnar 
> ---
>  kernel/time/timekeeping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index f6ee2e6..3739ac6 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1614,7 +1614,7 @@ static __always_inline void 
> timekeeping_freqadjust(struct timekeeper *tk,
> negative = (tick_error < 0);
>
> /* Sort out the magnitude of the correction */
> -   tick_error = abs(tick_error);
> +   tick_error = abs64(tick_error);
> for (adj = 0; tick_error > interval; adj++)
> tick_error >>= 1;
>

I think this got lost on its way to the linux-stable queue (or I don't
understand the stable rules).

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [tip:timers/urgent] time: Fix timekeeping_freqadjust()' s incorrect use of abs() instead of abs64()

[Nuno Gonçalves]

On Sun, Sep 13, 2015 at 12:07 PM, tip-bot for John Stultz
<tip...@zytor.com> wrote:
> Commit-ID:  2619d7e9c92d524cb155ec89fd72875321512e5b
> Gitweb: http://git.kernel.org/tip/2619d7e9c92d524cb155ec89fd72875321512e5b
> Author: John Stultz <john.stu...@linaro.org>
> AuthorDate: Wed, 9 Sep 2015 16:07:30 -0700
> Committer:  Ingo Molnar <mi...@kernel.org>
> CommitDate: Sun, 13 Sep 2015 10:30:47 +0200
>
> time: Fix timekeeping_freqadjust()'s incorrect use of abs() instead of abs64()
>
> The internal clocksteering done for fine-grained error
> correction uses a logarithmic approximation, so any time
> adjtimex() adjusts the clock steering, timekeeping_freqadjust()
> quickly approximates the correct clock frequency over a series
> of ticks.
>
> Unfortunately, the logic in timekeeping_freqadjust(), introduced
> in commit:
>
>   dc491596f639 ("timekeeping: Rework frequency adjustments to work better w/ 
> nohz")
>
> used the abs() function with a s64 error value to calculate the
> size of the approximated adjustment to be made.
>
> Per include/linux/kernel.h:
>
>   "abs() should not be used for 64-bit types (s64, u64, long long) - use 
> abs64()".
>
> Thus on 32-bit platforms, this resulted in the clocksteering to
> take a quite dampended random walk trying to converge on the
> proper frequency, which caused the adjustments to be made much
> slower then intended (most easily observed when large
> adjustments are made).
>
> This patch fixes the issue by using abs64() instead.
>
> Reported-by: Nuno Gonçalves <nuno...@gmail.com>
> Tested-by: Nuno Goncalves <nuno...@gmail.com>
> Signed-off-by: John Stultz <john.stu...@linaro.org>
> Cc: <sta...@vger.kernel.org> # v3.17+
> Cc: Linus Torvalds <torva...@linux-foundation.org>
> Cc: Miroslav Lichvar <mlich...@redhat.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Prarit Bhargava <pra...@redhat.com>
> Cc: Richard Cochran <richardcoch...@gmail.com>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Link: 
> http://lkml.kernel.org/r/1441840051-20244-1-git-send-email-john.stu...@linaro.org
> Signed-off-by: Ingo Molnar <mi...@kernel.org>
> ---
>  kernel/time/timekeeping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index f6ee2e6..3739ac6 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1614,7 +1614,7 @@ static __always_inline void 
> timekeeping_freqadjust(struct timekeeper *tk,
> negative = (tick_error < 0);
>
> /* Sort out the magnitude of the correction */
> -   tick_error = abs(tick_error);
> +   tick_error = abs64(tick_error);
> for (adj = 0; tick_error > interval; adj++)
> tick_error >>= 1;
>

I think this got lost on its way to the linux-stable queue (or I don't
understand the stable rules).

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Yes please.

Tested-by: Nuno Goncalves 

Thanks,
Nuno

On Wed, Sep 9, 2015 at 1:52 AM, John Stultz  wrote:
> On Sat, Sep 5, 2015 at 6:41 AM, Nuno Gonçalves  wrote:
>> Considering your last message I just tried to use abs64 instead. Fixes
>> the problem for me.
>
> Yea, I've since simplified my patch in the same way.
>
> Still looking good? Can I get a Tested-by: from you?
>
> thanks
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Yes please.

Tested-by: Nuno Goncalves <nuno...@gmail.com>

Thanks,
Nuno

On Wed, Sep 9, 2015 at 1:52 AM, John Stultz <john.stu...@linaro.org> wrote:
> On Sat, Sep 5, 2015 at 6:41 AM, Nuno Gonçalves <nuno...@gmail.com> wrote:
>> Considering your last message I just tried to use abs64 instead. Fixes
>> the problem for me.
>
> Yea, I've since simplified my patch in the same way.
>
> Still looking good? Can I get a Tested-by: from you?
>
> thanks
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Considering your last message I just tried to use abs64 instead. Fixes
the problem for me.

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 946acb7..414d9df 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1615,7 +1615,7 @@ static __always_inline void
timekeeping_freqadjust(struct timekeeper *tk,
  negative = (tick_error < 0);

  /* Sort out the magnitude of the correction */
- tick_error = abs(tick_error);
+ tick_error = abs64(tick_error);
  for (adj = 0; tick_error > interval; adj++)
  tick_error >>= 1;

Thanks!
Nuno

On Sat, Sep 5, 2015 at 2:00 AM, John Stultz  wrote:
> On Fri, Sep 4, 2015 at 5:57 PM, John Stultz  wrote:
>> On Thu, Sep 3, 2015 at 4:26 AM, Miroslav Lichvar  wrote:
>>> On Wed, Sep 02, 2015 at 04:16:00PM -0700, John Stultz wrote:
>>>> On Tue, Sep 1, 2015 at 6:14 PM, Nuno Gonçalves  wrote:
>>>> > And just installing chrony from the feeds. With any kernel from 3.17
>>>> > you'll have wrong estimates at chronyc sourcestats.
>>>>
>>>> Wrong estimates? Could you be more specific about what the failure
>>>> you're seeing is here? The
>>>>
>>>> I installed the image above, which comes with a 4.1.6 kernel, and
>>>> chrony seems to have gotten my BBB into ~1ms sync w/ servers over the
>>>> internet fairly quickly (at least according to chronyc tracking).
>>>
>>> To see the bug with chronyd the initial offset shouldn't be very close
>>> to zero, so it's forced to correct the offset by adjusting the
>>> frequency in a larger step.
>>>
>>> I'm attaching a simple C program that prints the frequency offset
>>> as measured between the REALTIME and MONOTONIC_RAW clocks when the
>>> adjtimex tick is set to 9000. It should show values close to -10
>>> ppm and I suspect on the BBB it will be much smaller.
>>
>> So I spent some time on this late last night and this afternoon.
>>
>> It was a little odd because things don't seem totally broken, but
>> something isn't quite right.
>>
>> Digging around it seems the iterative logrithmic approximation done in
>> timekeeping_freqadjust() wasn't working right. Instead of making
>> smaller order alternating positive and negative adjustments, it was
>> doing strange growing adjustments for the same value that wern't large
>> enough to actually correct things very quickly. This made it much
>> slower to adapt to specified frequency values.
>>
>> The odd bit, is it seems to come down to:
>> tick_error = abs(tick_error);
>>
>> Haven't chased down why yet, but apparently abs() isn't doing what one
>> would think when passed a s64 value.
>
> Well.. chasing it down wasn't hard.. from include/linux/kernel.h:
> /*
>  * abs() handles unsigned and signed longs, ints, shorts and chars.  For all
>  * input types abs() returns a signed long.
>  * abs() should not be used for 64-bit types (s64, u64, long long) - use 
> abs64()
>  * for those.
>  */
>
> Ouch.
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Considering your last message I just tried to use abs64 instead. Fixes
the problem for me.

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 946acb7..414d9df 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1615,7 +1615,7 @@ static __always_inline void
timekeeping_freqadjust(struct timekeeper *tk,
  negative = (tick_error < 0);

  /* Sort out the magnitude of the correction */
- tick_error = abs(tick_error);
+ tick_error = abs64(tick_error);
  for (adj = 0; tick_error > interval; adj++)
  tick_error >>= 1;

Thanks!
Nuno

On Sat, Sep 5, 2015 at 2:00 AM, John Stultz <john.stu...@linaro.org> wrote:
> On Fri, Sep 4, 2015 at 5:57 PM, John Stultz <john.stu...@linaro.org> wrote:
>> On Thu, Sep 3, 2015 at 4:26 AM, Miroslav Lichvar <mlich...@redhat.com> wrote:
>>> On Wed, Sep 02, 2015 at 04:16:00PM -0700, John Stultz wrote:
>>>> On Tue, Sep 1, 2015 at 6:14 PM, Nuno Gonçalves <nuno...@gmail.com> wrote:
>>>> > And just installing chrony from the feeds. With any kernel from 3.17
>>>> > you'll have wrong estimates at chronyc sourcestats.
>>>>
>>>> Wrong estimates? Could you be more specific about what the failure
>>>> you're seeing is here? The
>>>>
>>>> I installed the image above, which comes with a 4.1.6 kernel, and
>>>> chrony seems to have gotten my BBB into ~1ms sync w/ servers over the
>>>> internet fairly quickly (at least according to chronyc tracking).
>>>
>>> To see the bug with chronyd the initial offset shouldn't be very close
>>> to zero, so it's forced to correct the offset by adjusting the
>>> frequency in a larger step.
>>>
>>> I'm attaching a simple C program that prints the frequency offset
>>> as measured between the REALTIME and MONOTONIC_RAW clocks when the
>>> adjtimex tick is set to 9000. It should show values close to -10
>>> ppm and I suspect on the BBB it will be much smaller.
>>
>> So I spent some time on this late last night and this afternoon.
>>
>> It was a little odd because things don't seem totally broken, but
>> something isn't quite right.
>>
>> Digging around it seems the iterative logrithmic approximation done in
>> timekeeping_freqadjust() wasn't working right. Instead of making
>> smaller order alternating positive and negative adjustments, it was
>> doing strange growing adjustments for the same value that wern't large
>> enough to actually correct things very quickly. This made it much
>> slower to adapt to specified frequency values.
>>
>> The odd bit, is it seems to come down to:
>> tick_error = abs(tick_error);
>>
>> Haven't chased down why yet, but apparently abs() isn't doing what one
>> would think when passed a s64 value.
>
> Well.. chasing it down wasn't hard.. from include/linux/kernel.h:
> /*
>  * abs() handles unsigned and signed longs, ints, shorts and chars.  For all
>  * input types abs() returns a signed long.
>  * abs() should not be used for 64-bit types (s64, u64, long long) - use 
> abs64()
>  * for those.
>  */
>
> Ouch.
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Sorry,

The default chrony config from the debian package can't bring the
clock in sync because it doesn't do steps, and it starts 15 years in
the past. You had some other external help.

Assuming "all factory config" just make sure to disable timesyncd:

sudo systemctl disable systemd-timesyncd

And then reboot in a clean chrony state:

sudo systemctl stop chrony && sudo rm /var/lib/chrony/chrony.drift &&
sudo reboot

You can also just cycle power to get back to year 2000, since the
beaglebone doesn't have a battery backed RTC.

After boot you can see this frequency estimates are allways very large
(usually from +-10.000...80.000):

debian@bbb1:~$ chronyc sourcestats
210 Number of sources = 4
Name/IP AddressNP  NR  Span  Frequency  Freq Skew  Offset  Std Dev
==
ftp.claranet.pt 5   3 8 -40146.551 130300  -2720ms61ms
a88-157-128-22.cpe.netcab   5   3 8 -35628.008 129339  -2393ms68ms
a212-113-190-2.cpe.netcab   5   3 8 -45213.773 169140  -3057ms63ms
mirrors.dominios.pt 5   3 8 -39615.191  5.367  -2694ms53ms

To have it perform normally try for example 3.16.3:

sudo apt-get install linux-image-3.16.3-bone6

Thanks,
Nuno

On Thu, Sep 3, 2015 at 12:16 AM, John Stultz  wrote:
> On Tue, Sep 1, 2015 at 6:14 PM, Nuno Gonçalves  wrote:
>> On Wed, Sep 2, 2015 at 2:03 AM, John Stultz  wrote:
>>> On Tue, Sep 1, 2015 at 5:36 PM, Nuno Gonçalves  wrote:
>>>> On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner  wrote:
>>>>> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>>>>>
>>>>>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>>>>>> [1],
>>>>>
>>>>>> [1] 
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>>>>>
>>>>> That commit has absolutely nothing to do with NTP. I fear your bisect
>>>>> went down the wrong road somewhere.
>>>>
>>>> You are right. It is v3.16-rc5-114-gdc49159:
>>>>
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a
>>>>
>>>> I've triple checked it this time. Not sure where I did the mistake to
>>>> get it wrong by 3 commits.
>>>
>>> This commit is much more believable (though surprising as that change
>>> was found to greatly improve results for most uses).
>>>
>>> Can you provide any more details about how the problem is reproduced
>>> (kernel config, what userland images are you using, etc)?  I've got a
>>> BBB myself so I can try to see whats going on.
>>>
>>> thanks
>>> -john
>>
>> I'm using a clean Debian image:
>>
>> https://rcn-ee.com/rootfs/bb.org/testing/2015-08-31/console/bone-debian-8.1-console-armhf-2015-08-31-2gb.img.xz
>>
>> And just installing chrony from the feeds. With any kernel from 3.17
>> you'll have wrong estimates at chronyc sourcestats.
>
> Wrong estimates? Could you be more specific about what the failure
> you're seeing is here? The
>
> I installed the image above, which comes with a 4.1.6 kernel, and
> chrony seems to have gotten my BBB into ~1ms sync w/ servers over the
> internet fairly quickly (at least according to chronyc tracking).
>
> root@beaglebone:~# chronyc tracking
> Reference ID: 198.110.48.12 (time01.muskegonisd.org)
> Stratum : 3
> Ref time (UTC)  : Wed Sep  2 23:07:05 2015
> System time : 0.001320852 seconds fast of NTP time
> Last offset : +0.001209910 seconds
> RMS offset  : 0.002978454 seconds
> Frequency   : 44.684 ppm fast
> Residual freq   : +0.068 ppm
> Skew: 1.223 ppm
> Root delay  : 0.073661 seconds
> Root dispersion : 0.021902 seconds
> Update interval : 518.3 seconds
> Leap status : Normal
> root@beaglebone:~# chronyc sourcestats
> 210 Number of sources = 4
> Name/IP AddressNP  NR  Span  Frequency  Freq Skew  Offset  Std Dev
> ==
> 172.82.134.51   4   3   17m +1.593 30.515   +798us   802us
> unlawful.id.au  6   5   21m +1.080  2.312  +2731us   273us
> time.theplante.net 11   5   77m -0.536  0.915  -1165us  1044us
> time01.muskegonisd.org  4   3   25m +1.677 15.256   -342us   516us
>
>
> Can you send me your kernel config?
>
> thanks
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

Sorry,

The default chrony config from the debian package can't bring the
clock in sync because it doesn't do steps, and it starts 15 years in
the past. You had some other external help.

Assuming "all factory config" just make sure to disable timesyncd:

sudo systemctl disable systemd-timesyncd

And then reboot in a clean chrony state:

sudo systemctl stop chrony && sudo rm /var/lib/chrony/chrony.drift &&
sudo reboot

You can also just cycle power to get back to year 2000, since the
beaglebone doesn't have a battery backed RTC.

After boot you can see this frequency estimates are allways very large
(usually from +-10.000...80.000):

debian@bbb1:~$ chronyc sourcestats
210 Number of sources = 4
Name/IP AddressNP  NR  Span  Frequency  Freq Skew  Offset  Std Dev
==
ftp.claranet.pt 5   3 8 -40146.551 130300  -2720ms61ms
a88-157-128-22.cpe.netcab   5   3 8 -35628.008 129339  -2393ms68ms
a212-113-190-2.cpe.netcab   5   3 8 -45213.773 169140  -3057ms63ms
mirrors.dominios.pt 5   3 8 -39615.191  5.367  -2694ms53ms

To have it perform normally try for example 3.16.3:

sudo apt-get install linux-image-3.16.3-bone6

Thanks,
Nuno

On Thu, Sep 3, 2015 at 12:16 AM, John Stultz <john.stu...@linaro.org> wrote:
> On Tue, Sep 1, 2015 at 6:14 PM, Nuno Gonçalves <nuno...@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 2:03 AM, John Stultz <john.stu...@linaro.org> wrote:
>>> On Tue, Sep 1, 2015 at 5:36 PM, Nuno Gonçalves <nuno...@gmail.com> wrote:
>>>> On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner <t...@linutronix.de> wrote:
>>>>> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>>>>>
>>>>>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>>>>>> [1],
>>>>>
>>>>>> [1] 
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>>>>>
>>>>> That commit has absolutely nothing to do with NTP. I fear your bisect
>>>>> went down the wrong road somewhere.
>>>>
>>>> You are right. It is v3.16-rc5-114-gdc49159:
>>>>
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a
>>>>
>>>> I've triple checked it this time. Not sure where I did the mistake to
>>>> get it wrong by 3 commits.
>>>
>>> This commit is much more believable (though surprising as that change
>>> was found to greatly improve results for most uses).
>>>
>>> Can you provide any more details about how the problem is reproduced
>>> (kernel config, what userland images are you using, etc)?  I've got a
>>> BBB myself so I can try to see whats going on.
>>>
>>> thanks
>>> -john
>>
>> I'm using a clean Debian image:
>>
>> https://rcn-ee.com/rootfs/bb.org/testing/2015-08-31/console/bone-debian-8.1-console-armhf-2015-08-31-2gb.img.xz
>>
>> And just installing chrony from the feeds. With any kernel from 3.17
>> you'll have wrong estimates at chronyc sourcestats.
>
> Wrong estimates? Could you be more specific about what the failure
> you're seeing is here? The
>
> I installed the image above, which comes with a 4.1.6 kernel, and
> chrony seems to have gotten my BBB into ~1ms sync w/ servers over the
> internet fairly quickly (at least according to chronyc tracking).
>
> root@beaglebone:~# chronyc tracking
> Reference ID: 198.110.48.12 (time01.muskegonisd.org)
> Stratum : 3
> Ref time (UTC)  : Wed Sep  2 23:07:05 2015
> System time : 0.001320852 seconds fast of NTP time
> Last offset : +0.001209910 seconds
> RMS offset  : 0.002978454 seconds
> Frequency   : 44.684 ppm fast
> Residual freq   : +0.068 ppm
> Skew: 1.223 ppm
> Root delay  : 0.073661 seconds
> Root dispersion : 0.021902 seconds
> Update interval : 518.3 seconds
> Leap status : Normal
> root@beaglebone:~# chronyc sourcestats
> 210 Number of sources = 4
> Name/IP AddressNP  NR  Span  Frequency  Freq Skew  Offset  Std Dev
> ==
> 172.82.134.51   4   3   17m +1.593 30.515   +798us   802us
> unlawful.id.au  6   5   21m +1.080  2.312  +2731us   273us
> time.theplante.net 11   5   77m -0.536  0.915  -1165us  1044us
> time01.muskegonisd.org  4   3   25m +1.677 15.256   -342us   516us
>
>
> Can you send me your kernel config?
>
> thanks
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

On Wed, Sep 2, 2015 at 2:03 AM, John Stultz  wrote:
> On Tue, Sep 1, 2015 at 5:36 PM, Nuno Gonçalves  wrote:
>> On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner  wrote:
>>> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>>>
>>>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>>>> [1],
>>>
>>>> [1] 
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>>>
>>> That commit has absolutely nothing to do with NTP. I fear your bisect
>>> went down the wrong road somewhere.
>>
>> You are right. It is v3.16-rc5-114-gdc49159:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a
>>
>> I've triple checked it this time. Not sure where I did the mistake to
>> get it wrong by 3 commits.
>
> This commit is much more believable (though surprising as that change
> was found to greatly improve results for most uses).
>
> Can you provide any more details about how the problem is reproduced
> (kernel config, what userland images are you using, etc)?  I've got a
> BBB myself so I can try to see whats going on.
>
> thanks
> -john

I'm using a clean Debian image:

https://rcn-ee.com/rootfs/bb.org/testing/2015-08-31/console/bone-debian-8.1-console-armhf-2015-08-31-2gb.img.xz

And just installing chrony from the feeds. With any kernel from 3.17
you'll have wrong estimates at chronyc sourcestats.

Miroslav did some tests at a beaglebone I set for him, according to my
initial post.

Miroslav also dismissed this being related to nohz after some tests.

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner  wrote:
> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>
>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>> [1],
>
>> [1] 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>
> That commit has absolutely nothing to do with NTP. I fear your bisect
> went down the wrong road somewhere.

You are right. It is v3.16-rc5-114-gdc49159:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a

I've triple checked it this time. Not sure where I did the mistake to
get it wrong by 3 commits.

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

There is a regression on the clock system since v3.16-rc5-111-g4396e05
[1], where the clock doesn't apply frequency offsets above about
1000ppm [2].

This was found at a Beaglebone (armv7l, TI AM335x).

Thanks,
Nuno Goncalves

[1] 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
[2] 
http://listengine.tuxfamily.org/chrony.tuxfamily.org/chrony-users/2015/08/msg00022.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner <t...@linutronix.de> wrote:
> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>
>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>> [1],
>
>> [1] 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>
> That commit has absolutely nothing to do with NTP. I fear your bisect
> went down the wrong road somewhere.

You are right. It is v3.16-rc5-114-gdc49159:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a

I've triple checked it this time. Not sure where I did the mistake to
get it wrong by 3 commits.

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

On Wed, Sep 2, 2015 at 2:03 AM, John Stultz <john.stu...@linaro.org> wrote:
> On Tue, Sep 1, 2015 at 5:36 PM, Nuno Gonçalves <nuno...@gmail.com> wrote:
>> On Tue, Sep 1, 2015 at 9:25 PM, Thomas Gleixner <t...@linutronix.de> wrote:
>>> On Tue, 1 Sep 2015, Nuno Gonçalves wrote:
>>>
>>>> There is a regression on the clock system since v3.16-rc5-111-g4396e05
>>>> [1],
>>>
>>>> [1] 
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
>>>
>>> That commit has absolutely nothing to do with NTP. I fear your bisect
>>> went down the wrong road somewhere.
>>
>> You are right. It is v3.16-rc5-114-gdc49159:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dc491596f6394382fbc74ad331156207d619fa0a
>>
>> I've triple checked it this time. Not sure where I did the mistake to
>> get it wrong by 3 commits.
>
> This commit is much more believable (though surprising as that change
> was found to greatly improve results for most uses).
>
> Can you provide any more details about how the problem is reproduced
> (kernel config, what userland images are you using, etc)?  I've got a
> BBB myself so I can try to see whats going on.
>
> thanks
> -john

I'm using a clean Debian image:

https://rcn-ee.com/rootfs/bb.org/testing/2015-08-31/console/bone-debian-8.1-console-armhf-2015-08-31-2gb.img.xz

And just installing chrony from the feeds. With any kernel from 3.17
you'll have wrong estimates at chronyc sourcestats.

Miroslav did some tests at a beaglebone I set for him, according to my
initial post.

Miroslav also dismissed this being related to nohz after some tests.

Thanks,
Nuno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Regression: can't apply frequency offsets above 1000ppm.

[Nuno Gonçalves]

There is a regression on the clock system since v3.16-rc5-111-g4396e05
[1], where the clock doesn't apply frequency offsets above about
1000ppm [2].

This was found at a Beaglebone (armv7l, TI AM335x).

Thanks,
Nuno Goncalves

[1] 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4396e058c52e167729729cf64ea3dfa229637086
[2] 
http://listengine.tuxfamily.org/chrony.tuxfamily.org/chrony-users/2015/08/msg00022.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/