* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Arnd Bergmann @ 2010-08-19 15:48 UTC (permalink / raw)
To: Ira W. Snyder
Cc: john stultz, Richard Cochran, linuxppc-dev, linux-kernel, netdev,
Rodolfo Giometti, devicetree-discuss, linux-arm-kernel,
Krzysztof Halasa
In-Reply-To: <20100819152301.GB25193@ovro.caltech.edu>
On Thursday 19 August 2010, Ira W. Snyder wrote:
> Perhaps you were thinking of the vhost example (taken from
> drivers/vhost/Kconfig):
>
> config VHOST_NET
> tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)"
> depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP) && EXPERIMENTAL
>
> They have a similar construct with both TUN and MACVTAP there. Perhaps
> the parens are a necessary part of the "X || !X" syntax? Just a random
> guess.
Yes, that's the one I was thinking of.
My mistake was that the effect is slightly different here. VHOST and TUN are
both tristate. What we guarantee here is that if TUN is "m", VHOST cannot be "y",
because its dependency cannot be fulfilled for y.
Arnd
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Richard Cochran @ 2010-08-19 15:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rodolfo Giometti, john stultz, devicetree-discuss, linux-kernel,
netdev, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <201008191428.04546.arnd@arndb.de>
On Thu, Aug 19, 2010 at 02:28:04PM +0200, Arnd Bergmann wrote:
> My point was that a syscall is better than an ioctl based interface here,
> which I definitely still think. Given that John knows much more about
> clocks than I do, we still need to get agreement on the question that
> he raised, which is whether we actually need to expose this clock to the
> user or not.
>
> If we can find a way to sync system time accurate enough with PTP and
> PPS, user applications may not need to see two separate clocks at all.
At the very least, one user application (the PTPd) needs to see the
PTP clock.
> > SYSCALL_DEFINE3(clock_adjtime, const clockid_t, clkid,
> > int, ppb, struct timespec __user *, ts)
> >
> > ppb - desired frequency adjustment in parts per billion
> > ts - desired time step (or jump) in <sec,nsec> to correct
> > a measured offset
> >
> > Arguably, this syscall might be useful for other clocks, too.
>
> This is a mix of adjtime and adjtimex with the addition of
> the clkid parameter, right?
Sort of, but not really. ADJTIME(3) takes an offset and slowly
corrects the clock using a servo in the kernel, over hours.
For this function, the offset passed in the 'ts' parameter will be
immediately corrected, by jumping to the new time. This reflects the
way that PTP works. After the first few samples, the PTPd has an
estimate of the offset to the master and the rate difference. The PTPd
can proceed in one of two ways.
1. If resetting the clock is not desired, then the clock is set to the
maximum adjustment (in the right direction) until the clock time is
close to the master's time.
2. The estimated offset is added to the current time, resulting in a
jump in time.
We need clock_adjtime(id, 0, ts) for the second case.
> Have you considered passing a struct timex instead of ppb and ts?
Yes, but the timex is not suitable, IMHO.
> Is using ppb instead of the timex ppm required to get the accuracy
> you want?
That is one very good reason.
Another is this: can you explain what the 20+ fields mean?
Consider the field, freq. The comment says "frequency offset (scaled
ppm)." To what is it scaled? The only way I know of to find out is to
read the NTP code (which is fairly complex) and see what the unit
really is meant to be. Ditto for the other fields.
The timex structure reveals, AFAICT, the inner workings of the kernel
clock servo. For PTP, we don't need or want the kernel servo. The PTPd
has its own clock servo in user space.
Richard
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Ira W. Snyder @ 2010-08-19 15:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: john stultz, Richard Cochran, linuxppc-dev, linux-kernel, netdev,
Rodolfo Giometti, devicetree-discuss, linux-arm-kernel,
Krzysztof Halasa
In-Reply-To: <201008191429.50008.arnd@arndb.de>
On Thu, Aug 19, 2010 at 02:29:49PM +0200, Arnd Bergmann wrote:
> On Thursday 19 August 2010, Richard Cochran wrote:
> >
> > On Wed, Aug 18, 2010 at 05:02:03PM +0200, Arnd Bergmann wrote:
> > > You might want to use callbacks for these system calls that you
> > > can register to at module load time, like it is done for the
> > > existing syscalls.
> >
> > Can you point me to a specific example?
>
> The struct k_clock contains callback functions to clock source
> specific implementations of the syscalls and other functions.
> Adding a new clock source that is (potentially) implemented
> as a module means you have to define a new k_clock in that module
> that you register using register_posix_clock.
>
> If you need additional syscalls to be implemented by the same
> module, you can put more callback functions into struct k_clock
> that would then only be implemented by the new module.
>
> > > The simpler way (e.g. for testing) is using Kconfig dependencies, like
> > >
> > > config PTP
> > > bool "IEEE 1588 Precision Time Protocol"
> > >
> > > config PPS
> > > tristate "Pulse per Second"
> > > depends on PTP || !PTP
> > >
> > > The depends statement is a way of expressing that when PTP is enabled,
> > > PPS cannot be a module, while it may be a module if PTP is disabled.
> >
> > THis did not work for me.
> >
> > What I got was, in effect, two independent booleans.
>
> Hmm, right. I wonder what I was thinking of then.
>
> You can certainly do
>
> config PTP
> bool "IEEE 1588 Precision Time Protocol"
> depends on PPS != m
>
> but that will hide the PTP option if PPS is set to 'm', which is normally
> not what you want.
>
Arnd,
Perhaps you were thinking of the vhost example (taken from
drivers/vhost/Kconfig):
config VHOST_NET
tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)"
depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP) && EXPERIMENTAL
They have a similar construct with both TUN and MACVTAP there. Perhaps
the parens are a necessary part of the "X || !X" syntax? Just a random
guess.
Ira
^ permalink raw reply
* [PATCH] [powerpc] Wire up fanotify_init, fanotify_mark, prlimit64 syscalls
From: Andreas Schwab @ 2010-08-19 15:15 UTC (permalink / raw)
To: Linuxppc-dev
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/powerpc/include/asm/systbl.h | 3 +++
arch/powerpc/include/asm/unistd.h | 5 ++++-
arch/powerpc/kernel/sys_ppc32.c | 8 ++++++++
3 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index a5ee345..3d21266 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -326,3 +326,6 @@ SYSCALL_SPU(perf_event_open)
COMPAT_SYS_SPU(preadv)
COMPAT_SYS_SPU(pwritev)
COMPAT_SYS(rt_tgsigqueueinfo)
+SYSCALL(fanotify_init)
+COMPAT_SYS(fanotify_mark)
+SYSCALL_SPU(prlimit64)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index f0a1026..597e6f9 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -345,10 +345,13 @@
#define __NR_preadv 320
#define __NR_pwritev 321
#define __NR_rt_tgsigqueueinfo 322
+#define __NR_fanotify_init 323
+#define __NR_fanotify_mark 324
+#define __NR_prlimit64 325
#ifdef __KERNEL__
-#define __NR_syscalls 323
+#define __NR_syscalls 326
#define __NR__exit __NR_exit
#define NR_syscalls __NR_syscalls
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index 20fd701..b1b6043 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -616,3 +616,11 @@ asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags,
return sys_sync_file_range(fd, offset, nbytes, flags);
}
+
+asmlinkage long compat_sys_fanotify_mark(int fanotify_fd, unsigned int flags,
+ unsigned mask_hi, unsigned mask_lo,
+ int dfd, const char __user *pathname)
+{
+ u64 mask = ((u64)mask_hi << 32) | mask_lo;
+ return sys_fanotify_mark(fanotify_fd, flags, mask, dfd, pathname);
+}
--
1.7.2.1
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH 3/4] powerpc/4xx: Index interrupt stacks by physical cpu
From: Josh Boyer @ 2010-08-19 12:34 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev list
In-Reply-To: <1282149866-4710-4-git-send-email-shaggy@linux.vnet.ibm.com>
On Wed, Aug 18, 2010 at 11:44:25AM -0500, Dave Kleikamp wrote:
>The interrupt stacks need to be indexed by the physical cpu since the
>critical, debug and machine check handlers use the contents of SPRN_PIR to
>index the critirq_ctx, dbgirq_ctx, and mcheckirq_ctx arrays.
>
>Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>---
> arch/powerpc/kernel/irq.c | 15 ++++++++-------
> arch/powerpc/kernel/setup_32.c | 9 +++++----
> 2 files changed, 13 insertions(+), 11 deletions(-)
>
>diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
>index d3ce67c..52e9c95 100644
>--- a/arch/powerpc/kernel/irq.c
>+++ b/arch/powerpc/kernel/irq.c
>@@ -446,22 +446,23 @@ struct thread_info *mcheckirq_ctx[NR_CPUS] __read_mostly;
> void exc_lvl_ctx_init(void)
> {
> struct thread_info *tp;
>- int i;
>+ int i, hw_cpu;
>
> for_each_possible_cpu(i) {
>- memset((void *)critirq_ctx[i], 0, THREAD_SIZE);
>- tp = critirq_ctx[i];
>+ hw_cpu = get_hard_smp_processor_id(i);
This one throws this compile error when trying to build
ppc44x_defconfig:
CC arch/powerpc/kernel/irq.o
arch/powerpc/kernel/irq.c: In function 'exc_lvl_ctx_init':
arch/powerpc/kernel/irq.c:452: error: implicit declaration of function
'get_hard_smp_processor_id'
make[1]: *** [arch/powerpc/kernel/irq.o] Error 1
make: *** [arch/powerpc/kernel] Error 2
I'm guessing it needs a #include <asm/smp.h> added to it.
josh
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Arnd Bergmann @ 2010-08-19 12:29 UTC (permalink / raw)
To: Richard Cochran
Cc: Rodolfo Giometti, john stultz, devicetree-discuss, linux-kernel,
netdev, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <20100819092246.GA8484@riccoc20.at.omicron.at>
On Thursday 19 August 2010, Richard Cochran wrote:
>
> On Wed, Aug 18, 2010 at 05:02:03PM +0200, Arnd Bergmann wrote:
> > You might want to use callbacks for these system calls that you
> > can register to at module load time, like it is done for the
> > existing syscalls.
>
> Can you point me to a specific example?
The struct k_clock contains callback functions to clock source
specific implementations of the syscalls and other functions.
Adding a new clock source that is (potentially) implemented
as a module means you have to define a new k_clock in that module
that you register using register_posix_clock.
If you need additional syscalls to be implemented by the same
module, you can put more callback functions into struct k_clock
that would then only be implemented by the new module.
> > The simpler way (e.g. for testing) is using Kconfig dependencies, like
> >
> > config PTP
> > bool "IEEE 1588 Precision Time Protocol"
> >
> > config PPS
> > tristate "Pulse per Second"
> > depends on PTP || !PTP
> >
> > The depends statement is a way of expressing that when PTP is enabled,
> > PPS cannot be a module, while it may be a module if PTP is disabled.
>
> THis did not work for me.
>
> What I got was, in effect, two independent booleans.
Hmm, right. I wonder what I was thinking of then.
You can certainly do
config PTP
bool "IEEE 1588 Precision Time Protocol"
depends on PPS != m
but that will hide the PTP option if PPS is set to 'm', which is normally
not what you want.
Arnd
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Arnd Bergmann @ 2010-08-19 12:28 UTC (permalink / raw)
To: Richard Cochran
Cc: Rodolfo Giometti, john stultz, devicetree-discuss, linux-kernel,
netdev, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <20100819055518.GA4084@riccoc20.at.omicron.at>
On Thursday 19 August 2010, Richard Cochran wrote:
> On Wed, Aug 18, 2010 at 05:12:56PM -0700, john stultz wrote:
> > On Wed, 2010-08-18 at 09:19 +0200, Richard Cochran wrote:
> > > The timer/alarm stuff is "ancillary" and is not at all necessary. It
> > > is just a "nice to have." I will happily remove it, if it is too
> > > troubling for people.
> >
> > If there's a compelling argument for it, I'm interested to hear. But
> > again, it seems like just
> > yet-another-way-to-get-alarm/timer-functionality, so before we add an
> > extra API (or widen an existing API) I'd like to understand the need.
>
> We don't really need it, IMHO.
>
> But if we offer clockid_t CLOCK_PTP, then we get timer_settime()
> without any extra effort.
Makes sense.
> > So in thinking about this, try to focus on what the new clock_id
> > provides that the other existing clockids do not? Are they at comparable
> > levels of abstraction? 15 years from now, are folks likely to still be
> > using it? Will it be maintainable? etc...
>
> Arnd convinced me that clockid_t=CLOCK_PTP is a good fit.
My point was that a syscall is better than an ioctl based interface here,
which I definitely still think. Given that John knows much more about
clocks than I do, we still need to get agreement on the question that
he raised, which is whether we actually need to expose this clock to the
user or not.
If we can find a way to sync system time accurate enough with PTP and
PPS, user applications may not need to see two separate clocks at all.
> My plan would be to introduce just one additional syscall:
>
> SYSCALL_DEFINE3(clock_adjtime, const clockid_t, clkid,
> int, ppb, struct timespec __user *, ts)
>
> ppb - desired frequency adjustment in parts per billion
> ts - desired time step (or jump) in <sec,nsec> to correct
> a measured offset
>
> Arguably, this syscall might be useful for other clocks, too.
This is a mix of adjtime and adjtimex with the addition of
the clkid parameter, right?
Have you considered passing a struct timex instead of ppb and ts?
Is using ppb instead of the timex ppm required to get the accuracy
you want?
Arnd
^ permalink raw reply
* Re: 64-bit ppc rwsem
From: Benjamin Herrenschmidt @ 2010-08-19 10:24 UTC (permalink / raw)
To: David Miller
Cc: torvalds, paulus, linux-kernel, sparclinux, akpm, linuxppc-dev
In-Reply-To: <20100818.222925.233689776.davem@davemloft.net>
On Wed, 2010-08-18 at 22:29 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Thu, 19 Aug 2010 15:23:23 +1000
>
> > Similar here, but using atomic_long_t instead so it works for 32-bit too
> > for me. I suppose we could make that part common indeed.
> >
> > What about asm-generic/rwsem-atomic.h or rwsem-cmpxchg.h ?
>
> Using rwsem-cmpxchg.h sounds best I guess.
Ok, I'll send a new patch tomorrow that does that.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Richard Cochran @ 2010-08-19 9:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rodolfo Giometti, john stultz, devicetree-discuss, linux-kernel,
netdev, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <201008181702.03384.arnd@arndb.de>
On Wed, Aug 18, 2010 at 05:02:03PM +0200, Arnd Bergmann wrote:
> You might want to use callbacks for these system calls that you
> can register to at module load time, like it is done for the
> existing syscalls.
Can you point me to a specific example?
> The simpler way (e.g. for testing) is using Kconfig dependencies, like
>
> config PTP
> bool "IEEE 1588 Precision Time Protocol"
>
> config PPS
> tristate "Pulse per Second"
> depends on PTP || !PTP
>
> The depends statement is a way of expressing that when PTP is enabled,
> PPS cannot be a module, while it may be a module if PTP is disabled.
THis did not work for me.
What I got was, in effect, two independent booleans.
Thanks,
Richard
^ permalink raw reply
* mpc8xx microcode patch?
From: Shawn Jin @ 2010-08-19 7:56 UTC (permalink / raw)
To: ppcdev
Hi,
The kernel supports three microcode patch: USB_SOF_UCODE_PATCH,
I2C_SPI_UCODE_PATCH, and I2C_SPI_SMC1_UCODE_PATCH. However from the
mpc8xxmc01.zip downloaded from freescale website, there are 4
combinations of microcode (I2C/SPI, SMC/SPI, SMC/I2C, and SMC). Also
in the EB662.pdf it's mentioned that SMC relocation patch is
applicable to all MPC8xx families and revisions. Surprisingly the SMC
only relocation patch cannot be found in the kernel. My processor is
MPC870 and I'm not sure what ucode patch I shall apply.
What am I missing here? This EB662.pdf was published in Jan. 2006.
It's hard to believe that the kernel hasn't incorporate what needs to
be done.
Thanks,
-Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/85xx: Cleanup QE initialization for MPC85xxMDS boards
From: Anton Vorontsov @ 2010-08-19 6:45 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <AANLkTinOz72GNpVD=+f5n7mMaHpoEeJ4QMwAON9nnu2A@mail.gmail.com>
On Wed, Aug 18, 2010 at 02:31:42PM -0500, Timur Tabi wrote:
> On Tue, Jun 8, 2010 at 2:55 PM, Anton Vorontsov <avorontsov@mvista.com> wrote:
> > The mpc85xx_mds_setup_arch() function is incomprehensible
> > and unmaintainable. Factor out all QE specific stuff into
> > mpc85xx_mds_qe_init() and mpc85xx_mds_reset_ucc_phys().
> >
> > Also move QE stuff out of mpc85xx_mds_pic_init().
> >
> > The diff is unreadable, but only because the code was so. ;-)
> > It should be better now, and less indented.
> >
> > Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
> > ---
>
> This patch introduces breaks mpc85xx_smp_defconfig:
>
> CC arch/powerpc/platforms/85xx/mpc85xx_mds.o
> arch/powerpc/platforms/85xx/mpc85xx_mds.c: In function 'mpc85xx_mds_setup_arch':
> arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: 'np' undeclared
> (first use in this function)
> arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: (Each undeclared
> identifier is reported only once
> arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: for each
> function it appears in.)
Thanks for the report, apparently I tested my patch without
CONFIG_PCI... But the issue should be already fixed by
powerpc/85xx: Fix compile error in mpc85xx_mds.c
http://patchwork.ozlabs.org/patch/60933/
(Though, not in Linus' tree yet.)
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Richard Cochran @ 2010-08-19 5:55 UTC (permalink / raw)
To: john stultz
Cc: Rodolfo Giometti, Arnd Bergmann, netdev, devicetree-discuss,
linux-kernel, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <1282176776.2865.100.camel@localhost.localdomain>
On Wed, Aug 18, 2010 at 05:12:56PM -0700, john stultz wrote:
> On Wed, 2010-08-18 at 09:19 +0200, Richard Cochran wrote:
> > The timer/alarm stuff is "ancillary" and is not at all necessary. It
> > is just a "nice to have." I will happily remove it, if it is too
> > troubling for people.
>
> If there's a compelling argument for it, I'm interested to hear. But
> again, it seems like just
> yet-another-way-to-get-alarm/timer-functionality, so before we add an
> extra API (or widen an existing API) I'd like to understand the need.
We don't really need it, IMHO.
But if we offer clockid_t CLOCK_PTP, then we get timer_settime()
without any extra effort.
> > I was emulating the posix interface. Instead I should use it directly.
>
> I'm definitely interested to see what you come up with here. I'm still
> hesitant with adding a PTP clock_id, but extending the posix-clocks
> interface in this way isn't unprecedented (see: CLOCK_SGI_CYCLE) I just
> would like to make sure we don't end up with a clock_id namespace
> littered with oddball clocks that were not well abstracted (see:
> CLOCK_SGI_CYCLE :).
>
> For instance: imagine if instead of keeping the clocksource abstraction
> internal to the timekeeping core, we exposed each clocksource to
> userland via a clock_id. Every arch would have different ids, and each
> arch might have multiple ids. Programming against that would be a huge
> pain.
The clockid_t CLOCK_PTP will be arch-neutral.
> So in thinking about this, try to focus on what the new clock_id
> provides that the other existing clockids do not? Are they at comparable
> levels of abstraction? 15 years from now, are folks likely to still be
> using it? Will it be maintainable? etc...
Arnd convinced me that clockid_t=CLOCK_PTP is a good fit. My plan
would be to introduce just one additional syscall:
SYSCALL_DEFINE3(clock_adjtime, const clockid_t, clkid,
int, ppb, struct timespec __user *, ts)
ppb - desired frequency adjustment in parts per billion
ts - desired time step (or jump) in <sec,nsec> to correct
a measured offset
Arguably, this syscall might be useful for other clocks, too.
I think the ancillary features from PTP hardware clocks should be made
available throught the sysfs. A syscall for these would end up very
ugly, looking like an ioctl. Also, it is hard to see how these
features relate to the more general idea of the clockid.
In contrast, sysfs attributes will fit the need nicely:
1. enable or disable pps
2. enable or disable external timestamps
3. read out external timestamp
4. configure period for periodic output
> > 1. Use Case: SW timestamping
> The way I tend to see it: PTP is just one of the many ways to sync
> system time.
> > 2. Use Case: HW timestamping for industrial control
> These specialized applications are part of what concerns me the most.
PTP was not invented to just to get a computer's system time in the
ball park. For that, NTP is good enough. Rather, some people want to
use their computers for tasks that require close synchronization, like
industrial control, audio/video streaming, and many others.
Are you saying that we should not support such applications?
> For example, I can see some parallels between things like audio
> processing, where you have a buffer consumed by the card at a certain
> rate. Now, the card has its own crystal it uses to time its consumption,
> so it has its own time domain, and could drift from system time. Thus
> you want to trigger buffer-refill interrupts off of the audio card's
> clock, not the system time which might run the risk of being late.
>
> But again, we don't expose the audio hardware clock to userland in the
> same way we expose system time.
This is a good example of the poverty (in regards to time
synchronization) of our current systems.
Lets say I want to build a surround sound audio system, using a set of
distributed computers, each host connected to one speaker. How I can
be sure that the samples in one channel (ie one host) pass through the
DA converter at exactly the same time?
> Again, my knowledge in the networking stack is pretty limited. But it
> would seem that having an interface that does something to the effect of
> "adjust the timestamp clock on the hardware that generated it from this
> packet by Xppb" would feel like the right level of abstraction. Its
> closely related to SO_TIMESTAMP, option right? Would something like
> using the setsockopt/getsockopt interface with
> SO_TIMESTAMP_ADJUST/OFFSET/SET/etc be reasonable?
The clock and its adjustment have nothing to do with a network
socket. The current PTP hacks floating around all add private ioctls
to the MAC driver. That is the *wrong* way to do it.
> > 3. Use Case: HW timestamping with PPS to host
...
> And yes, this seems perfectly reasonable feature to add. Its not
> controversial to me, because its likely to work within the existing
> interfaces and won't expose anything new to userland.
Okay, then will you support an elegant solution for case 3, that also
supports cases 1 and 2 without any additional work?
> Again, sorry to be such a pain about all of this. I know its frustrating...
Thanks for your comments!
Richard
^ permalink raw reply
* Re: 64-bit ppc rwsem
From: David Miller @ 2010-08-19 5:29 UTC (permalink / raw)
To: benh; +Cc: torvalds, paulus, linux-kernel, sparclinux, akpm, linuxppc-dev
In-Reply-To: <1282195403.22370.296.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Thu, 19 Aug 2010 15:23:23 +1000
> Similar here, but using atomic_long_t instead so it works for 32-bit too
> for me. I suppose we could make that part common indeed.
>
> What about asm-generic/rwsem-atomic.h or rwsem-cmpxchg.h ?
Using rwsem-cmpxchg.h sounds best I guess.
^ permalink raw reply
* Re: 64-bit ppc rwsem
From: Benjamin Herrenschmidt @ 2010-08-19 5:23 UTC (permalink / raw)
To: David Miller
Cc: torvalds, paulus, linux-kernel, sparclinux, akpm, linuxppc-dev
In-Reply-To: <20100817.222818.193699062.davem@davemloft.net>
On Tue, 2010-08-17 at 22:28 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Wed, 18 Aug 2010 15:03:23 +1000
>
> > I tried various tricks but so far they didn't work. I'll have another
> > look tomorrow, but I may end up having to keep all the crap typecasts.
>
> The casts are pretty much unavoidable.
>
> Here's what I'm going to end up using on sparc64:
Similar here, but using atomic_long_t instead so it works for 32-bit too
for me. I suppose we could make that part common indeed.
What about asm-generic/rwsem-atomic.h or rwsem-cmpxchg.h ?
Below is my current patch, seems to boot fine here so far.
Cheers,
Ben
Subject: [PATCH] powerpc: Make rwsem use "long" type
This makes the 64-bit kernel use 64-bit signed integers for the counter
(effectively supporting 32-bit of active count in the semaphore), thus
avoiding things like overflow of the mmap_sem if you use a really crazy
number of threads
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/include/asm/rwsem.h | 64 ++++++++++++++++++++++----------------
1 files changed, 37 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/include/asm/rwsem.h b/arch/powerpc/include/asm/rwsem.h
index 24cd928..8447d89 100644
--- a/arch/powerpc/include/asm/rwsem.h
+++ b/arch/powerpc/include/asm/rwsem.h
@@ -21,15 +21,20 @@
/*
* the semaphore definition
*/
-struct rw_semaphore {
- /* XXX this should be able to be an atomic_t -- paulus */
- signed int count;
-#define RWSEM_UNLOCKED_VALUE 0x00000000
-#define RWSEM_ACTIVE_BIAS 0x00000001
-#define RWSEM_ACTIVE_MASK 0x0000ffff
-#define RWSEM_WAITING_BIAS (-0x00010000)
+#ifdef CONFIG_PPC64
+# define RWSEM_ACTIVE_MASK 0xffffffffL
+#else
+# define RWSEM_ACTIVE_MASK 0x0000ffffL
+#endif
+
+#define RWSEM_UNLOCKED_VALUE 0x00000000L
+#define RWSEM_ACTIVE_BIAS 0x00000001L
+#define RWSEM_WAITING_BIAS (-RWSEM_ACTIVE_MASK-1)
#define RWSEM_ACTIVE_READ_BIAS RWSEM_ACTIVE_BIAS
#define RWSEM_ACTIVE_WRITE_BIAS (RWSEM_WAITING_BIAS + RWSEM_ACTIVE_BIAS)
+
+struct rw_semaphore {
+ long count;
spinlock_t wait_lock;
struct list_head wait_list;
#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -43,9 +48,13 @@ struct rw_semaphore {
# define __RWSEM_DEP_MAP_INIT(lockname)
#endif
-#define __RWSEM_INITIALIZER(name) \
- { RWSEM_UNLOCKED_VALUE, __SPIN_LOCK_UNLOCKED((name).wait_lock), \
- LIST_HEAD_INIT((name).wait_list) __RWSEM_DEP_MAP_INIT(name) }
+#define __RWSEM_INITIALIZER(name) \
+{ \
+ RWSEM_UNLOCKED_VALUE, \
+ __SPIN_LOCK_UNLOCKED((name).wait_lock), \
+ LIST_HEAD_INIT((name).wait_list) \
+ __RWSEM_DEP_MAP_INIT(name) \
+}
#define DECLARE_RWSEM(name) \
struct rw_semaphore name = __RWSEM_INITIALIZER(name)
@@ -70,13 +79,13 @@ extern void __init_rwsem(struct rw_semaphore *sem, const char *name,
*/
static inline void __down_read(struct rw_semaphore *sem)
{
- if (unlikely(atomic_inc_return((atomic_t *)(&sem->count)) <= 0))
+ if (unlikely(atomic_long_inc_return((atomic_long_t *)&sem->count) <= 0))
rwsem_down_read_failed(sem);
}
static inline int __down_read_trylock(struct rw_semaphore *sem)
{
- int tmp;
+ long tmp;
while ((tmp = sem->count) >= 0) {
if (tmp == cmpxchg(&sem->count, tmp,
@@ -92,10 +101,10 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
*/
static inline void __down_write_nested(struct rw_semaphore *sem, int subclass)
{
- int tmp;
+ long tmp;
- tmp = atomic_add_return(RWSEM_ACTIVE_WRITE_BIAS,
- (atomic_t *)(&sem->count));
+ tmp = atomic_long_add_return(RWSEM_ACTIVE_WRITE_BIAS,
+ (atomic_long_t *)&sem->count);
if (unlikely(tmp != RWSEM_ACTIVE_WRITE_BIAS))
rwsem_down_write_failed(sem);
}
@@ -107,7 +116,7 @@ static inline void __down_write(struct rw_semaphore *sem)
static inline int __down_write_trylock(struct rw_semaphore *sem)
{
- int tmp;
+ long tmp;
tmp = cmpxchg(&sem->count, RWSEM_UNLOCKED_VALUE,
RWSEM_ACTIVE_WRITE_BIAS);
@@ -119,9 +128,9 @@ static inline int __down_write_trylock(struct rw_semaphore *sem)
*/
static inline void __up_read(struct rw_semaphore *sem)
{
- int tmp;
+ long tmp;
- tmp = atomic_dec_return((atomic_t *)(&sem->count));
+ tmp = atomic_long_dec_return((atomic_long_t *)&sem->count);
if (unlikely(tmp < -1 && (tmp & RWSEM_ACTIVE_MASK) == 0))
rwsem_wake(sem);
}
@@ -131,17 +140,17 @@ static inline void __up_read(struct rw_semaphore *sem)
*/
static inline void __up_write(struct rw_semaphore *sem)
{
- if (unlikely(atomic_sub_return(RWSEM_ACTIVE_WRITE_BIAS,
- (atomic_t *)(&sem->count)) < 0))
+ if (unlikely(atomic_long_sub_return(RWSEM_ACTIVE_WRITE_BIAS,
+ (atomic_long_t *)&sem->count) < 0))
rwsem_wake(sem);
}
/*
* implement atomic add functionality
*/
-static inline void rwsem_atomic_add(int delta, struct rw_semaphore *sem)
+static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
{
- atomic_add(delta, (atomic_t *)(&sem->count));
+ atomic_long_add(delta, (atomic_long_t *)&sem->count);
}
/*
@@ -149,9 +158,10 @@ static inline void rwsem_atomic_add(int delta, struct rw_semaphore *sem)
*/
static inline void __downgrade_write(struct rw_semaphore *sem)
{
- int tmp;
+ long tmp;
- tmp = atomic_add_return(-RWSEM_WAITING_BIAS, (atomic_t *)(&sem->count));
+ tmp = atomic_long_add_return(-RWSEM_WAITING_BIAS,
+ (atomic_long_t *)&sem->count);
if (tmp < 0)
rwsem_downgrade_wake(sem);
}
@@ -159,14 +169,14 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
/*
* implement exchange and add functionality
*/
-static inline int rwsem_atomic_update(int delta, struct rw_semaphore *sem)
+static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
{
- return atomic_add_return(delta, (atomic_t *)(&sem->count));
+ return atomic_long_add_return(delta, (atomic_long_t *)&sem->count);
}
static inline int rwsem_is_locked(struct rw_semaphore *sem)
{
- return (sem->count != 0);
+ return sem->count != 0;
}
#endif /* __KERNEL__ */
^ permalink raw reply related
* [PATCH v2 2/3] Prevent multiple reservations for cpu-release-addr
From: Matthew McClintock @ 2010-08-19 4:56 UTC (permalink / raw)
To: kexec
Cc: muvarov, linuxppc-dev, Kumar Gala, Sebastian Andrzej Siewior,
Matthew McClintock
In-Reply-To: <20100819042023.GC13965@verge.net.au>
Currently, we can add a lot of reservations over a small range, this
does a simple check to verify the previous entry is not the same
as the current one and skips it if so
Signed-off-by: Matthew McClintock <msm@freescale.com>
---
kexec/arch/ppc/fixup_dtb.c | 28 +++++++++++++++++-----------
1 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/kexec/arch/ppc/fixup_dtb.c b/kexec/arch/ppc/fixup_dtb.c
index 09f9ac1..910a3f0 100644
--- a/kexec/arch/ppc/fixup_dtb.c
+++ b/kexec/arch/ppc/fixup_dtb.c
@@ -139,6 +139,7 @@ static void fixup_reserve_regions(struct kexec_info *info, char *blob_buf, off_t
{
int ret, i;
int nodeoffset;
+ u64 val = 0;
/* If this is a KEXEC kernel we add all regions since they will
* all need to be saved */
@@ -175,20 +176,25 @@ static void fixup_reserve_regions(struct kexec_info *info, char *blob_buf, off_t
while (nodeoffset != -FDT_ERR_NOTFOUND) {
const void *buf;
int sz, ret;
- u64 val = 0;
+ u64 tmp;
buf = fdt_getprop(blob_buf, nodeoffset, "cpu-release-addr", &sz);
- if (sz == 4) {
- val = *(u32 *)buf;
- } else if (sz == 8) {
- val = *(u64 *)buf;
- }
- if (val) {
- ret = fdt_add_mem_rsv(blob_buf, PAGE_ALIGN(val-PAGE_SIZE), PAGE_SIZE);
- if (ret)
- printf("%s: Unable to add reserve for cpu-release-addr!\n",
- fdt_strerror(ret));
+ if (buf) {
+ if (sz == 4) {
+ tmp = *(u32 *)buf;
+ } else if (sz == 8) {
+ tmp = *(u64 *)buf;
+ }
+
+ /* crude check to see if last value is repeated */
+ if (_ALIGN_DOWN(tmp, PAGE_SIZE) != _ALIGN_DOWN(val, PAGE_SIZE)) {
+ val = tmp;
+ ret = fdt_add_mem_rsv(blob_buf, _ALIGN_DOWN(val, PAGE_SIZE), PAGE_SIZE);
+ if (ret)
+ printf("%s: Unable to add reserve for cpu-release-addr!\n",
+ fdt_strerror(ret));
+ }
}
nodeoffset = fdt_node_offset_by_prop_value(blob_buf, nodeoffset,
--
1.6.0.6
^ permalink raw reply related
* [PATCH v2 3/3] Prevent initrd-start and initrd-end from appearing multiple times
From: Matthew McClintock @ 2010-08-19 4:56 UTC (permalink / raw)
To: kexec
Cc: muvarov, linuxppc-dev, Kumar Gala, Sebastian Andrzej Siewior,
Matthew McClintock
In-Reply-To: <20100819042023.GC13965@verge.net.au>
We always remove the old entry, and add it back if it is needed
on for the kexec'ed kernel
Signed-off-by: Matthew McClintock <msm@freescale.com>
---
kexec/arch/ppc/fixup_dtb.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/kexec/arch/ppc/fixup_dtb.c b/kexec/arch/ppc/fixup_dtb.c
index 910a3f0..205fd77 100644
--- a/kexec/arch/ppc/fixup_dtb.c
+++ b/kexec/arch/ppc/fixup_dtb.c
@@ -160,7 +160,7 @@ static void fixup_reserve_regions(struct kexec_info *info, char *blob_buf, off_t
goto out;
}
}
- } else {
+ } else if (ramdisk || reuse_initrd) {
/* Otherwise we just add back the ramdisk and the device tree
* is already in the list */
ret = fdt_add_mem_rsv(blob_buf, ramdisk_base, ramdisk_size);
@@ -300,13 +300,16 @@ static void fixup_initrd(char *blob_buf)
nodeoffset = fdt_path_offset(blob_buf, "/chosen");
- if ((reuse_initrd || ramdisk) &&
- ((ramdisk_base != 0) && (ramdisk_size != 0))) {
- if (nodeoffset < 0) {
- printf("fdt_initrd: %s\n", fdt_strerror(nodeoffset));
- return;
- }
+ if (nodeoffset < 0) {
+ printf("fdt_initrd: %s\n", fdt_strerror(nodeoffset));
+ return;
+ }
+
+ fdt_delprop(blob_buf, nodeoffset, "linux,initrd-start");
+ fdt_delprop(blob_buf, nodeoffset, "linux,initrd-end");
+ if ((reuse_initrd || ramdisk) &&
+ ((ramdisk_base != 0) && (ramdisk_size != 0))) {
tmp = ramdisk_base;
err = fdt_setprop(blob_buf, nodeoffset,
"linux,initrd-start", &tmp, sizeof(tmp));
@@ -325,9 +328,6 @@ static void fixup_initrd(char *blob_buf)
fdt_strerror(err));
return;
}
- } else {
- fdt_delprop(blob_buf, nodeoffset, "linux,initrd-start");
- fdt_delprop(blob_buf, nodeoffset, "linux,initrd-end");
}
}
--
1.6.0.6
^ permalink raw reply related
* [PATCH v2 0/3] Misc. bug fixes for ppc32
From: Matthew McClintock @ 2010-08-19 4:56 UTC (permalink / raw)
To: kexec
Cc: muvarov, linuxppc-dev, Kumar Gala, Sebastian Andrzej Siewior,
Matthew McClintock
In-Reply-To: <20100819042023.GC13965@verge.net.au>
This patch series fixes a few issues I have discovered with my previous
patch series. Nothing new has been added.
v2: Missed signoff, removed initializing variable twice
Matthew McClintock (3):
Ramdisk address was not copied correctly on kexec'ed kernel
Prevent multiple reservations for cpu-release-addr
Prevent initrd-start and initrd-end from appearing multiple times
kexec/arch/ppc/fixup_dtb.c | 50 ++++++++++++++++++++++++-------------------
kexec/arch/ppc/kexec-ppc.c | 2 +-
2 files changed, 29 insertions(+), 23 deletions(-)
^ permalink raw reply
* [PATCH v2 1/3] Ramdisk address was not copied correctly on kexec'ed kernel
From: Matthew McClintock @ 2010-08-19 4:56 UTC (permalink / raw)
To: kexec
Cc: muvarov, linuxppc-dev, Kumar Gala, Sebastian Andrzej Siewior,
Matthew McClintock
In-Reply-To: <20100819042023.GC13965@verge.net.au>
Signed-off-by: Matthew McClintock <msm@freescale.com>
---
kexec/arch/ppc/fixup_dtb.c | 2 +-
kexec/arch/ppc/kexec-ppc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kexec/arch/ppc/fixup_dtb.c b/kexec/arch/ppc/fixup_dtb.c
index 26c23a3..09f9ac1 100644
--- a/kexec/arch/ppc/fixup_dtb.c
+++ b/kexec/arch/ppc/fixup_dtb.c
@@ -311,7 +311,7 @@ static void fixup_initrd(char *blob_buf)
return;
}
- tmp = ramdisk_base + ramdisk_size + 1;
+ tmp = ramdisk_base + ramdisk_size;
err = fdt_setprop(blob_buf, nodeoffset,
"linux,initrd-end", &tmp, sizeof(tmp));
if (err < 0) {
diff --git a/kexec/arch/ppc/kexec-ppc.c b/kexec/arch/ppc/kexec-ppc.c
index c36c7b3..ab76d6f 100644
--- a/kexec/arch/ppc/kexec-ppc.c
+++ b/kexec/arch/ppc/kexec-ppc.c
@@ -481,7 +481,7 @@ static int get_devtree_details(unsigned long kexec_flags)
if ((initrd_end - initrd_start) != 0 ) {
initrd_base = initrd_start;
- initrd_size = initrd_end - initrd_start + 1;
+ initrd_size = initrd_end - initrd_start;
}
if (reuse_initrd) {
--
1.6.0.6
^ permalink raw reply related
* Re: [PATCH 0/4] Some 47x patches for the powerpc-4xx tree
From: Benjamin Herrenschmidt @ 2010-08-19 3:19 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev list
In-Reply-To: <1282149960.3679.0.camel@shaggy-w500.austin.ibm.com>
On Wed, 2010-08-18 at 11:45 -0500, Dave Kleikamp wrote:
> Sorry! Forgot to change the subject.
>
> Shaggy
>
> On Wed, 2010-08-18 at 11:44 -0500, Dave Kleikamp wrote:
> > Josh,
> > Here are some bug fixes for the powerpc-4xx tree. It'd be nice if they
> > could make it into 2.6.46.
Yeah and I'm sure they can make it into 2.6.46... if you want to wait
that long ! In the meantime, 2.6.36 will do :-)
Cheers,
Ben.
> > Thanks,
> > Shaggy
> >
> > Dave Kleikamp (4):
> > powerpc/47x: Make sure mcsr is cleared before enabling machine check
> > interrupts
> > powerpc/47x: Remove redundant line from cputable.c
> > powerpc/4xx: Index interrupt stacks by physical cpu
> > powerpc/47x: Add an isync before the tlbivax instruction
> >
> > arch/powerpc/kernel/cputable.c | 1 -
> > arch/powerpc/kernel/head_44x.S | 4 ++++
> > arch/powerpc/kernel/irq.c | 15 ++++++++-------
> > arch/powerpc/kernel/setup_32.c | 9 +++++----
> > arch/powerpc/mm/tlb_nohash_low.S | 1 +
> > 5 files changed, 18 insertions(+), 12 deletions(-)
> >
>
^ permalink raw reply
* Re:
From: Stephen Rothwell @ 2010-08-19 1:04 UTC (permalink / raw)
To: Wolfgang Denk
Cc: linuxppc-dev, Rupjyoti Sarmah, Prodyut Hazarika, Mark Miesfeld
In-Reply-To: <20100818205646.57783157D71@gemini.denx.de>
[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]
Hi Wolfgang,
On Wed, 18 Aug 2010 22:56:46 +0200 Wolfgang Denk <wd@denx.de> wrote:
>
> drivers/ata/sata_dwc_460ex.c: At top level:
> drivers/ata/sata_dwc_460ex.c:1592: warning: 'struct of_device' declared inside parameter list
> drivers/ata/sata_dwc_460ex.c:1592: warning: its scope is only this definition or declaration, which is probably not what you want
> drivers/ata/sata_dwc_460ex.c: In function 'sata_dwc_probe':
> drivers/ata/sata_dwc_460ex.c:1607: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1614: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1616: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1622: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1628: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1630: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1646: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1650: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1652: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1658: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1660: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1667: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1676: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1678: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1691: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1693: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c: At top level:
> drivers/ata/sata_dwc_460ex.c:1705: warning: 'struct of_device' declared inside parameter list
> drivers/ata/sata_dwc_460ex.c: In function 'sata_dwc_remove':
> drivers/ata/sata_dwc_460ex.c:1707: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c:1720: error: dereferencing pointer to incomplete type
> drivers/ata/sata_dwc_460ex.c: At top level:
> drivers/ata/sata_dwc_460ex.c:1736: warning: initialization from incompatible pointer type
> drivers/ata/sata_dwc_460ex.c:1737: warning: initialization from incompatible pointer type
I think most of these (if not all) are fixed in Linus' tree today.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-19 0:12 UTC (permalink / raw)
To: Richard Cochran
Cc: Rodolfo Giometti, Arnd Bergmann, netdev, devicetree-discuss,
linux-kernel, linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <20100818071942.GA4096@riccoc20.at.omicron.at>
On Wed, 2010-08-18 at 09:19 +0200, Richard Cochran wrote:
> On Tue, Aug 17, 2010 at 05:22:43PM -0700, john stultz wrote:
>>
> > So while to me, it think it would be more ideal (or maybe just less
> > different) to have a read-only interface (like the RTC), leaving PTPd to
> > manage offset calculations and use that to steer the system time. I can
> > acknowledge the need to have some way to correct the freq so the packet
> > timestamps are corrected.
>
> The PTPd need not change the system time at all for PTP clock to be
> useful. (see below)
Right, obviously an ok-solution is often more useful then no-solution.
But that doesn't mean we shouldn't shoot for a good or even
great-solution. :)
> > I still feel a little concerned over the timer/alarm related interfaces.
> > Could you explain why the alarm interface is necessary?
>
> The timer/alarm stuff is "ancillary" and is not at all necessary. It
> is just a "nice to have." I will happily remove it, if it is too
> troubling for people.
If there's a compelling argument for it, I'm interested to hear. But
again, it seems like just
yet-another-way-to-get-alarm/timer-functionality, so before we add an
extra API (or widen an existing API) I'd like to understand the need.
But maybe it might simplify the discussion to pull it for now, but
keeping it in mind to possibly include later as an extension?
> > So really I think my initial negative gut reaction to this was mostly
> > out of the fact that you introduced a char dev that provides almost 100%
> > coverage of the posix-time interface. That is duplication we definitely
> > don't want.
>
> The reason why I modelled the char device on the posix interface was
> to make the API more familiar to application programmers. After the
> recent discussion (and having reviewed the posix clock implementation
> in Linux), I now think it would be even better to simply offer a new
> posic clock ID for PTP.
>
> I was emulating the posix interface. Instead I should use it directly.
I'm definitely interested to see what you come up with here. I'm still
hesitant with adding a PTP clock_id, but extending the posix-clocks
interface in this way isn't unprecedented (see: CLOCK_SGI_CYCLE) I just
would like to make sure we don't end up with a clock_id namespace
littered with oddball clocks that were not well abstracted (see:
CLOCK_SGI_CYCLE :).
For instance: imagine if instead of keeping the clocksource abstraction
internal to the timekeeping core, we exposed each clocksource to
userland via a clock_id. Every arch would have different ids, and each
arch might have multiple ids. Programming against that would be a huge
pain.
So in thinking about this, try to focus on what the new clock_id
provides that the other existing clockids do not? Are they at comparable
levels of abstraction? 15 years from now, are folks likely to still be
using it? Will it be maintainable? etc...
> > Also I think the documentation I've read about PTP (likely just due to
> > the engineering focus) has an odd inverted sense of priority, focusing
> > on keeping obscure hardware clocks on NIC cards in sync, rather then the
> > the more tangible feature of keeping the system time in sync.
> >
> > This could be comically interpreted as trying to create a shadow-time on
> > the system that is the "real time" and "yea, maybe we'll let the system
> > know what time it is, but user-apps who want to know the score can send
> > a magic ioctl to /dev/something and get the real deal". ;) I'm sure
> > that's not the case, but I'd like to keep any confusion in userland
> > about which time is the best time to a minimum (ie: use the system
> > time).
>
> You are right. As John Eidson's excellent book points out, modern
> computers and operating systems provide surprisingly little support
> for programming based on absolute time. It is not PTP's fault. PTP is
> actually a step in the right direction, but it doesn't yet really fit
> in to the present computing world.
You'll have to forgive me, as I haven't had the time to check out that
book. What exactly do you mean by operating systems provide little
support for programming based on absolute time?
> Okay, here is the Big Picture.
>
> 1. Use Case: SW timestamping
>
> PTP with software timestamping (ie without special hardware) can
> acheive synchronization within a few dozen microseconds, after
> about twenty minutes. This is sufficient for very many people. The
> new API (whether char device or syscall) fully and simply supports
> this use case. When the PTPd adjusts the PTP clock, it is actually
> adjusting the system time, just like NTPd.
Again this illustrates the inversion of focus: system time is merely one
of many possible PTP clocks in the larger PTP framework.
The way I tend to see it: PTP is just one of the many ways to sync
system time.
> 2. Use Case: HW timestamping for industrial control
>
> PTP with hardware timestamping can acheive synchronization within
> 100 nanoseconds after one minute. If you want to do something with
> your wonderfully synchronization PTP clock, it must have some kind
> of special hardware, like timestamping external signals or
> generating one-shot or periodic outputs. The new API (whether char
> device or syscall) supports this use case via the ancillary
> commands.
>
> In this case, the end user has an application that interfaces with
> the outside world via the PTP clock API. Such a specialized
> application (for example, motor control) uses only the PTP API,
> since it knows that the standard posix API cannot help. It is
> irrelevant that the system time is not synchronized, in this case.
>
> The PTP clock hardware may or may not provide a hardware interface
> (interrupt) to the main CPU. In this case, it does not matter. The
> PTP clock is useful all by itself.
These specialized applications are part of what concerns me the most.
For example, I can see some parallels between things like audio
processing, where you have a buffer consumed by the card at a certain
rate. Now, the card has its own crystal it uses to time its consumption,
so it has its own time domain, and could drift from system time. Thus
you want to trigger buffer-refill interrupts off of the audio card's
clock, not the system time which might run the risk of being late.
But again, we don't expose the audio hardware clock to userland in the
same way we expose system time.
So, instead of a chardev or a posix clock id, would it make more sense
for the PTP adjustment interface to be more closely to the
nic/phy/whatever interface? Much like how the audio apis do it?
Again, my knowledge in the networking stack is pretty limited. But it
would seem that having an interface that does something to the effect of
"adjust the timestamp clock on the hardware that generated it from this
packet by Xppb" would feel like the right level of abstraction. Its
closely related to SO_TIMESTAMP, option right? Would something like
using the setsockopt/getsockopt interface with
SO_TIMESTAMP_ADJUST/OFFSET/SET/etc be reasonable?
> 3. Use Case: HW timestamping with PPS to host
>
> This case is the same as case 2, with the exception that the PTP
> clock can interrupt the main CPU. The PTP clock driver advertises
> the "PPS" capability. When enabled, the PTP layer delivers events
> via the existing Linux PPS subsystem. Programs like NTPd can use
> these events to regulate the system time.
>
> This means that the system clock and the PTP clock will be at least
> as well synchronized as when using a traditionial radio clock, GPS,
> or IRIG-B method. In my opinion, this will be good enough for any
> practical purpose. For example, let's say you want to run a
> periodic task synchronized to the absolute wall clock time. Your
> scheduling latency will be a dozen microseconds or so. Your PPS
> synchronized system clock should be close enough to the PTP clock
> to support this.
And yes, this seems perfectly reasonable feature to add. Its not
controversial to me, because its likely to work within the existing
interfaces and won't expose anything new to userland.
Again, sorry to be such a pain about all of this. I know its frustrating
to bring your hard work to lkml and then get a lot of non-specific
push-back to "do it differently". You're earlier comments about being
wary of adding syscalls because it requires lots of review and debate
were spot on, but *any* user-visible API really requires the same level
of care (which, of course, they don't always get). So please don't take
my comments personally. Keep pushing and I will either come around or
maybe we can find a better middle-ground.
thanks
-john
^ permalink raw reply
* (no subject)
From: Wolfgang Denk @ 2010-08-18 20:56 UTC (permalink / raw)
To: Rupjyoti Sarmah; +Cc: linuxppc-dev, Prodyut Hazarika, Mark Miesfeld
Dear Rupjyoti,
drivers/ata/sata_dwc_460ex.c fails to build in current mainline:
...
CC drivers/ata/sata_dwc_460ex.o
drivers/ata/sata_dwc_460ex.c:43:1: warning: "DRV_NAME" redefined
In file included from drivers/ata/sata_dwc_460ex.c:38:
drivers/ata/libata.h:31:1: warning: this is the location of the previous definition
drivers/ata/sata_dwc_460ex.c:44:1: warning: "DRV_VERSION" redefined
drivers/ata/libata.h:32:1: warning: this is the location of the previous definition
drivers/ata/sata_dwc_460ex.c: In function 'sata_dwc_exec_command_by_tag':
drivers/ata/sata_dwc_460ex.c:1356: warning: passing argument 1 of 'ata_get_cmd_descript' makes integer from pointer without a cast
drivers/ata/sata_dwc_460ex.c: At top level:
drivers/ata/sata_dwc_460ex.c:1592: warning: 'struct of_device' declared inside parameter list
drivers/ata/sata_dwc_460ex.c:1592: warning: its scope is only this definition or declaration, which is probably not what you want
drivers/ata/sata_dwc_460ex.c: In function 'sata_dwc_probe':
drivers/ata/sata_dwc_460ex.c:1607: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1614: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1616: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1622: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1628: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1630: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1646: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1650: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1652: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1658: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1660: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1667: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1676: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1678: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1691: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1693: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c: At top level:
drivers/ata/sata_dwc_460ex.c:1705: warning: 'struct of_device' declared inside parameter list
drivers/ata/sata_dwc_460ex.c: In function 'sata_dwc_remove':
drivers/ata/sata_dwc_460ex.c:1707: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c:1720: error: dereferencing pointer to incomplete type
drivers/ata/sata_dwc_460ex.c: At top level:
drivers/ata/sata_dwc_460ex.c:1736: warning: initialization from incompatible pointer type
drivers/ata/sata_dwc_460ex.c:1737: warning: initialization from incompatible pointer type
make[2]: *** [drivers/ata/sata_dwc_460ex.o] Error 1
Do you have any hints how to fix that?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Few people do business well who do nothing else.
-- Philip Earl of Chesterfield
^ permalink raw reply
* [PATCH] Correct rtas_data_buf locking in dlpar code
From: Nathan Fontenot @ 2010-08-18 19:58 UTC (permalink / raw)
To: linuxppc-dev
The dlpar code can cause a deadlock to occur when making the RTAS
configure-connector call. This occurs because we make kmalloc calls,
which can block, while parsing the rtas_data_buf and holding the
rtas_data_buf_lock. This an cause issues if someone else attempts
to grab the rtas_data_bug_lock.
This patch alleviates this issue by copying the contents of the rtas_data_buf
to a local buffer before parsing. This allows us to only hold the
rtas_data_buf_lock around the RTAS configure-connector calls.
Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 42 ++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 13 deletions(-)
Index: powerpc/arch/powerpc/platforms/pseries/dlpar.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/dlpar.c 2010-08-05 10:55:46.000000000 -0500
+++ powerpc/arch/powerpc/platforms/pseries/dlpar.c 2010-08-18 11:42:10.000000000 -0500
@@ -129,20 +129,35 @@ struct device_node *dlpar_configure_conn
struct property *property;
struct property *last_property = NULL;
struct cc_workarea *ccwa;
+ char *data_buf;
int cc_token;
- int rc;
+ int rc = -1;
cc_token = rtas_token("ibm,configure-connector");
if (cc_token == RTAS_UNKNOWN_SERVICE)
return NULL;
- spin_lock(&rtas_data_buf_lock);
- ccwa = (struct cc_workarea *)&rtas_data_buf[0];
+ data_buf = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
+ if (!data_buf)
+ return NULL;
+
+ ccwa = (struct cc_workarea *)&data_buf[0];
ccwa->drc_index = drc_index;
ccwa->zero = 0;
- rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
- while (rc) {
+ do {
+ /* Since we release the rtas_data_buf lock between configure
+ * connector calls we want to re-populate the rtas_data_buffer
+ * with the contents of the previous call.
+ */
+ spin_lock(&rtas_data_buf_lock);
+
+ memcpy(rtas_data_buf, data_buf, RTAS_DATA_BUF_SIZE);
+ rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
+ memcpy(data_buf, rtas_data_buf, RTAS_DATA_BUF_SIZE);
+
+ spin_unlock(&rtas_data_buf_lock);
+
switch (rc) {
case NEXT_SIBLING:
dn = dlpar_parse_cc_node(ccwa);
@@ -197,18 +212,19 @@ struct device_node *dlpar_configure_conn
"returned from configure-connector\n", rc);
goto cc_error;
}
+ } while (rc);
- rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
+cc_error:
+ kfree(data_buf);
+
+ if (rc) {
+ if (first_dn)
+ dlpar_free_cc_nodes(first_dn);
+
+ return NULL;
}
- spin_unlock(&rtas_data_buf_lock);
return first_dn;
-
-cc_error:
- if (first_dn)
- dlpar_free_cc_nodes(first_dn);
- spin_unlock(&rtas_data_buf_lock);
- return NULL;
}
static struct device_node *derive_parent(const char *path)
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/85xx: Cleanup QE initialization for MPC85xxMDS boards
From: Timur Tabi @ 2010-08-18 19:31 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20100608195557.GC11446@oksana.dev.rtsoft.ru>
On Tue, Jun 8, 2010 at 2:55 PM, Anton Vorontsov <avorontsov@mvista.com> wrote:
> The mpc85xx_mds_setup_arch() function is incomprehensible
> and unmaintainable. Factor out all QE specific stuff into
> mpc85xx_mds_qe_init() and mpc85xx_mds_reset_ucc_phys().
>
> Also move QE stuff out of mpc85xx_mds_pic_init().
>
> The diff is unreadable, but only because the code was so. ;-)
> It should be better now, and less indented.
>
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
> ---
This patch introduces breaks mpc85xx_smp_defconfig:
CC arch/powerpc/platforms/85xx/mpc85xx_mds.o
arch/powerpc/platforms/85xx/mpc85xx_mds.c: In function 'mpc85xx_mds_setup_arch':
arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: 'np' undeclared
(first use in this function)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: (Each undeclared
identifier is reported only once
arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: for each
function it appears in.)
Frankly, I've been seeing a lot of build breaks with
mpc85xx_[smp_]defconfig. I had a patch that added a
p1022_ds_defconfig, but was told instead to update
mpc85xx_[smp_]defconfig. However, it appears that no one is really
testing mpc85xx_[smp_]defconfig, since there are now *two* failures
(this one, and CONFIG_RAPIDIO) with these defconfig files.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* [PATCH] powerpc/pci: fix checking for child bridges in PCI code.
From: Grant Likely @ 2010-08-18 18:27 UTC (permalink / raw)
To: benh, linuxppc-dev, linux-kernel; +Cc: Julia Lawall
pci_device_to_OF_node() can return null, and list_for_each_entry will
never enter the loop when dev is NULL, so it looks like this test is
a typo.
Reported-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
arch/powerpc/kernel/pci_of_scan.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 6ddb795..e751506 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -336,7 +336,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
struct device_node *child = pci_device_to_OF_node(dev);
- if (dev)
+ if (child)
of_scan_pci_bridge(child, dev);
}
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox