[PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument

2018-11-20 Thread Dmitry V. Levin
This argument is required to extend the generic ptrace API
with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
called from ptrace_request() along with other syscall_get_* functions
with a tracee as their argument.

This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
remove useless function arguments").

Cc: linux-al...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-au...@redhat.com
Cc: linux-c6x-...@linux-c6x.org
Cc: linux-hexa...@vger.kernel.org
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@lists.linux-m68k.org
Cc: linux-m...@linux-mips.org
Cc: linux-par...@vger.kernel.org
Cc: linux-ri...@lists.infradead.org
Cc: linux-s...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linux-snps-...@lists.infradead.org
Cc: linux...@lists.infradead.org
Cc: linux-xte...@linux-xtensa.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: nios2-...@lists.rocketboards.org
Cc: openr...@lists.librecores.org
Cc: sparcli...@vger.kernel.org
Cc: uclinux-h8-de...@lists.sourceforge.jp
Cc: x...@kernel.org
Signed-off-by: Dmitry V. Levin 
---
 arch/alpha/include/asm/syscall.h  |  2 +-
 arch/arc/include/asm/syscall.h|  2 +-
 arch/arm/include/asm/syscall.h|  2 +-
 arch/arm64/include/asm/syscall.h  |  4 ++--
 arch/c6x/include/asm/syscall.h|  2 +-
 arch/h8300/include/asm/syscall.h  |  2 +-
 arch/hexagon/include/asm/syscall.h|  2 +-
 arch/ia64/include/asm/syscall.h   |  2 +-
 arch/m68k/include/asm/syscall.h   |  2 +-
 arch/microblaze/include/asm/syscall.h |  2 +-
 arch/mips/include/asm/syscall.h   |  8 
 arch/mips/kernel/ptrace.c |  2 +-
 arch/nds32/include/asm/syscall.h  |  2 +-
 arch/nios2/include/asm/syscall.h  |  2 +-
 arch/openrisc/include/asm/syscall.h   |  2 +-
 arch/parisc/include/asm/syscall.h |  4 ++--
 arch/powerpc/include/asm/syscall.h| 10 --
 arch/riscv/include/asm/syscall.h  |  2 +-
 arch/s390/include/asm/syscall.h   |  4 ++--
 arch/sh/include/asm/syscall_32.h  |  2 +-
 arch/sh/include/asm/syscall_64.h  |  2 +-
 arch/sparc/include/asm/syscall.h  |  5 +++--
 arch/unicore32/include/asm/syscall.h  |  2 +-
 arch/x86/include/asm/syscall.h|  8 +---
 arch/x86/um/asm/syscall.h |  2 +-
 arch/xtensa/include/asm/syscall.h |  2 +-
 include/asm-generic/syscall.h |  3 ++-
 kernel/auditsc.c  |  4 ++--
 kernel/seccomp.c  |  4 ++--
 29 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/arch/alpha/include/asm/syscall.h b/arch/alpha/include/asm/syscall.h
index d73a6fcb519c..11c688c1d7ec 100644
--- a/arch/alpha/include/asm/syscall.h
+++ b/arch/alpha/include/asm/syscall.h
@@ -4,7 +4,7 @@
 
 #include 
 
-static inline int syscall_get_arch(void)
+static inline int syscall_get_arch(struct task_struct *task)
 {
return AUDIT_ARCH_ALPHA;
 }
diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
index 10b2e7523bc8..7834baa61de8 100644
--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -69,7 +69,7 @@ syscall_get_arguments(struct task_struct *task, struct 
pt_regs *regs,
}
 }
 
