LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 4/9] RapidIO: Add relation links between RIO device structures
From: Micha Nelissen @ 2010-08-17  7:08 UTC (permalink / raw)
  To: Bounine, Alexandre; +Cc: linuxppc-dev, akpm, linux-kernel
In-Reply-To: <0CE8B6BE3C4AD74AB97D9D29BD24E552011D5F1F@CORPEXCH1.na.ads.idt.com>

Bounine, Alexandre wrote:
>> As RapidIO is a switched network, the concept of 'previous' and 'next'
>> devices is invalid. Perhaps it's just the way they were
>> discovered/enumerated, but that does not matter any more at runtime.
>> Or at least, should not matter.
>>
> 
> Yes, the "previous" and "next" have to be considered in context of
> enumeration/discovery.
> At runtime, it does not matter for data traffic, but is valuable
> information for error recovery

I agree it's desirable to have this information. Notes:
1) is rio_dev->prev used anywhere? (maybe I missed it)
2) is the nextdev[port] list complete? I mean are all connected switches 
in the list? My guess is that multiply connected switches are enumerated 
only once therefore only appear in the nextdev if only one switch, 
instead of all
3) it would be nice to have all switch connections information.

In case ever the network is rerouted, this information will become 
useful; instead of having a tree representation of the network only.

Micha

^ permalink raw reply

* RE: [PATCH 1/2] mmc: change ACMD12 to AUTO_CMD12 for more clear
From: Zang Roy-R61911 @ 2010-08-17  6:48 UTC (permalink / raw)
  To: Zang Roy-R61911, akpm, linux-mmc; +Cc: linuxppc-dev, mirqus
In-Reply-To: <3850A844E6A3854C827AC5C0BEC7B60A056400@zch01exm23.fsl.freescale.net>

=20

> -----Original Message-----
> From: Zang Roy-R61911=20
> Sent: Wednesday, August 11, 2010 12:47 PM
> To: Zang Roy-R61911; akpm@linux-foundation.org;=20
> linux-mmc@vger.kernel.org
> Cc: linuxppc-dev@ozlabs.org; mirqus@gmail.com;=20
> cbouatmailru@gmail.com; grant.likely@secretlab.ca
> Subject: RE: [PATCH 1/2] mmc: change ACMD12 to AUTO_CMD12 for=20
> more clear
>=20
> =20
>=20
> > -----Original Message-----
> > From: Zang Roy-R61911=20
> > Sent: Tuesday, August 10, 2010 17:47 PM
> > To: akpm@linux-foundation.org; linux-mmc@vger.kernel.org
> > Cc: linuxppc-dev@ozlabs.org; mirqus@gmail.com;=20
> > cbouatmailru@gmail.com; grant.likely@secretlab.ca
> > Subject: [PATCH 1/2] mmc: change ACMD12 to AUTO_CMD12 for more clear
> >=20
> > Change ACMD12 to AUTO_CMD12 to reduce the confusion.
> > ACMD12 might be confused with MMC/SD App CMD 12 (CMD55+CMD12 combo).
> >=20
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-of-core.c |    2 +-
> >  drivers/mmc/host/sdhci.c         |    8 ++++----
> >  drivers/mmc/host/sdhci.h         |   10 +++++-----
> >  3 files changed, 10 insertions(+), 10 deletions(-)
> Andrew,=20
> Could you help to pick up this minor fix?
> Thanks.
> Roy
Any update on these two patches?
Thanks.
Roy

^ permalink raw reply

* Re: Query regarding 2.6.335 RT[Ingo's] and Non-RT performance
From: Manikandan Ramachandran @ 2010-08-17  5:26 UTC (permalink / raw)
  To: linuxppc-dev

> ------------------------------------------------------
> > Date: Thu, 12 Aug 2010 13:53:51 -0400
> > From: Jeff Angielski <jeff@theptrgroup.com>
> > To: linuxppc-dev@lists.ozlabs.org
> > Subject: Re: Query regarding 2.6.335 RT[Ingo's] and Non-RT performance
> > Message-ID: <4C64352F.4090005@theptrgroup.com>
> > Content-Type: text/plain; charset=3DISO-8859-1; format=3Dflowed
> >
> > On 08/11/2010 06:18 PM, Manikandan Ramachandran wrote:
> > > Hello All,
> > > =A0 =A0 =A0I created a very simple program which has higher priority =
than
> > > normal tasks and runs a tight loop. Under same test environment I ran
> > > this program on both non-rt and rt 2.6.33.5 kernel. =A0To my suprise =
I see
> > > that performance of non-RT kernel is better than RT. non-RT kernel to=
ok
> > > 3 sec and 366156 usec while RT kernel took about 3 sec and 418011
> > > usec.Can someone please explain why the performance of non-rt kernel =
is
> > > better than rt kernel? From the face of the test result, I feel RT ha=
s
> > > more overhead,Is there any configuration that I could do to bring dow=
n
> > > the overhead?
> >
> > Your "surprise" is due to your definition of "performance".
> >
> > The purpose of the -rt kernels is to reduce the kernel latency. =A0This=
 is
> > important for servicing hardware. =A0Normal users find the -rt useful f=
or
> > audio/video applications. =A0Engineering and scientific users find the =
-rt
> > beneficially for servicing hardware like sensors or control systems.
> >
> > If you are just trying to run calculations as fast as you can in user
> > space, you'd be better off using the non-rt variants.
> >
> >
> > --
> > Jeff Angielski
> > The PTR Group
> > www.theptrgroup.com



 Thanks for your response.

On one hand I hear that RT-kernel is meant for reducing kernel latency on
other hand I see that there is RT-kernel overhead. So what really RT-kernel
brings to system performance?

Actually I see that latency for higher priority is more or less same for
non-rt system.

One more thing, since irqs being threaded in RT, and with CFS scheduler in
2.6.33, wouldn't we bring down system performance as CFS is O(log(n)) =A0in
nature?
 --
 Thanks,
 Manik

^ permalink raw reply

* Re: [Resend][PATCHv3] Xilinx Virtex 4 FX Soft FPU support
From: Grant Likely @ 2010-08-16 21:56 UTC (permalink / raw)
  To: Sergey Temerkhanov; +Cc: linuxppc-dev, John Linn
In-Reply-To: <201008140809.54230.temerkhanov@cifronik.ru>

Josh,

This one looks okay to me, but I've left it for you to comment on.  I
haven't seen any comments, so should I go ahead and pick it up for my
2.6.37 -next branch?

Cheers,
g.

On Fri, Aug 13, 2010 at 10:09 PM, Sergey Temerkhanov
<temerkhanov@cifronik.ru> wrote:
> This patch enables support for Xilinx Virtex 4 FX singe-float FPU.
>
> Changelog v2-v3:
> =9A =9A =9A =9A-Fixed whitespaces for SAVE_FPR/REST_FPR.
> =9A =9A =9A =9A-Changed description of MSR_AP bit.
> =9A =9A =9A =9A-Removed the stub for APU unavailable exception.
>
> Changelog v1->v2:
> =9A =9A =9A =9A-Added MSR_AP bit definition
> =9A =9A =9A =9A-Renamed CONFIG_XILINX_FPU to CONFIG_XILINX_SOFTFPU, moved=
 it to
> =9A =9A =9A =9A 'Platform support' and made it Virtex4-FX-only.
> =9A =9A =9A =9A-Changed SAVE_FPR/REST_FPR definition style.
>
> Caveats:
> =9A =9A =9A =9A- Hard-float binaries which rely on in-kernel math emulati=
on will
> =9A =9A =9A =9Agive wrong results since they expect 64-bit double-precisi=
on instead of
> =9A =9A =9A =9A32-bit single-precision numbers which Xilinx V4-FX Soft FP=
U produces.
>
>
> Signed-off-by: Sergey Temerkhanov<temerkhanov@cifronik.ru>
>
> diff -r 626de0d94469 arch/powerpc/include/asm/ppc_asm.h
> --- a/arch/powerpc/include/asm/ppc_asm.h =9A =9A =9A =9AWed May 26 15:33:=
32 2010 +0400
> +++ b/arch/powerpc/include/asm/ppc_asm.h =9A =9A =9A =9AWed May 26 20:30:=
43 2010 +0400
> @@ -85,13 +85,21 @@
> =9A#define REST_8GPRS(n, base) =9A =9AREST_4GPRS(n, base); REST_4GPRS(n+4=
, base)
> =9A#define REST_10GPRS(n, base) =9A REST_8GPRS(n, base); REST_2GPRS(n+8, =
base)
>
> +
> +#ifdef CONFIG_XILINX_SOFTFPU
> +#define SAVE_FPR(n, base) =9A =9A =9Astfs =9A =9An,THREAD_FPR0+8*TS_FPRW=
IDTH*(n)(base)
> +#define REST_FPR(n, base) =9A =9A =9Alfs =9A =9A n,THREAD_FPR0+8*TS_FPRW=
IDTH*(n)(base)
> +#else
> =9A#define SAVE_FPR(n, base) =9A =9A =9Astfd =9A =9An,THREAD_FPR0+8*TS_FP=
RWIDTH*(n)(base)
> +#define REST_FPR(n, base) =9A =9A =9Alfd =9A =9A n,THREAD_FPR0+8*TS_FPRW=
IDTH*(n)(base)
> +#endif
> +
> =9A#define SAVE_2FPRS(n, base) =9A =9ASAVE_FPR(n, base); SAVE_FPR(n+1, ba=
se)
> =9A#define SAVE_4FPRS(n, base) =9A =9ASAVE_2FPRS(n, base); SAVE_2FPRS(n+2=
, base)
> =9A#define SAVE_8FPRS(n, base) =9A =9ASAVE_4FPRS(n, base); SAVE_4FPRS(n+4=
, base)
> =9A#define SAVE_16FPRS(n, base) =9A SAVE_8FPRS(n, base); SAVE_8FPRS(n+8, =
base)
> =9A#define SAVE_32FPRS(n, base) =9A SAVE_16FPRS(n, base); SAVE_16FPRS(n+1=
6, base)
> -#define REST_FPR(n, base) =9A =9A =9Alfd =9A =9A n,THREAD_FPR0+8*TS_FPRW=
IDTH*(n)(base)
> +
> =9A#define REST_2FPRS(n, base) =9A =9AREST_FPR(n, base); REST_FPR(n+1, ba=
se)
> =9A#define REST_4FPRS(n, base) =9A =9AREST_2FPRS(n, base); REST_2FPRS(n+2=
, base)
> =9A#define REST_8FPRS(n, base) =9A =9AREST_4FPRS(n, base); REST_4FPRS(n+4=
, base)
> diff -r 626de0d94469 arch/powerpc/include/asm/reg.h
> --- a/arch/powerpc/include/asm/reg.h =9A =9AWed May 26 15:33:32 2010 +040=
0
> +++ b/arch/powerpc/include/asm/reg.h =9A =9AWed May 26 20:30:43 2010 +040=
0
> @@ -30,6 +30,7 @@
> =9A#define MSR_ISF_LG =9A =9A 61 =9A =9A =9A =9A =9A =9A =9A/* Interrupt =
64b mode valid on 630 */
> =9A#define MSR_HV_LG =9A =9A =9A60 =9A =9A =9A =9A =9A =9A =9A/* Hypervis=
or state */
> =9A#define MSR_VEC_LG =9A =9A 25 =9A =9A =9A =9A =9A =9A =9A/* Enable Alt=
iVec */
> +#define MSR_AP_LG =9A =9A =9A25 =9A =9A =9A =9A =9A =9A =9A/* Enable PPC=
405 APU */
> =9A#define MSR_VSX_LG =9A =9A 23 =9A =9A =9A =9A =9A =9A =9A/* Enable VSX=
 */
> =9A#define MSR_POW_LG =9A =9A 18 =9A =9A =9A =9A =9A =9A =9A/* Enable Pow=
er Management */
> =9A#define MSR_WE_LG =9A =9A =9A18 =9A =9A =9A =9A =9A =9A =9A/* Wait Sta=
te Enable */
> @@ -71,6 +72,7 @@
> =9A#define MSR_HV =9A =9A =9A =9A 0
> =9A#endif
>
> +#define MSR_AP =9A =9A =9A =9A __MASK(MSR_AP_LG) =9A =9A =9A /* Enable P=
PC405 APU */
> =9A#define MSR_VEC =9A =9A =9A =9A =9A =9A =9A =9A__MASK(MSR_VEC_LG) =9A =
=9A =9A/* Enable AltiVec */
> =9A#define MSR_VSX =9A =9A =9A =9A =9A =9A =9A =9A__MASK(MSR_VSX_LG) =9A =
=9A =9A/* Enable VSX */
> =9A#define MSR_POW =9A =9A =9A =9A =9A =9A =9A =9A__MASK(MSR_POW_LG) =9A =
=9A =9A/* Enable Power Management */
> diff -r 626de0d94469 arch/powerpc/kernel/fpu.S
> --- a/arch/powerpc/kernel/fpu.S Wed May 26 15:33:32 2010 +0400
> +++ b/arch/powerpc/kernel/fpu.S Wed May 26 20:30:43 2010 +0400
> @@ -57,6 +57,9 @@
> =9A_GLOBAL(load_up_fpu)
> =9A =9A =9A =9Amfmsr =9A r5
> =9A =9A =9A =9Aori =9A =9A r5,r5,MSR_FP
> +#ifdef CONFIG_XILINX_SOFTFPU
> + =9A =9A =9A oris =9A =9Ar5,r5,MSR_AP@h
> +#endif
> =9A#ifdef CONFIG_VSX
> =9ABEGIN_FTR_SECTION
> =9A =9A =9A =9Aoris =9A =9Ar5,r5,MSR_VSX@h
> @@ -85,6 +88,9 @@
> =9A =9A =9A =9Atoreal(r5)
> =9A =9A =9A =9APPC_LL =9Ar4,_MSR-STACK_FRAME_OVERHEAD(r5)
> =9A =9A =9A =9Ali =9A =9A =9Ar10,MSR_FP|MSR_FE0|MSR_FE1
> +#ifdef CONFIG_XILINX_SOFTFPU
> + =9A =9A =9A oris =9A =9Ar10,r10,MSR_AP@h
> +#endif
> =9A =9A =9A =9Aandc =9A =9Ar4,r4,r10 =9A =9A =9A =9A =9A =9A =9A /* disab=
le FP for previous task */
> =9A =9A =9A =9APPC_STL r4,_MSR-STACK_FRAME_OVERHEAD(r5)
> =9A1:
> @@ -94,6 +100,9 @@
> =9A =9A =9A =9Amfspr =9A r5,SPRN_SPRG3 =9A =9A =9A =9A =9A /* current tas=
k's THREAD (phys) */
> =9A =9A =9A =9Alwz =9A =9A r4,THREAD_FPEXC_MODE(r5)
> =9A =9A =9A =9Aori =9A =9A r9,r9,MSR_FP =9A =9A =9A =9A =9A =9A/* enable =
FP for current */
> +#ifdef CONFIG_XILINX_SOFTFPU
> + =9A =9A =9A oris =9A =9Ar9,r9,MSR_AP@h
> +#endif
> =9A =9A =9A =9Aor =9A =9A =9Ar9,r9,r4
> =9A#else
> =9A =9A =9A =9Ald =9A =9A =9Ar4,PACACURRENT(r13)
> @@ -124,6 +133,9 @@
> =9A_GLOBAL(giveup_fpu)
> =9A =9A =9A =9Amfmsr =9A r5
> =9A =9A =9A =9Aori =9A =9A r5,r5,MSR_FP
> +#ifdef CONFIG_XILINX_SOFTFPU
> + =9A =9A =9A oris =9A =9Ar5,r5,MSR_AP@h
> +#endif
> =9A#ifdef CONFIG_VSX
> =9ABEGIN_FTR_SECTION
> =9A =9A =9A =9Aoris =9A =9Ar5,r5,MSR_VSX@h
> @@ -145,6 +157,9 @@
> =9A =9A =9A =9Abeq =9A =9A 1f
> =9A =9A =9A =9APPC_LL =9Ar4,_MSR-STACK_FRAME_OVERHEAD(r5)
> =9A =9A =9A =9Ali =9A =9A =9Ar3,MSR_FP|MSR_FE0|MSR_FE1
> +#ifdef CONFIG_XILINX_SOFTFPU
> + =9A =9A =9A oris =9A =9Ar3,r3,MSR_AP@h
> +#endif
> =9A#ifdef CONFIG_VSX
> =9ABEGIN_FTR_SECTION
> =9A =9A =9A =9Aoris =9A =9Ar3,r3,MSR_VSX@h
> diff -r 626de0d94469 arch/powerpc/kernel/head_40x.S
> --- a/arch/powerpc/kernel/head_40x.S =9A =9AWed May 26 15:33:32 2010 +040=
0
> +++ b/arch/powerpc/kernel/head_40x.S =9A =9AWed May 26 20:30:43 2010 +040=
0
> @@ -420,7 +420,19 @@
> =9A =9A =9A =9Aaddi =9A =9Ar3,r1,STACK_FRAME_OVERHEAD
> =9A =9A =9A =9AEXC_XFER_STD(0x700, program_check_exception)
>
> +/* 0x0800 - FPU unavailable Exception */
> +#ifdef CONFIG_PPC_FPU
> + =9A =9A =9A START_EXCEPTION(0x0800, FloatingPointUnavailable)
> + =9A =9A =9A NORMAL_EXCEPTION_PROLOG
> + =9A =9A =9A beq =9A =9A 1f; =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A=
 =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A \
> + =9A =9A =9A bl =9A =9A =9Aload_up_fpu; =9A =9A =9A =9A =9A =9A/* if fro=
m user, just load it up */ =9A \
> + =9A =9A =9A b =9A =9A =9A fast_exception_return; =9A =9A =9A =9A =9A =
=9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A\
> +1: =9A =9A addi =9A =9Ar3,r1,STACK_FRAME_OVERHEAD; =9A =9A =9A =9A =9A =
=9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A \
> + =9A =9A =9A EXC_XFER_EE_LITE(0x800, kernel_fp_unavailable_exception)
> +#else
> =9A =9A =9A =9AEXCEPTION(0x0800, Trap_08, unknown_exception, EXC_XFER_EE)
> +#endif
> +
> =9A =9A =9A =9AEXCEPTION(0x0900, Trap_09, unknown_exception, EXC_XFER_EE)
> =9A =9A =9A =9AEXCEPTION(0x0A00, Trap_0A, unknown_exception, EXC_XFER_EE)
> =9A =9A =9A =9AEXCEPTION(0x0B00, Trap_0B, unknown_exception, EXC_XFER_EE)
> @@ -821,8 +833,10 @@
> =9A* The PowerPC 4xx family of processors do not have an FPU, so this jus=
t
> =9A* returns.
> =9A*/
> +#ifndef CONFIG_PPC_FPU
> =9A_ENTRY(giveup_fpu)
> =9A =9A =9A =9Ablr
> +#endif
>
> =9A/* This is where the main kernel code starts.
> =9A*/
> diff -r 626de0d94469 arch/powerpc/platforms/Kconfig
> --- a/arch/powerpc/platforms/Kconfig =9A =9AWed May 26 15:33:32 2010 +040=
0
> +++ b/arch/powerpc/platforms/Kconfig =9A =9AWed May 26 20:30:43 2010 +040=
0
> @@ -333,4 +333,9 @@
> =9A =9A =9A =9Abool "Xilinx PCI host bridge support"
> =9A =9A =9A =9Adepends on PCI && XILINX_VIRTEX
>
> +config XILINX_SOFTFPU
> + =9A =9A =9A bool "Xilinx Soft FPU"
> + =9A =9A =9A select PPC_FPU
> + =9A =9A =9A depends on XILINX_VIRTEX_4_FX && !PPC40x_SIMPLE && !405GP &=
& !405GPR
> +
> =9Aendmenu
>
> --
> Regards, Sergey Temerkhanov
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] booting-without-of: Remove nonexistent chapters from TOC, fix numbering
From: Grant Likely @ 2010-08-16 21:09 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20100811165603.GA22708@oksana.dev.rtsoft.ru>

On Wed, Aug 11, 2010 at 08:56:03PM +0400, Anton Vorontsov wrote:
> Marvell and GPIO bindings live in their own files, so the TOC should not
> mention them.
> 
> Also fix chapters numbering.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>

Applied, thanks.

g.

> ---
>  Documentation/powerpc/booting-without-of.txt |   31 +------------------------
>  1 files changed, 2 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index 46d2210..3f454b7 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -49,40 +49,13 @@ Table of Contents
>        f) MDIO on GPIOs
>        g) SPI busses
>  
> -  VII - Marvell Discovery mv64[345]6x System Controller chips
> -    1) The /system-controller node
> -    2) Child nodes of /system-controller
> -      a) Marvell Discovery MDIO bus
> -      b) Marvell Discovery ethernet controller
> -      c) Marvell Discovery PHY nodes
> -      d) Marvell Discovery SDMA nodes
> -      e) Marvell Discovery BRG nodes
> -      f) Marvell Discovery CUNIT nodes
> -      g) Marvell Discovery MPSCROUTING nodes
> -      h) Marvell Discovery MPSCINTR nodes
> -      i) Marvell Discovery MPSC nodes
> -      j) Marvell Discovery Watch Dog Timer nodes
> -      k) Marvell Discovery I2C nodes
> -      l) Marvell Discovery PIC (Programmable Interrupt Controller) nodes
> -      m) Marvell Discovery MPP (Multipurpose Pins) multiplexing nodes
> -      n) Marvell Discovery GPP (General Purpose Pins) nodes
> -      o) Marvell Discovery PCI host bridge node
> -      p) Marvell Discovery CPU Error nodes
> -      q) Marvell Discovery SRAM Controller nodes
> -      r) Marvell Discovery PCI Error Handler nodes
> -      s) Marvell Discovery Memory Controller nodes
> -
> -  VIII - Specifying interrupt information for devices
> +  VII - Specifying interrupt information for devices
>      1) interrupts property
>      2) interrupt-parent property
>      3) OpenPIC Interrupt Controllers
>      4) ISA Interrupt Controllers
>  
> -  IX - Specifying GPIO information for devices
> -    1) gpios property
> -    2) gpio-controller nodes
> -
> -  X - Specifying device power management information (sleep property)
> +  VIII - Specifying device power management information (sleep property)
>  
>    Appendix A - Sample SOC node for MPC8540
>  
> -- 
> 1.7.0.5

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Arnd Bergmann @ 2010-08-16 19:59 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: netdev, Richard Cochran, linux-kernel, Rodolfo Giometti,
	devicetree-discuss, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <20100816190003.GB4166@riccoc20.at.omicron.at>

On Monday 16 August 2010 21:00:03 Richard Cochran wrote:
> 
> On Mon, Aug 16, 2010 at 04:26:23PM +0200, Arnd Bergmann wrote:
> > Have you considered integrating the subsystem into the Posix clock/timer
> > framework?
> 
> Yes, but see below.
>  
> > I can't really tell from reading the source if this is possible or
> > not, but my feeling is that if it can be done, that would be a much
> > nicer interface. We already have clock_gettime/clock_settime/
> > timer_settime/... system calls, and while you'd need to add another
> > clockid and some syscalls, my feeling is that it will be more
> > usable in the end.
> 
> You are not the first person to ask about this. See this link for
> longer explanation of why I did not go that way:
> 
>   http://marc.info/?l=linux-netdev&m=127669810232201&w=2
> 
> You could offer the PTP clock as a Linux clock source/event device,
> and I agree that it would be nicer. However, the problem is, what do
> you do with the PHY based clocks?  Just one 16 bit read from a PHY
> clock can take 40 usec, and you need four such read operations just to
> get the current time value.

Why does it matter how long it takes to read the clock? I wasn't thinking
of replacing the system clock with this, just exposing the additional
clock as a new clockid_t value that can be accessed using the existing
syscalls.

> Also, I really did not want to add or change any syscalls. I could not
> see a practical way to extend the existing syscalls to accommodate PTP
> clocks.

Why did you not want to add syscalls? Adding ioctls instead of syscalls
does not make the interface better, just less visible.

Out of the ioctl commands you define, we already seem to have half or more:

PTP_CLOCK_APIVERS -> not needed
PTP_CLOCK_ADJFREQ -> new clock_adjfreq
PTP_CLOCK_ADJTIME -> new clock_adjtime
PTP_CLOCK_GETTIME -> clock_gettime
PTP_CLOCK_SETTIME -> clock_settime
PTP_CLOCK_GETCAPS -> new clock_getcaps
PTP_CLOCK_GETTIMER -> timer_gettime
PTP_CLOCK_SETTIMER -> timer_create/timer_settime
PTP_FEATURE_REQUEST -> possibly clock_feature

	Arnd

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-16 19:38 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Rodolfo Giometti, netdev, devicetree-discuss, linux-kernel,
	linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <AANLkTik_2MKMhOuDGOmu8Kzyq-ipLe+Bxrb3FaD+Tv4U@mail.gmail.com>

On Mon, Aug 16, 2010 at 12:24 PM, john stultz <johnstul@us.ibm.com> wrote:
> On Mon, Aug 16, 2010 at 4:17 AM, Richard Cochran
> A few comments below....
>
>> +** PTP user space API
>> +
>> + =A0 The class driver creates a character device for each registered PT=
P
>> + =A0 clock. User space programs may control the clock using standardize=
d
>> + =A0 ioctls. A program may query, enable, configure, and disable the
>> + =A0 ancillary clock features. User space can receive time stamped
>> + =A0 events via blocking read() and poll(). One shot and periodic
>> + =A0 signals may be configured via an ioctl API with semantics similar
>> + =A0 to the POSIX timer_settime() system call.
>
> As I mentioned earlier, I'm not a huge fan of the char device
> interface for abstracted PTP clocks.
> If it was just the direct hardware access, similar to RTC, which user
> apps then use as a timesource, I'd not have much of a problem. But as
> I mentioned in an earlier private mail, the abstraction level concerns
> me.

[snip]

> 2) As Arnd already mentioned, the chardev interface seems to duplicate
> the clock_gettime/settime() and adjtimex() interfaces.

And maybe just to clarify, as I saw your response to Arnd, I'm not
suggesting using PTP clocks as clocksources for the internal
timekeeping core. Instead I'm trying to understand why PTP clocks need
the equivalent of the existing posix clocks/timer interface. Why would
only having a read-time interface not suffice?

thanks
-john

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-16 19:24 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Rodolfo Giometti, netdev, devicetree-discuss, linux-kernel,
	linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <363bd749a38d0b785d8431e591bf54c38db4c2d7.1281956490.git.richard.cochran@omicron.at>

On Mon, Aug 16, 2010 at 4:17 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> This patch adds an infrastructure for hardware clocks that implement
> IEEE 1588, the Precision Time Protocol (PTP). A class driver offers a
> registration method to particular hardware clock drivers. Each clock is
> exposed to user space as a character device with ioctls that allow tuning
> of the PTP clock.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>

Hey Richard!
   Its very cool to see this work on lkml! I'm excited to see more
work done on ptp.  We had a short private thread discussion earlier (I
got busy and never replied to your last message, my apologies!), but I
wanted to bring up the concerns I have here as well.

A few comments below....

> +** PTP user space API
> +
> + =A0 The class driver creates a character device for each registered PTP
> + =A0 clock. User space programs may control the clock using standardized
> + =A0 ioctls. A program may query, enable, configure, and disable the
> + =A0 ancillary clock features. User space can receive time stamped
> + =A0 events via blocking read() and poll(). One shot and periodic
> + =A0 signals may be configured via an ioctl API with semantics similar
> + =A0 to the POSIX timer_settime() system call.

As I mentioned earlier, I'm not a huge fan of the char device
interface for abstracted PTP clocks.
If it was just the direct hardware access, similar to RTC, which user
apps then use as a timesource, I'd not have much of a problem. But as
I mentioned in an earlier private mail, the abstraction level concerns
me.

1) The driver-like model exposes a char dev for each clock, which
allows for poorly-written userland applications to hit portability
issues  (ie: /dev/hpet vs /dev/rtc). Granted this isn't a huge flaw,
but good APIs should be hard to get wrong.

2) As Arnd already mentioned, the chardev interface seems to duplicate
the clock_gettime/settime() and adjtimex() interfaces.

3) I'm not sure I see the benefit of being able to have multiple
frequency corrected time domains.  In other words, what benefit would
you get from adjusting a PTP clock's frequency instead of just
adjusting the system's time freq? Having the PTP time as a reference
to correct the system time seems reasonable, but I'm not sure I see
why userland would want to adjust the PTP clock's freq.

thanks
-john

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Richard Cochran @ 2010-08-16 19:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rodolfo Giometti, netdev, devicetree-discuss, linux-kernel,
	linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <201008161626.24083.arnd@arndb.de>

On Mon, Aug 16, 2010 at 04:26:23PM +0200, Arnd Bergmann wrote:
> Have you considered integrating the subsystem into the Posix clock/timer
> framework?

Yes, but see below.
 
> I can't really tell from reading the source if this is possible or
> not, but my feeling is that if it can be done, that would be a much
> nicer interface. We already have clock_gettime/clock_settime/
> timer_settime/... system calls, and while you'd need to add another
> clockid and some syscalls, my feeling is that it will be more
> usable in the end.

You are not the first person to ask about this. See this link for
longer explanation of why I did not go that way:

  http://marc.info/?l=linux-netdev&m=127669810232201&w=2

You *could* offer the PTP clock as a Linux clock source/event device,
and I agree that it would be nicer. However, the problem is, what do
you do with the PHY based clocks?  Just one 16 bit read from a PHY
clock can take 40 usec, and you need four such read operations just to
get the current time value.

Also, I really did not want to add or change any syscalls. I could not
see a practical way to extend the existing syscalls to accommodate PTP
clocks.

Richard

^ permalink raw reply