-static inline int syscall_get_arch(void)
+static inline int syscall_get_arch(struct task_struct *task)
 {
return IS_ENABLED(CONFIG_ISA_ARCOMPACT)
? (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)
diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index 06dea6bce293..3940ceac0bdc 100644
--- a/arch/arm/include/asm/syscall.h
+++ b/arch/arm/include/asm/syscall.h
@@ -104,7 +104,7 @@ static inline void syscall_set_arguments(struct task_struct 
*task,
memcpy(>ARM_r0 + i, args, n * sizeof(args[0]));
 }
 
-static inline int syscall_get_arch(void)
+static inline int syscall_get_arch(struct task_struct *task)
 {
/* ARM tasks don't change audit architectures on the fly. */
return AUDIT_ARCH_ARM;
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index ad8be16a39c9..1870df03f774 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -117,9 +117,9 @@ static inline void syscall_set_arguments(struct task_struct 
*task,
  * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
  * AArch64 has the same system calls both on little- and big- endian.
  */
-static inline int syscall_get_arch(void)
+static inline int syscall_get_arch(struct task_struct *task)
 {
-   if (is_compat_task())
+   if (is_compat_thread(task_thread_info(task)))
return AUDIT_ARCH_ARM;
 
return AUDIT_ARCH_AARCH64;
diff --git a/arch/c6x/include/asm/syscall.h b/arch/c6x/include/asm/syscall.h
index 39dbd1ef994c..595057191c9c 100644
--- a/arch/c6x/include/asm/syscall.h
+++ b/arch/c6x/include/asm/syscall.h
@@ -121,7 +121,7 @@ static inline void syscall_set_arguments(struct task_struct 
*task,
}
 }
 
-static inline int 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

2018-11-20 Thread Finn Thain
On Tue, 20 Nov 2018, Kars de Jong wrote:

> Op ma 19 nov. 2018 om 02:10 schreef Finn Thain :
> >
> > hp300_gettimeoffset() never checks the timer interrupt flag and will
> > fail to notice when the timer counter gets reloaded. That means the
> > clock could jump backwards.
> >
> > Remove this code and leave this platform on the 'jiffies' clocksource.
> > Note that this amounts to a regression in clock precision. However,
> > adopting the 'jiffies' clocksource does resolve the monotonicity issue.
> >
> > Signed-off-by: Finn Thain 
> > ---
> > hp300_gettimeoffset() cannot be used in a clocksource conversion
> > unless it can be made monotonic. I can't fix this without knowing the
> > details of the timer implementation, such as the relationship between
> > the timer count and the interrupt flag.
> 
> I don't really like this regression...
> 

Me neither...

I'll see if I can write a conversion patch based on the information you've 
provided. Can you test it?

> According to NetBSD sources, there are 3 timers in the chip (originally 
> an MC6840 PTM).

Thanks for the tip. I will examine the datasheet for the 6840.

I'll also take another look at the NetBSD code.

> Timer 1 is used as the system timer, timer 3 runs at the same rate and 
> is unused on Linux (on NetBSD it is used as the statistics/profiling 
> timer), and timer 3 is connected to timer 2 so you can make a 32-bit 
> timer out of the two timers together (also unused on Linux).
> 
> Timers 1 counts down at 25 MHz.

You mean, 250 kHz, right? The code in mainline programs the timer for 2500 
cycles, hoping to get 10 ms. That is, 250 cycles per ms.

> The interrupt flag is set when the counter reaches 0 after which it is 
> automatically reloaded and starts counting down again.
> 

Thanks.

On atari, the 68901 counts down to 0x01 and raises an interrupt. On mac, 
the 6522 counts down to 0x then raises an interrupt. No idea about 
amiga (Geert?) -- this has to be handled correctly to get a monotonic 
clocksource. I'll fix this in v3 (where the information is available).

-- 

> > ---
> >  arch/m68k/hp300/time.c | 19 ---
> >  1 file changed, 19 deletions(-)
> 


Re: [PATCH v2 00/15] Prepare for PTRACE_GET_SYSCALL_INFO

2018-11-20 Thread Paul Moore
On Mon, Nov 19, 2018 at 7:11 PM Dmitry V. Levin  wrote:
>
> syscall_get_arch() is required to be implemented on all architectures
> in order to extend the generic ptrace API with PTRACE_GET_SYSCALL_INFO
> request.
>
> The primary intent is that the triple (audit_arch, syscall_nr, arg1..arg6)
> should describe what system call is being called and what its arguments are.

No real comment from me, most of this is arch specific code so I'll
let the individual arch folks comment on that; they know far better
than I do what is correct.

That said, anything that gets us closer to being able to offer syscall
auditing for these arches gets a big thumbs up from me - thanks!

> Dmitry V. Levin (15):
>   Move EM_HEXAGON to uapi/linux/elf-em.h
>   Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
>   Move EM_UNICORE to uapi/linux/elf-em.h
>   elf-em.h: add EM_NDS32
>   elf-em.h: add EM_XTENSA
>   m68k: define syscall_get_arch()
>   arc: define syscall_get_arch()
>   c6x: define syscall_get_arch()
>   h8300: define syscall_get_arch()
>   hexagon: define syscall_get_arch()
>   nds32: define syscall_get_arch()
>   nios2: define syscall_get_arch()
>   riscv: define syscall_get_arch()
>   unicore32: define syscall_get_arch()
>   xtensa: define syscall_get_arch()
>
>  arch/arc/include/asm/elf.h   |  6 +-
>  arch/arc/include/asm/syscall.h   | 10 ++
>  arch/c6x/include/asm/syscall.h   |  7 +++
>  arch/h8300/include/asm/syscall.h |  5 +
>  arch/hexagon/include/asm/elf.h   |  6 +-
>  arch/hexagon/include/asm/syscall.h   |  8 
>  arch/m68k/include/asm/syscall.h  | 12 
>  arch/nds32/include/asm/syscall.h |  8 
>  arch/nios2/include/asm/syscall.h |  6 ++
>  arch/riscv/include/asm/syscall.h | 10 ++
>  arch/unicore32/include/asm/elf.h |  3 +--
>  arch/unicore32/include/asm/syscall.h | 12 
>  arch/xtensa/include/asm/syscall.h|  7 +++
>  include/uapi/linux/audit.h   | 15 +++
>  include/uapi/linux/elf-em.h  |  7 +++
>  15 files changed, 110 insertions(+), 12 deletions(-)
>  create mode 100644 arch/m68k/include/asm/syscall.h
>  create mode 100644 arch/unicore32/include/asm/syscall.h

-- 
paul moore
www.paul-moore.com


Re: [PATCH] clk: Add (devm_)clk_get_optional() functions

2018-11-20 Thread Andy Shevchenko
On Tue, Nov 20, 2018 at 01:56:52PM +0100, Uwe Kleine-König wrote:
> On Tue, Nov 20, 2018 at 12:38:33PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote:
> > > + if (clk == ERR_PTR(-ENOENT))
> > > + return NULL;
> > > + else
> > > + return clk;
> > 
> > return clk == ERR_PTR(-ENOENT) ? NULL : clk;
> > 
> > ?
> 
> Not sure this adds to the readability of the expression. Personally I
> prefer the explicit if. Maybe even:
> 
>   clk = clk_get(...);
> 
>   if (clk == ERR_PTR(-ENOENT))
>   clk = NULL;
> 
>   return clk;

So, it almost repeats the initial variant.
I'm fine with no 'else' in initial code, like

if (...)
return NULL;

return clk;


-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH] clk: Add (devm_)clk_get_optional() functions

2018-11-20 Thread Uwe Kleine-König
Hello,

On Tue, Nov 20, 2018 at 12:38:33PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote:
> > +   if (clk == ERR_PTR(-ENOENT))
> > +   return NULL;
> > +   else
> > +   return clk;
> 
> return clk == ERR_PTR(-ENOENT) ? NULL : clk;
> 
> ?

Not sure this adds to the readability of the expression. Personally I
prefer the explicit if. Maybe even:

clk = clk_get(...);

if (clk == ERR_PTR(-ENOENT))
clk = NULL;

return clk;

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |


Re: [PATCH] clk: Add (devm_)clk_get_optional() functions

2018-11-20 Thread Andy Shevchenko
On Tue, Nov 20, 2018 at 10:53:33AM +, Phil Edworthy wrote:
> On 20 November 2018 10:39 Andy Shevchenko wrote:
> > On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote:
> > > This adds clk_get_optional() and devm_clk_get_optional() functions to
> > > get optional clocks.
> > > They behave the same as (devm_)clk_get except where there is no clock
> > > producer. In this case, instead of returning -ENOENT, the function
> > > returns NULL. This makes error checking simpler and allows
> > > clk_prepare_enable, etc to be called on the returned reference without
> > > additional checks.

> > >  - Instead of messing with the core functions, simply wrap them for the
> > >_optional() versions. By putting clk_get_optional() inline in the 
> > > header
> > >file, we can get rid of the arch specific patches as well.

> > Fine if it would have no surprises with error handling.
> There shouldn't be any surprises. My earlier attempts at implementing this
> were hampered by the fact that of_clk_get_by_name() can return -EINVAL
> in some circumstances. By directly wrapping the (devm_)clk_get() functions
> that problem goes away.

Good!

After my comments being addressed,
Reviewed-by: Andy Shevchenko 


> > > + if (ERR_PTR(-ENOENT))
> Huh? That wasn't the code I sent...

Yup, it's my wrong editing flow. Anyway, you got the idea.

> > > + return NULL;
> > > + else
> > > + return clk;

> > return clk == ERR_PTR(-ENOENT) ? NULL : clk;

-- 
With Best Regards,
Andy Shevchenko




RE: [PATCH] clk: Add (devm_)clk_get_optional() functions

2018-11-20 Thread Phil Edworthy
Hi Andy,

On 20 November 2018 10:39 Andy Shevchenko wrote:
> On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote:
> > This adds clk_get_optional() and devm_clk_get_optional() functions to
> > get optional clocks.
> > They behave the same as (devm_)clk_get except where there is no clock
> > producer. In this case, instead of returning -ENOENT, the function
> > returns NULL. This makes error checking simpler and allows
> > clk_prepare_enable, etc to be called on the returned reference without
> > additional checks.
> 
> >  - Instead of messing with the core functions, simply wrap them for the
> >_optional() versions. By putting clk_get_optional() inline in the header
> >file, we can get rid of the arch specific patches as well.
> 
> Fine if it would have no surprises with error handling.
There shouldn't be any surprises. My earlier attempts at implementing this
were hampered by the fact that of_clk_get_by_name() can return -EINVAL
in some circumstances. By directly wrapping the (devm_)clk_get() functions
that problem goes away.

> > +   if (ERR_PTR(-ENOENT))
Huh? That wasn't the code I sent...

> > +   return NULL;
> > +   else
> > +   return clk;
> 
> return clk == ERR_PTR(-ENOENT) ? NULL : clk;
> 
> ?
> 
> > +   if (clk == ERR_PTR(-ENOENT))
> > +   return NULL;
> > +   else
> > +   return clk;
> 
> Ditto.
Sure, will fix both.

Thanks
Phil


Re: [PATCH] clk: Add (devm_)clk_get_optional() functions

2018-11-20 Thread Andy Shevchenko
On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote:
> This adds clk_get_optional() and devm_clk_get_optional() functions to get
> optional clocks.
> They behave the same as (devm_)clk_get except where there is no clock
> producer. In this case, instead of returning -ENOENT, the function
> returns NULL. This makes error checking simpler and allows
> clk_prepare_enable, etc to be called on the returned reference
> without additional checks.

>  - Instead of messing with the core functions, simply wrap them for the
>_optional() versions. By putting clk_get_optional() inline in the header
>file, we can get rid of the arch specific patches as well.

Fine if it would have no surprises with error handling.

> + if (ERR_PTR(-ENOENT))
> + return NULL;
> + else
> + return clk;

return clk == ERR_PTR(-ENOENT) ? NULL : clk;

?

> + if (clk == ERR_PTR(-ENOENT))
> + return NULL;
> + else
> + return clk;

Ditto.


-- 
With Best Regards,
Andy Shevchenko




Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

2018-11-20 Thread Andreas Schwab
On Nov 20 2018, Linus Walleij  wrote:

> Yes you already see the same as I see: this chip MK68901 has
> no less than four timers. I bet the kernel is just using one of them,
> out of habit.

Note that not all timers can be used freely.  Some of them are hardwired
to generate the clock for the serial interfaces.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

2018-11-20 Thread Linus Walleij
On Tue, Nov 20, 2018 at 10:00 AM Finn Thain  wrote:

> If you use one timer as a clock event device and the other as a
> clocksource, there are no timers left to run the existing
> timer_interrupt() handler. So this arrangement would require
> GENERIC_CLOCKEVENTS=y, right?

I think so, yes.

> Then, wouldn't all relevant platforms have to support
> GENERIC_CLOCKEVENTS=y, if a single binary was to support all of those
> platforms?

Good point. I wonder of there is a path forward where we
could have peaceful GENERIC_CLOCKEVENTS and
!GENERIC_CLOCKEVENTS co-existence.

Yours,
Linus Walleij


Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

2018-11-20 Thread Linus Walleij
On Tue, Nov 20, 2018 at 10:30 AM Finn Thain  wrote:
> On Tue, 20 Nov 2018, Linus Walleij wrote:

> > As with the Amiga, this chip also has an RTC clock that should go to the
> > RTC subsystem, naturally.
>
> I think some Atari's have an MC146818, which is drivers/rtc/rtc-cmos.c,
> arch/alpha/kernel/rtc.c etc.

Indeed, some systems have more than one RTC, often one in their
SoC silicon and one battery backed elsewhere (like a dedicated
chip on I2C or inside a PMIC).

We usually just register more of them. The RTC subsystem can
handle any amount. It's up to userspace to figure out which RTC
to actually use, which is not so good, we should probably
have some heuristic like a quality indicator on them.

Yours,
Linus Walleij


Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

2018-11-20 Thread Finn Thain
On Tue, 20 Nov 2018, Linus Walleij wrote:

> 
> Yes you already see the same as I see: this chip MK68901 has no less 
> than four timers. I bet the kernel is just using one of them, out of 
> habit.
> 
> By just setting another timer as free-running we get a classic and clean 
> Linux clocksource for the Atari.
> 

These are all 8-bit timers. Whereas the smallest clocksource mask I can 
find with grep is 24-bits.

You can divide the oscillator down to 12288 Hz giving a maximum period of 
20 ms. My concern would be that clocksource counter wrap could still go 
undetected given a little interrupt latency.

> This is however a very good start in untangling the mess (as is the 
> whole patch series).
> 

It should be exciting to see what happens when some of these changes get 
tested 8-) I've only seen results for Mac and Atari so far.

> As with the Amiga, this chip also has an RTC clock that should go to the 
> RTC subsystem, naturally.
> 

I think some Atari's have an MC146818, which is drivers/rtc/rtc-cmos.c,
arch/alpha/kernel/rtc.c etc.

-- 

> Yours,
> Linus Walleij
> 


Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

2018-11-20 Thread Kars de Jong
Op ma 19 nov. 2018 om 02:10 schreef Finn Thain :
>
> hp300_gettimeoffset() never checks the timer interrupt flag and will
> fail to notice when the timer counter gets reloaded. That means the
> clock could jump backwards.
>
> Remove this code and leave this platform on the 'jiffies' clocksource.
> Note that this amounts to a regression in clock precision. However,
> adopting the 'jiffies' clocksource does resolve the monotonicity issue.
>
> Signed-off-by: Finn Thain 
> ---
> hp300_gettimeoffset() cannot be used in a clocksource conversion
> unless it can be made monotonic. I can't fix this without knowing the
> details of the timer implementation, such as the relationship between
> the timer count and the interrupt flag.

I don't really like this regression...

According to NetBSD sources, there are 3 timers in the chip
(originally an MC6840 PTM). Timer 1 is used as the system timer, timer
3 runs at the same rate and is unused on Linux (on NetBSD it is used
as the statistics/profiling timer), and timer 3 is connected to timer
2 so you can make a 32-bit timer out of the two timers together (also
unused on Linux).

Timers 1 counts down at 25 MHz. The interrupt flag is set when the
counter reaches 0 after which it is automatically reloaded and starts
counting down again.

> ---
>  arch/m68k/hp300/time.c | 19 ---
>  1 file changed, 19 deletions(-)


Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

2018-11-20 Thread Finn Thain
On Tue, 20 Nov 2018, Linus Walleij wrote:

> On Mon, Nov 19, 2018 at 2:22 AM Finn Thain  wrote:
> 
> > Add a platform clocksource by adapting the existing arch_gettimeoffset
> > implementation.
> >
> > Signed-off-by: Finn Thain 
> > Acked-by: Linus Walleij 
> > Tested-by: Stan Johnson 
> 
> As noted for the Amiga CIA (which is pretty much a sibling to this MOS 
> 6522 VIA) this chip also has two counters and could use one as 
> clocksource and the other as clock event.
> 
> Again I bet this is just using one timer out of habit.
> 
> It will be an easy feat to make a clean clocksource+clock event for this 
> hardware as well once this refactoring is complete.
> 

Right. Both timer 1 and timer 2 have a timed interrupt mode, and either 
could probably serve as a clock event device or a clocksource. Some Mac 
models have a second VIA with two more timers, but not all.

(As clock event devices, the longest interval you can program is about 83 
ms. As 783360 Hz, 16-bit clocksources, they wrap about 12 times every 
second.)

If you use one timer as a clock event device and the other as a 
clocksource, there are no timers left to run the existing 
timer_interrupt() handler. So this arrangement would require 
GENERIC_CLOCKEVENTS=y, right?

Then, wouldn't all relevant platforms have to support 
GENERIC_CLOCKEVENTS=y, if a single binary was to support all of those 
platforms?

-- 

> Yours,
> Linus Walleij
> 


Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

2018-11-20 Thread Geert Uytterhoeven
Hi Linus,

On Tue, Nov 20, 2018 at 9:10 AM Linus Walleij  wrote:
> As with the Amiga, this chip also has an RTC clock that should
> go to the RTC subsystem, naturally.

Please note the Amiga CIA is an 8520, not 6526, hence it has a 24-bit TOD
instead of a BCD TOD, so it's not suitable for use as an RTC.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

2018-11-20 Thread Linus Walleij
On Mon, Nov 19, 2018 at 2:22 AM Finn Thain  wrote:

> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Signed-off-by: Finn Thain 
> Acked-by: Linus Walleij 
> Tested-by: Stan Johnson 

As noted for the Amiga CIA (which is pretty much a sibling to this
MOS 6522 VIA) this chip also has two counters and could use one
as clocksource and the other as clock event.

Again I bet this is just using one timer out of habit.

It will be an easy feat to make a clean clocksource+clock event
for this hardware as well once this refactoring is complete.

Yours,
Linus Walleij


Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

2018-11-20 Thread Linus Walleij
On Mon, Nov 19, 2018 at 2:22 AM Finn Thain  wrote:

> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Normally the MFP timer C interrupt flag would be used to check for
> timer counter wrap-around. Unfortunately, that flag gets cleared by the
> MFP itself (due to automatic EOI mode). This means that
> mfp_timer_c_handler() and atari_read_clk() must race when accounting
> for counter wrap-around.
>
> That problem is avoided here by effectively stopping the clock when it
> might otherwise jump backwards. This harms clock accuracy; the result
> is not much better than the jiffies clocksource. Also, clock error is
> no longer uniformly distributed.
>
> Signed-off-by: Finn Thain 
> Acked-by: Linus Walleij 
> ---
> TODO: find a spare counter for the clocksource, rather than hanging
> it off the HZ timer.

Yes you already see the same as I see: this chip MK68901 has
no less than four timers. I bet the kernel is just using one of them,
out of habit.

By just setting another timer as free-running we get a classic
and clean Linux clocksource for the Atari.

This is however a very good start in untangling the mess
(as is the whole patch series).

As with the Amiga, this chip also has an RTC clock that should
go to the RTC subsystem, naturally.

Yours,
Linus Walleij


Re: [RFC PATCH v2 06/14] m68k: amiga: Convert to clocksource API

2018-11-20 Thread Linus Walleij
On Mon, Nov 19, 2018 at 2:22 AM Finn Thain  wrote:

> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Signed-off-by: Finn Thain 
> Acked-by: Linus Walleij 
> ---
> Changed since v1:
>  - Moved clk_total access to within the irq lock.

Came to think of it, Geert can probably answer to the use cases
for the CIAs in Linux: the Amiga CIA has two counters.

It would make sense to use one as a free-runing clocksource and
the other one as clock event. Then Linux is extremely happy
without any complex workarounds trying to use just one timer
for both jobs.

Is there some specific reason why we can't use both counters
like this, except for legacy? (I am thinking it would be an improvement
on top of Finn's series once they go in.)

Yours,
Linus Walleij