* RE: [PATCH 9/9] RapidIO: Add support for IDT CPS Gen2 switches
From: Bounine, Alexandre @ 2010-08-16 18:30 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C6930EA.8030406@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > +	rio_mport_write_config_32(mport, destid, hopcount,
> > +				  LOCAL_RTE_CONF_DESTID_SEL, table);
> > +
> > +	for (i =3D 0x80000000; i <=3D 0x800000ff;) {
> > +		rio_mport_write_config_32(mport, destid, hopcount,
> > +			RIO_STD_RTE_CONF_DESTID_SEL_CSR, i);
>=20
> The 0x80000000 is that an autoincrement bit? If so, it only needs to
do
> this once I think? If so please call it like that, and loop the 'i'
> variable through the destination IDs.

No. This is EXTENDED_CONFIGURATION_ENABLE bit defined in Part 3 of RIO
spec (v.2.1, sect.3.5.4).
Anyway, I should add definition for this bit. Will do now.

Alex.
  =20

^ permalink raw reply

* RE: [PATCH 0/9] RapidIO: Set of patches to add Gen2 switches
From: Bounine, Alexandre @ 2010-08-16 18:18 UTC (permalink / raw)
  To: Micha Nelissen
  Cc: linuxppc-dev, akpm, Bounine, Alexandre, linux-kernel, Thomas Moll
In-Reply-To: <4C692F7E.1060208@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > This set of RapidIO patches adds support for new IDT Gen2 sRIO
switch
> > devices - CPS-1848 and CPS-1616.
> > Adding these sRIO switches required to implement standard error
recovery
> > mechanism defined by the RapidIO specification.
>=20
> This is not 'Gen2' specific, as these error management extensions also
> exist in v1.2/1.3 (?) of the specification? E.g. tsi56x and tsi57x
could
> support this functionality?

Correct. EM extensions exist since v1.3. But implementation before Gen2
switches relied
on proprietary device specific mechanism (tsi57x). Addition of Gen2
switches just=20
pushed forward the standard implementation. The RIO standard methods may
be used on 1.3 switches
(tested on tsi57x) as well as the standard routing table operations that
have been added
in our patches for 2.6.35.
   =20
>=20
> Micha

Alex.

^ permalink raw reply

* RE: [PATCH 7/9] RapidIO: Add handling for PW message from a lost device
From: Bounine, Alexandre @ 2010-08-16 18:02 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C692F31.2020705@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > Add check if PW message source device is accessible and change PW
message
> > handler to recover if PW message source device is not available
anymore (power
> > down or link disconnect).
>=20
> I am not quite sure what the point is of this patch. What do you need
to
> recover from?

>From failed maintenance read. In the previous version PW handler had
troubles if maintenance
read request fails. Now I am trying to detect lost or remover devices as
soon as I see broken link.

>=20
> > To avoid possible loss of notification, the PW message handler scans
the route
> > back from the source device to identify end of the broken link.
>=20
> Do you mean if port-writes are dropped? Then they did not reach you in
> the first place. If a link in between is broken, the associated switch
> will 'complain' and send port-writes, no?

Situation that I am trying to resolve is mostly applicable to larger
systems that have multiple complex boards (or chassis/domains) connected
together. Power down sequence on the board (chassis) combined with
switch hierarchy may allow switch to send PW message to the host before
its power is off. This will create an orphaned PW message.=20
At the same time there is no guarantee that PW message from the
associated switch will reach the host.
That "real" PW message may be dropped by the controller (85xx is good
example). Everything depends on number of PW messages directed to the
host/controller. I am trying to use the first available notification to
service device removal. If the "real" PW message is received it should
be processed without any further action.=20

Alex.

^ permalink raw reply

* RE: [PATCH 6/9] RapidIO: Add switch-specific sysfs initialization callback
From: Bounine, Alexandre @ 2010-08-16 17:10 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C692E4C.4040300@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > -	if (!rdev->rswitch)
> > -		goto out;
> > -
>=20
> Is it safe? All devices have a switch?

Yes. Because end-points should not have the "routes" attribute at all
(corrected by this patch).

>=20
> > @@ -63,10 +59,11 @@ struct device_attribute rio_dev_attrs[] =3D {
> >  	__ATTR_RO(asm_did),
> >  	__ATTR_RO(asm_vid),
> >  	__ATTR_RO(asm_rev),
> > -	__ATTR_RO(routes),
> >  	__ATTR_NULL,
> >  };
> >
> > +static DEVICE_ATTR(routes, S_IRUGO, routes_show, NULL);
> > +
>=20
> This seems a separate change from the sw_sysfs? Why make it separate?

I assume that your question was "Why do not make it separate?"
Both changes are specific to switches, both address sysfs and both are
not big enough to justify
a separate patch.
I agree that make separate patches would give more clarity, so would do
better description.
Because there are changes that should be made to other patches in this
set, I will regenerate this patch with better description.

>=20
> >   */
> >  struct rio_switch {
> > @@ -256,6 +257,7 @@ struct rio_switch {
> >  			   u8 *sw_domain);
> >  	int (*em_init) (struct rio_dev *dev);
> >  	int (*em_handle) (struct rio_dev *dev, u8 swport);
> > +	int (*sw_sysfs) (struct rio_dev *dev, int create);
> >  	struct rio_dev *nextdev[0];
> >  };
>=20
> Why not make a sw_sysfs_create and sw_sysfs_remove? Is better for
> readability. Now you call 'sw_sysfs(dev, 0)' or 'sw_sysfs(dev, 1)';

I just do not want to have an extra member here. Not every switch will
require own sysfs attributes, but every switch will be presented by a
data structure. Based on its intended use I do not see any problem here.

^ permalink raw reply

* Re: Linuxppc-dev Digest, Vol 72, Issue 63
From: Manikandan Ramachandran @ 2010-08-16 17:01 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <mailman.5.1281664801.6486.linuxppc-dev@lists.ozlabs.org>

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

> Date: Thu, 12 Aug 2010 13:53:51 -0400
> From: Jeff Angielski <jeff@theptrgroup.com>
> To: linuxppc-dev@lists.ozlabs.org
> Subject: Re: Query regarding 2.6.335 RT[Ingo's] and Non-RT performance
> Message-ID: <4C64352F.4090005@theptrgroup.com>
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
>
> On 08/11/2010 06:18 PM, Manikandan Ramachandran wrote:
> > Hello All,
> >      I created a very simple program which has higher priority than
> > normal tasks and runs a tight loop. Under same test environment I ran
> > this program on both non-rt and rt 2.6.33.5 kernel.  To my suprise I see
> > that performance of non-RT kernel is better than RT. non-RT kernel took
> > 3 sec and 366156 usec while RT kernel took about 3 sec and 418011
> > usec.Can someone please explain why the performance of non-rt kernel is
> > better than rt kernel? From the face of the test result, I feel RT has
> > more overhead,Is there any configuration that I could do to bring down
> > the overhead?
>
> Your "surprise" is due to your definition of "performance".
>
> The purpose of the -rt kernels is to reduce the kernel latency.  This is
> important for servicing hardware.  Normal users find the -rt useful for
> audio/video applications.  Engineering and scientific users find the -rt
> beneficially for servicing hardware like sensors or control systems.
>
> If you are just trying to run calculations as fast as you can in user
> space, you'd be better off using the non-rt variants.
>
>
> --
> Jeff Angielski
> The PTR Group
> www.theptrgroup.com
>
>
> Thanks for your response.
>
> On one hand I hear that RT-kernel is meant for reducing kernel latency on
> other hand I see that there is RT-kernel overhead. So what really RT-kernel
> brings to system performance?
>
> Actually I see that latency for higher priority is more or less same for
> non-rt system.
>
> One more thing, since irqs being threaded in RT, and with CFS scheduler in
> 2.6.33, wouldn't we bring down system performance as CFS is O(log(n))  in
> nature?
> --
> Thanks,
> Manik
>

[-- Attachment #2: Type: text/html, Size: 2639 bytes --]

^ permalink raw reply

* RE: [PATCH 2/9] RapidIO, powerpc/85xx: modify RIO port-write interrupt handler
From: Bounine, Alexandre @ 2010-08-16 15:27 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C692B42.9050407@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > - Rearranged RIO port-write interrupt handling to perform message
buffering
> > as soon as possible.
>=20
> I don't understand this comment: you still schedule work to read the
> port-write queue; so how is this message buffering performed as soon
as
> possible?


Compared to the original code, I rearranged order of checking interrupt
status bits to check the queue status first. The 85xx PW controller is
capable to receive and keep only one PW message. Therefore, I copy it
into the driver's FIFO and re-enable HW Rx queue (it is called queue but
can accept only one entry) ASAP. I have a test setup that is capable
generate multiple PW messages and see many messages discarded by PW
controller
because of this single-entry HW queue.
=20
>=20
> > - Modified to disable port-write controller when clearing
Transaction Error (TE)
> > bit.
> >  	/* Schedule deferred processing if PW was received */
> > -	if (ipwsr & RIO_IPWSR_QFI) {
> > +	if ((ipwmr & RIO_IPWMR_QFIE) && (ipwsr & RIO_IPWSR_QFI)) {
>=20
> Why check the QFIE bit also?

Oops! Leftover from some testing. Will clean it up.

>=20
> > +pw_done:
> > +	if (epwisr & 0x80000000) {
>=20
> Magic value.


Agree. Will correct.

^ permalink raw reply

* Re: Distinguish between kernel and user space
From: Ira W. Snyder @ 2010-08-16 15:22 UTC (permalink / raw)
  To: Ravi Gupta; +Cc: linuxppc-dev, MJ embd, linuxppc-dev
In-Reply-To: <AANLkTi=ZSeYf0apwK3Tuf3b3-OTsRrddMkNHkxgViw-x@mail.gmail.com>

On Mon, Aug 16, 2010 at 05:52:41PM +0530, Ravi Gupta wrote:
> Hi,
> 
> I have defined a header file for ioctls macros definitions. I am including
> it in both, my user space application as well as in my device driver. Now
> there are some macros that I want to be visible only in device drive and
> some only in user space application. Is there any set of macros defined in
> linux for such purpose? This is my present header file
> 
> gpio_ioctl.h
> 
> #ifndef _GPIO_IOCTL_H_
> #define _GPIO_IOCTL_H_
> 
> /*
>  * Ioctl definitions
>  */
> 
> /* Use 250 as type/magic number */
> #define GPIO_IOC_MAGIC    250
> #define GPIO_READ         _IOW (GPIO_IOC_MAGIC, 0, int)
> #define GPIO_WRITE        _IOW (GPIO_IOC_MAGIC, 1, int)
> #define GPIO_IOC_MAXNR    2
> 
> 
> #ifdef _DEVICE_DRIVE_ ---> these macros should not be visible in user space
> application
> #define ...
> ...
> ..
> #endif
> 
> #ifdef _USER_SPACE_ ---> these macros should not be visible in device driver
> i.e kernel space
> #define ...
> ...
> ..
> 
> #endif  /*_GPIO_IOCTL_H_ */
> 

#ifdef __KERNEL__
/* kernel only definitions here */
#else
/* userspace only definitions here */
#endif

See tons of headers in include/linux/ for examples.

Ira

^ permalink raw reply

* RE: [PATCH 5/9] RapidIO: Add default handler for error_stopped state
From: Bounine, Alexandre @ 2010-08-16 14:47 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C692D4D.1030401@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > +
> > +	if (err_status & RIO_PORT_N_ERR_STS_PW_OUT_ES) {
> > +		pr_debug("RIO_EM: servicing Output Error-Stopped
state\n");
> > +		/*
> > +		 * Send a Link-Request/Input-Status control symbol
> > +		 */
> > +
> > +		/* Read from link maintenance response register
> > +		 * to clear valid bit */
> > +		rio_mport_read_config_32(mport, destid, hopcount,
> > +			rdev->phys_efptr + RIO_PORT_N_MNT_RSP_CSR(pnum),
> > +			&regval);
> > +		udelay(50);
>=20
> Perhaps this whole part of operating the MNT_RSP registers (sending
> link-request symbol is the only useful action IIRC?) can be put in a
> separate function for readability.
>=20
I was on the fence with this one. Now I have an extra vote in favor of a
separate function ;)
Will do.
=20

^ permalink raw reply

* Re: [PATCH 0/8] v5 De-couple sysfs memory directories from memory sections
From: Nathan Fontenot @ 2010-08-16 14:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linuxppc-dev, Greg KH, linux-kernel, Dave Hansen, linux-mm,
	KAMEZAWA Hiroyuki
In-Reply-To: <20100812120816.e97d8b9e.akpm@linux-foundation.org>

On 08/12/2010 02:08 PM, Andrew Morton wrote:
> On Mon, 09 Aug 2010 12:53:00 -0500
> Nathan Fontenot <nfont@austin.ibm.com> wrote:
> 
>> This set of patches de-couples the idea that there is a single
>> directory in sysfs for each memory section.  The intent of the
>> patches is to reduce the number of sysfs directories created to
>> resolve a boot-time performance issue.  On very large systems
>> boot time are getting very long (as seen on powerpc hardware)
>> due to the enormous number of sysfs directories being created.
>> On a system with 1 TB of memory we create ~63,000 directories.
>> For even larger systems boot times are being measured in hours.
> 
> And those "hours" are mainly due to this problem, I assume.

Yes, those hours are spent creating the sysfs directories for each
of the memory sections.

> 
>> This set of patches allows for each directory created in sysfs
>> to cover more than one memory section.  The default behavior for
>> sysfs directory creation is the same, in that each directory
>> represents a single memory section.  A new file 'end_phys_index'
>> in each directory contains the physical_id of the last memory
>> section covered by the directory so that users can easily
>> determine the memory section range of a directory.
> 
> What you're proposing appears to be a non-back-compatible
> userspace-visible change.  This is a big issue!
> 
> It's not an unresolvable issue, as this is a must-fix problem.  But you
> should tell us what your proposal is to prevent breakage of existing
> installations.  A Kconfig option would be good, but a boot-time kernel
> command line option which selects the new format would be much better.

This shouldn't break existing installations, unless an architecture chooses
to do so.  With my patch only the powerpc/pseries arch is updated such that
what is seen in userspace is different.

The default behavior is maintained for all architectures unless they define
their own version of memory_block_size_bytes().  The default definition of
this routine (defined as __weak in Patch 5/8) sets the memory block size
to the same size it currently is, and thus preserving the exisitng 1 sysfs
directory per memory section.  The only change that will be seen is a new
propery for memory section, end_phys_addr, which will have the same value
as the existing 'phys_addr' property.

> 
> However you didn't mention this issue at all, and it's the most
> important one.
> 
> 
>> Updates for version 5 of the patchset include the following:
>>
>> Patch 4/8 Add mutex for add/remove of memory blocks
>> - Define the mutex using DEFINE_MUTEX macro.
>>
>> Patch 8/8 Update memory-hotplug documentation
>> - Add information concerning memory holes in phys_index..end_phys_index.
> 
> And you forgot to tell us how long those machines boot with the
> patchset applied, which is the entire point of the patchset!

Yes,  I am working on getting more time on our large systems to get
performance numbers with this patch.  I'll post them when I get them.

-Nathan

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Arnd Bergmann @ 2010-08-16 14:26 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Rodolfo Giometti, netdev, devicetree-discuss, linux-kernel,
	linuxppc-dev, linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <363bd749a38d0b785d8431e591bf54c38db4c2d7.1281956490.git.richard.cochran@omicron.at>

On Monday 16 August 2010, Richard Cochran wrote:
> This patch adds an infrastructure for hardware clocks that implement
> IEEE 1588, the Precision Time Protocol (PTP). A class driver offers a
> registration method to particular hardware clock drivers. Each clock is
> exposed to user space as a character device with ioctls that allow tuning
> of the PTP clock.

Have you considered integrating the subsystem into the Posix clock/timer
framework?

I can't really tell from reading the source if this is possible or
not, but my feeling is that if it can be done, that would be a much
nicer interface. We already have clock_gettime/clock_settime/
timer_settime/... system calls, and while you'd need to add another
clockid and some syscalls, my feeling is that it will be more
usable in the end.

> +static const struct file_operations ptp_fops = {
> +       .owner          = THIS_MODULE,
> +       .ioctl          = ptp_ioctl,
> +       .open           = ptp_open,
> +       .poll           = ptp_poll,
> +       .read           = ptp_read,
> +       .release        = ptp_release,
> +};

.ioctl is gone, you have to use .unlocked_ioctl and make sure that
your ptp_ioctl function can handle being called concurrently.

You should also add a .compat_ioctl function, ideally one that
points to ptp_ioctl so you don't have to list every command as
being compatible. Right now, some commands are incompatible,
which means you need more changes to the interface.

> +struct ptp_clock_timer {
> +       int alarm_index;       /* Which alarm to query or configure. */
> +       int signum;            /* Requested signal. */
> +       int flags;             /* Zero or TIMER_ABSTIME, see TIMER_SETTIME(2) */
> +       struct itimerspec tsp; /* See TIMER_SETTIME(2) */
> +};

This data structure is incompatible between 32 and 64 bit user space,
so you would need a compat_ioctl() function to convert between the
two formats. Better define the structure in a compatible way, avoiding
variable-length fields and implicit padding.

> +struct ptp_clock_request {
> +       int type;  /* One of the ptp_request_types enumeration values. */
> +       int index; /* Which channel to configure. */
> +       struct timespec ts; /* For period signals, the desired period. */
> +       int flags; /* Bit field for PTP_ENABLE_FEATURE or other flags. */
> +};

Same problem here, timespec is either 64 or 128 bits long and has different
alignment.

> +struct ptp_extts_event {
> +       int index;
> +       struct timespec ts;
> +};

here too.

> +#define PTP_CLOCK_VERSION 0x00000001
> +
> +#define PTP_CLK_MAGIC '='
> +
> +#define PTP_CLOCK_APIVERS _IOR (PTP_CLK_MAGIC, 1, __u32)

Try avoiding versioned ABIs. It's better to just add new ioctls
or syscalls when you need some extra functionality and leave the
old ones around.

> +#define PTP_CLOCK_ADJTIME _IOW (PTP_CLK_MAGIC, 3, struct timespec)
> +#define PTP_CLOCK_GETTIME _IOR (PTP_CLK_MAGIC, 4, struct timespec)
> +#define PTP_CLOCK_SETTIME _IOW (PTP_CLK_MAGIC, 5, struct timespec)

These are also incompatible.

	Arnd

^ permalink raw reply

* RE: [PATCH 4/9] RapidIO: Add relation links between RIO device structures
From: Bounine, Alexandre @ 2010-08-16 14:00 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel, Bounine, Alexandre
In-Reply-To: <4C692C9D.6040100@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > Create back and forward links between RIO devices. These links are
intended for
> > use by error management and hot-plug extensions.
>=20
> As RapidIO is a switched network, the concept of 'previous' and 'next'
> devices is invalid. Perhaps it's just the way they were
> discovered/enumerated, but that does not matter any more at runtime.
Or
> at least, should not matter.
>

Yes, the "previous" and "next" have to be considered in context of
enumeration/discovery.
At runtime, it does not matter for data traffic, but is valuable
information for error recovery
And hot-swap. This provides snapshot of device connections. For example,
when servicing
error-stopped state of link between two switches you need to know
ingress port of far switch on
the link. It may be a problem to read it directly from the device.
Searching through the device list
becomes time consuming as a system size grows.     =20
=20
> > @@ -237,6 +241,7 @@ struct rio_switch {
> >  	u16 hopcount;
> >  	u16 destid;
> >  	u8  inport;
> > +	u8  nports;
> >  	u8 *route_table;
> >  	u32 port_ok;
>=20
> This can be extracted from the swpinfo field (which you removed in
> another patch)?
>=20
Agreed in response for patch #3

^ permalink raw reply

* RE: [PATCH 3/9] RapidIO: Add the ingress port number into the RIO switch data structure
From: Bounine, Alexandre @ 2010-08-16 13:40 UTC (permalink / raw)
  To: Micha Nelissen; +Cc: linuxppc-dev, akpm, linux-kernel
In-Reply-To: <4C692BBB.8090707@neli.hopto.org>

Micha Nelissen wrote:
>=20
> Alexandre Bounine wrote:
> > A switch ingress port number has to be saved for software assisted
error
> > recovery from the error-stopped state. Saving this information also
allows
> > to remove several register reads from the RIO enumeration process.
>=20
> Why not keep using the swpinfo field, as you can extract it from
there?
> And your code actually does this.
>=20
> Also processor devices can have multiple ports (not only switches), it
> would be good to be ready for that use case.
>=20
Agree. I missed the Multiport bit introduced in RIO spec 2.1. Now it is
justified to be as it was before.
I will rework this and next patches.=20

^ permalink raw reply

* Re: [PATCH 00/27] KVM PPC PV framework v3
From: Alexander Graf @ 2010-08-16 13:11 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, KVM list, kvm-ppc
In-Reply-To: <17DF4824-9EC7-44F4-9408-EA6241C33E70@kernel.crashing.org>


On 06.08.2010, at 18:28, Kumar Gala wrote:

>=20
> On Jul 29, 2010, at 7:47 AM, Alexander Graf wrote:
>=20
>> [without]
>>=20
>> debian-powerpc:~# time for i in {1..1000}; do /bin/echo hello > =
/dev/null; done
>>=20
>> real    0m14.659s
>> user    0m8.967s
>> sys     0m5.688s
>>=20
>> [with]
>>=20
>> debian-powerpc:~# time for i in {1..1000}; do /bin/echo hello > =
/dev/null; done
>>=20
>> real    0m7.557s
>> user    0m4.121s
>> sys     0m3.426s
>=20
> Do you have #'s for w/o HV to compare to.

Native was around 2-3 seconds overall. So we still have quite a big =
impact on performance, but things are improving :).

Alex

^ permalink raw reply

* Re: [PATCH 9/9] RapidIO: Add support for IDT CPS Gen2 switches
From: Micha Nelissen @ 2010-08-16 12:36 UTC (permalink / raw)
  To: Alexandre Bounine; +Cc: linuxppc-dev, akpm, linux-kernel
In-Reply-To: <1281712686-31308-10-git-send-email-alexandre.bounine@idt.com>

Alexandre Bounine wrote:
> +	rio_mport_write_config_32(mport, destid, hopcount,
> +				  LOCAL_RTE_CONF_DESTID_SEL, table);
> +
> +	for (i = 0x80000000; i <= 0x800000ff;) {
> +		rio_mport_write_config_32(mport, destid, hopcount,
> +			RIO_STD_RTE_CONF_DESTID_SEL_CSR, i);

The 0x80000000 is that an autoincrement bit? If so, it only needs to do 
this once I think? If so please call it like that, and loop the 'i' 
variable through the destination IDs.

Micha

^ permalink raw reply

* Re: [PATCH 0/9] RapidIO: Set of patches to add Gen2 switches
From: Micha Nelissen @ 2010-08-16 12:30 UTC (permalink / raw)
  To: Alexandre Bounine; +Cc: linuxppc-dev, akpm, linux-kernel, Thomas Moll
In-Reply-To: <1281712686-31308-1-git-send-email-alexandre.bounine@idt.com>

Alexandre Bounine wrote:
> This set of RapidIO patches adds support for new IDT Gen2 sRIO switch
> devices - CPS-1848 and CPS-1616.
> Adding these sRIO switches required to implement standard error recovery
> mechanism defined by the RapidIO specification.

This is not 'Gen2' specific, as these error management extensions also 
exist in v1.2/1.3 (?) of the specification? E.g. tsi56x and tsi57x could 
support this functionality?

Micha

^ permalink raw reply

* Re: [PATCH 7/9] RapidIO: Add handling for PW message from a lost device
From: Micha Nelissen @ 2010-08-16 12:29 UTC (permalink / raw)
  To: Alexandre Bounine; +Cc: linuxppc-dev, akpm, linux-kernel
In-Reply-To: <1281712686-31308-8-git-send-email-alexandre.bounine@idt.com>

Alexandre Bounine wrote:
> Add check if PW message source device is accessible and change PW message
> handler to recover if PW message source device is not available anymore (power
> down or link disconnect).

I am not quite sure what the point is of this patch. What do you need to 
recover from?

> To avoid possible loss of notification, the PW message handler scans the route
> back from the source device to identify end of the broken link.

Do you mean if port-writes are dropped? Then they did not reach you in 
the first place. If a link in between is broken, the associated switch 
will 'complain' and send port-writes, no?

Micha

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox