* [PATCH v3 0/4] powerpc: Open PIC binding and "pic-no-reset"
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
This patch set provides a binding for Open PIC and implements support for
a new property, specified by that binding, called "pic-no-reset". With
"pic-no-reset" in place the "protected-sources" property is no longer needed
and its full implementation was removed. "protected-sources" is still checked
for, however, for legacy purposes.
For v3 of this patch the Open PIC binding was changed to be more consistent
with existing bindings, several DTS files were cleaned up, "no-reset" was
changed to "pic-no-reset", and a check to treat "protected-sources" as a
synonym for "pic-no-reset" was added.
Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Meador Inge (4):
powerpc: Removing support for 'protected-sources'
powerpc: document the Open PIC device tree binding
powerpc: make MPIC honor the "pic-no-reset" device tree property
powerpc: Replacing "protected-sources" with "pic-no-reset" in DTS
files
Documentation/powerpc/dts-bindings/open-pic.txt | 98 ++++++++++++++++++++++
arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts | 6 +-
arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts | 11 +--
arch/powerpc/boot/dts/p2020rdb_camp_core0.dts | 7 +--
arch/powerpc/boot/dts/p2020rdb_camp_core1.dts | 7 +--
arch/powerpc/include/asm/mpic.h | 7 +-
arch/powerpc/sysdev/mpic.c | 102 ++++++++++++-----------
7 files changed, 157 insertions(+), 81 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
^ permalink raw reply
* Re: BootX
From: Benjamin Herrenschmidt @ 2011-02-04 20:47 UTC (permalink / raw)
To: kevin diggs; +Cc: linuxppc-dev
In-Reply-To: <AANLkTikxL1ZrTmKLvQ--JDZyu=80AM8kUUhi4BGNLz0O@mail.gmail.com>
On Fri, 2011-02-04 at 11:21 -0600, kevin diggs wrote:
>
> P.S.: I posted some documentation for dump_stack()/show_stack() but
> have not heard anything? Is that not something we are interested in
> doing?
Or I haven't had a chance to review it yet ... I'm very very busy at the
moment so things are lagging a bit.
Ben.
^ permalink raw reply
* Re: BootX
From: kevin diggs @ 2011-02-04 17:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1296685762.2349.739.camel@pasglop>
Hi,
FYI:
This driver has some pretty good diagnostics/debug capabilities built
in. Once that is enabled it shows that the inquiry works and the sync
negotiation works. The next command (I think) is test unit ready,
which does not work. It is retried multiple times. The result is 20000
which I think is DID_BUS_BUSY. Probably in mesh_start_cmd(), possibly
something off in the loop at line 462?
kevin
P.S.: I posted some documentation for dump_stack()/show_stack() but
have not heard anything? Is that not something we are interested in
doing?
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Dave Kleikamp @ 2011-02-04 14:44 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20110204135627.GF17643@zod.rchland.ibm.com>
On Fri, 2011-02-04 at 08:56 -0500, Josh Boyer wrote:
> On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> >diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c
> >index 2a030d8..b33c5e6 100644
> >--- a/arch/powerpc/mm/tlb_nohash.c
> >+++ b/arch/powerpc/mm/tlb_nohash.c
> >@@ -35,6 +35,7 @@
> > #include <linux/preempt.h>
> > #include <linux/spinlock.h>
> > #include <linux/memblock.h>
> >+#include <linux/of_fdt.h>
> >
> > #include <asm/tlbflush.h>
> > #include <asm/tlb.h>
> >@@ -153,6 +154,17 @@ EXPORT_SYMBOL(local_flush_tlb_page);
> > */
> > #ifdef CONFIG_SMP
> >
> >+static int amp;
> >+
> >+#ifdef CONFIG_44x
> >+void __init early_init_mmu_44x(void)
> >+{
> >+ unsigned long root = of_get_flat_dt_root();
> >+ if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> >+ amp = 1;
> >+}
> >+#endif /* CONFIG_44x */
>
> Something aside from the property thing sits weirdly with me on this as
> well.
>
> We have this guarded by CONFIG_44x but also CONFIG_SMP, and we're doing
> 476 specific checks (for now). There is at least one 44x board that has
> dual-CPUs (AMCC Arches, iirc) that can theoretically be run in AMP mode.
> However, it won't be using an SMP kernel because it's a single core per CPU.
> Admittedly I don't think it supports the tlbivax instruction either so
> the patch as it stands doesn't impact that theoretical scenario much.
I should have used CONFIG_PPC_47x here.
> I do wonder if we really need to guard the call to this behind
> CONFIG_SMP though. Maybe a slight performance increase I suppose, but
> if we wind up using the AMP check elsewhere then it might be needed
> anyway. Something to think about.
I agree that it's awkward. The code affected by this is all behind
CONFIG_SMP. There's no reason to use tlbivax, or the alternate ipi, in
a uni kernel. An alternative would be to define early_init_mmu_44x (or
47x) outside of CONFIG_SMP, but the contents of the function would still
be inside CONFIG_SMP, and it would be an empty function otherwise.
> Oh, and I agree 'cooperative-partition' or something would be a better
> check.
I'm good with that then.
>
> josh
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH V2 5/6] powerpc/44x: boot wrapper: allow kernel to load into non-zero address
From: Dave Kleikamp @ 2011-02-04 14:44 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20110204140732.GG17643@zod.rchland.ibm.com>
On Fri, 2011-02-04 at 09:07 -0500, Josh Boyer wrote:
> On Tue, Feb 01, 2011 at 12:48:45PM -0600, Dave Kleikamp wrote:
> >+static ibm4xx_memstart;
> >+
> > static void iss_4xx_fixups(void)
> > {
> >- ibm4xx_sdram_fixup_memsize();
> >+ void *memory;
> >+ u32 reg[3];
> >+
> >+ memory = finddevice("/memory");
> >+ if (!memory)
> >+ fatal("Can't find memory node\n");
> >+ getprop(memory, "reg", reg, sizeof(reg));
> >+ if (reg[1] || reg[2])
>
> Wouldn't this test allow for a faulty device tree that specified a 0
> size (0x0 in reg[2])? Maybe just check reg[2] for non-zero instead?
That make sense.
>
> josh
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH V2 5/6] powerpc/44x: boot wrapper: allow kernel to load into non-zero address
From: Josh Boyer @ 2011-02-04 14:07 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296586126-32765-6-git-send-email-shaggy@linux.vnet.ibm.com>
On Tue, Feb 01, 2011 at 12:48:45PM -0600, Dave Kleikamp wrote:
>+static ibm4xx_memstart;
>+
> static void iss_4xx_fixups(void)
> {
>- ibm4xx_sdram_fixup_memsize();
>+ void *memory;
>+ u32 reg[3];
>+
>+ memory = finddevice("/memory");
>+ if (!memory)
>+ fatal("Can't find memory node\n");
>+ getprop(memory, "reg", reg, sizeof(reg));
>+ if (reg[1] || reg[2])
Wouldn't this test allow for a faulty device tree that specified a 0
size (0x0 in reg[2])? Maybe just check reg[2] for non-zero instead?
josh
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Josh Boyer @ 2011-02-04 13:56 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296586126-32765-5-git-send-email-shaggy@linux.vnet.ibm.com>
On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
>diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c
>index 2a030d8..b33c5e6 100644
>--- a/arch/powerpc/mm/tlb_nohash.c
>+++ b/arch/powerpc/mm/tlb_nohash.c
>@@ -35,6 +35,7 @@
> #include <linux/preempt.h>
> #include <linux/spinlock.h>
> #include <linux/memblock.h>
>+#include <linux/of_fdt.h>
>
> #include <asm/tlbflush.h>
> #include <asm/tlb.h>
>@@ -153,6 +154,17 @@ EXPORT_SYMBOL(local_flush_tlb_page);
> */
> #ifdef CONFIG_SMP
>
>+static int amp;
>+
>+#ifdef CONFIG_44x
>+void __init early_init_mmu_44x(void)
>+{
>+ unsigned long root = of_get_flat_dt_root();
>+ if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
>+ amp = 1;
>+}
>+#endif /* CONFIG_44x */
Something aside from the property thing sits weirdly with me on this as
well.
We have this guarded by CONFIG_44x but also CONFIG_SMP, and we're doing
476 specific checks (for now). There is at least one 44x board that has
dual-CPUs (AMCC Arches, iirc) that can theoretically be run in AMP mode.
However, it won't be using an SMP kernel because it's a single core per CPU.
Admittedly I don't think it supports the tlbivax instruction either so
the patch as it stands doesn't impact that theoretical scenario much.
I do wonder if we really need to guard the call to this behind
CONFIG_SMP though. Maybe a slight performance increase I suppose, but
if we wind up using the AMP check elsewhere then it might be needed
anyway. Something to think about.
Oh, and I agree 'cooperative-partition' or something would be a better
check.
josh
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc: Removing support for 'protected-sources'
From: Arnd Bergmann @ 2011-02-04 12:17 UTC (permalink / raw)
To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <4D4B3A4E.2070106@mentor.com>
On Friday 04 February 2011, Meador Inge wrote:
> On 02/03/2011 09:56 AM, Arnd Bergmann wrote:
> So in 'mpic_init' we don't initialize anything and then in
> 'mpic_host_map' we lazily do the VECPRI and CPU binding initialization with:
>
> if (!(mpic->flags & MPIC_WANTS_RESET))
> if (!(mpic_is_ipi(mpic, hw)
> || mpic_is_timer_interrupt(mpic, hw)))
> mpic_init_vector(mpic, hw);
>
> Thus when 'no-reset' is thrown it ensures that only the sources which
> are mentioned in the device tree are actually initialized. The net
> effect should be the same as what 'protected-sources' was accomplishing,
> but without having to maintain the list of sources in the property cell.
That sounds like a good idea, but unfortunately, it's not what SLOF
implements on QS21/QS22. It's a legacy product and there won't be
any firmware updates. Moreover, it relies on the open firmware
implementation and cannot boot with a flattened device tree image,
so I don't see how your patch can work on the old systems.
Maybe you can treat the presence of a 'protected-sources' property
the same way that you treat the no-reset property?
Arnd
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Timur Tabi @ 2011-02-04 2:22 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <1296774957.14077.23.camel@shaggy-w500>
On Thu, Feb 3, 2011 at 5:15 PM, Dave Kleikamp <shaggy@linux.vnet.ibm.com> w=
rote:
>> Bit too short, I think. =A0I'd suggest either spelling out
>> 'asymmetric-multiprocessor' or 'cooperative-partition' (a more
>> accurate term, IMO).
>
> I could be wrong, but I thought the A stands for Asynchronous, not
> Asymmetric. =A0I thought Asymmetric means that different types of tasks
> run on the secondary processors, as on the Cell. =A0Anyway, going with
> 'cooperative-partition' would avoid that confusion.
Well, if we pretend that everyone already knows what the "A" stands
for, that's not going to avoid confusion. Some people still are going
to be wrong. It would be great if we could settle the matter once and
for all.
I've always thought the A stood for asymmetric, since you're running
multiple cores, but each OS is not aware of the others. It doesn't
necessarily have to be two copies of Linux. In fact, it usually
isn't.
As for "MPC8572DS-CAMP", I've always hated that. A specific property
that defines an AMP environment is a much better idea.
--=20
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-03 23:29 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <201102031656.38222.arnd@arndb.de>
On 02/03/2011 09:56 AM, Arnd Bergmann wrote:
> On Thursday 03 February 2011, Meador Inge wrote:
>> In a recent discussion [1, 2] concerning device trees for AMP systems, the
>> question of whether we really need 'protected-sources' arose. The general
>> consensus was that if you don't want a source to be used, then it should *not*
>> be mentioned in an 'interrupts' property. If a source really needs to be
>> mentioned, then it should be put in a property other than 'interrupts' with
>> a specific binding for that use case.
>>
>> [1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
>> [2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html
>
> That doesn't work in the case that this code was written for:
>
> http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg01394.html
>
> The problem is that you don't want the mpic to initialize the interrupt
> line to the default, but instead leave it at whatever the boot firmware
> has set up. Note that interrupt is not listed in any "interrupts"
> property of any of the devices on the CPU interpreting the device
> tree, but it may be mentioned in the device tree that another CPU
> uses to access the same MPIC.
>
> Arnd
We touched on that use case before on list. However, I did a really bad
job of explaining things in the above patch description. I understand
that the sources that are being protected are mentioned in a device tree
other than the one that actually interprets the 'protected-sources'
property.
The idea is to try and expand the meaning of the 'no-reset' property to
cover what 'protected-sources' was taking care of, but without
explicitly naming the sources.
In the protected sources version of the code, the relevant MPIC
initialization went something like (in 'mpic_init'):
for (i = 0; i < mpic->num_sources; i++) {
/* start with vector = source number, and masked */
u32 vecpri = MPIC_VECPRI_MASK | i |
(8 << MPIC_VECPRI_PRIORITY_SHIFT);
/* check if protected */
if (mpic->protected && test_bit(i, mpic->protected))
continue;
/* init hw */
mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
}
So unless a particular source was marked as protected, it would get the
VECPRI and CPU binding initialization. This is the exact behavior that
you describe above, Arnd.
In the 'no-reset' model, the initialization looks more like (see PATCH 3
in the set for the full implementation):
if (mpic->flags & MPIC_WANTS_RESET) {
for (i = 0; i < mpic->num_sources; i++) {
mpic_init_vector(mpic, hw);
}
}
So in 'mpic_init' we don't initialize anything and then in
'mpic_host_map' we lazily do the VECPRI and CPU binding initialization with:
if (!(mpic->flags & MPIC_WANTS_RESET))
if (!(mpic_is_ipi(mpic, hw)
|| mpic_is_timer_interrupt(mpic, hw)))
mpic_init_vector(mpic, hw);
Thus when 'no-reset' is thrown it ensures that only the sources which
are mentioned in the device tree are actually initialized. The net
effect should be the same as what 'protected-sources' was accomplishing,
but without having to maintain the list of sources in the property cell.
--
Meador Inge | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Dave Kleikamp @ 2011-02-03 23:15 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20110203050351.GK3032@yookeroo>
On Thu, 2011-02-03 at 16:03 +1100, David Gibson wrote:
> On Wed, Feb 02, 2011 at 05:53:59PM -0600, Dave Kleikamp wrote:
> > On Thu, 2011-02-03 at 10:08 +1100, David Gibson wrote:
> > > On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> > > > Since other OS's may be running on the other cores don't use tlbivax
> > >
> > > [snip]
> > > > +#ifdef CONFIG_44x
> > > > +void __init early_init_mmu_44x(void)
> > > > +{
> > > > + unsigned long root = of_get_flat_dt_root();
> > > > + if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> > > > + amp = 1;
> > > > +}
> > > > +#endif /* CONFIG_44x */
> > >
> > > A test against a hardcoded compatible string seems a nasty way to do
> > > this. Maybe we should define a new boolean property for the root
> > > node.
> >
> > I'm not crazy about this string, but I needed something in the device
> > tree to key off of. Freescale has something similar (i.e.
> > MPC8572DS-CAMP), so I chose to follow their example. I'd be happy to
> > replace it with a boolean property. Any objection to just using
> > "amp"?
>
> Bit too short, I think. I'd suggest either spelling out
> 'asymmetric-multiprocessor' or 'cooperative-partition' (a more
> accurate term, IMO).
I could be wrong, but I thought the A stands for Asynchronous, not
Asymmetric. I thought Asymmetric means that different types of tasks
run on the secondary processors, as on the Cell. Anyway, going with
'cooperative-partition' would avoid that confusion.
Shaggy
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: MMC on MPC8379
From: Gary Thomas @ 2011-02-03 18:48 UTC (permalink / raw)
To: Linux PPC Development
In-Reply-To: <4D4717AD.2040905@mlbassoc.com>
On 01/31/2011 01:12 PM, Gary Thomas wrote:
> I'm running 2.6.32 on MPC8379, trying to use the SDHCI interface.
> It seems to work, reads are fine, but when I write to the device
> (I've tried both eMMC soldered-on and pluggable MMC/SD cards),
> I get intermittent failures. Specifically, some blocks will be
> written while others fail. I haven't yet found a pattern.
>
> Is there anything special about this interface? I notice that
> it's not really used by any FSL platforms although the driver
> claims support.
On the surface, it looks like it should work and I'm perplexed
by the failures.
Does anyone have any ideas or experience with this device?
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
^ permalink raw reply
* [OT - MPC5200B] strange framing, break problems with uart
From: Albrecht Dreß @ 2011-02-03 18:34 UTC (permalink / raw)
To: Linux PPC Development
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
Hi all,
sorry for a slightly off-topic question, but I hope someone here on the list may be able to help me...
I have a strange problem with the psc uart of the mpc5200b, running 2.6.32.26 (still), with my baud rate divisor selection patch [1].
The uart runs at 115.2 kBaud with rtc/cts handshake to send bigger chunks of data to the '5200. I noticed "missing" data in the input stream, and inspected the uart status using the TIOCGICOUNT ioctl which tells me that a bunch of framing and break errors occurred. I "tapped" the RxD line and connected it via a level shifter to a standard 16450-style uart in a (much faster) Linux PC, and *that* one receives the *complete* stream *without any* break or framing errors!
I also looked at the waveforms with an oscilloscope, and they look pretty fine. The port configuration should also be ok, re-checked with a bdi3000 jtag debugger - it's PSC3, set to '1100', with PSC3_0 .. PSC3_3 being used here.
This leads me to the assumption that either the hardware handshake or the Linux driver or both are broken... any insight would be highly appreciated!
Cheers,
Albrecht.
[1] <http://patchwork.ozlabs.org/patch/48884/>; included in 2.6.37
[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* [PATCH] powerpc/85xx: fix compatible properties of the P1022DS DMA nodes used for audio
From: Timur Tabi @ 2011-02-03 17:39 UTC (permalink / raw)
To: stable, linux-kernel, linuxppc-dev
In order to prevent the fsl_dma driver from claiming the DMA channels that the
P1022DS audio driver needs, the compatible properties for those nodes must say
"fsl,ssi-dma-channel" instead of "fsl,eloplus-dma-channel".
Commit b2e0861e51f2961954330dcafe6d148ee3ab5cff upstream.
Signed-off-by: Timur Tabi <timur@freescale.com>
Cc: stable@kernel.org
---
arch/powerpc/boot/dts/p1022ds.dts | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/boot/dts/p1022ds.dts b/arch/powerpc/boot/dts/p1022ds.dts
index 2bbecbb..69422eb 100644
--- a/arch/powerpc/boot/dts/p1022ds.dts
+++ b/arch/powerpc/boot/dts/p1022ds.dts
@@ -291,13 +291,13 @@
ranges = <0x0 0xc100 0x200>;
cell-index = <1>;
dma00: dma-channel@0 {
- compatible = "fsl,eloplus-dma-channel";
+ compatible = "fsl,ssi-dma-channel";
reg = <0x0 0x80>;
cell-index = <0>;
interrupts = <76 2>;
};
dma01: dma-channel@80 {
- compatible = "fsl,eloplus-dma-channel";
+ compatible = "fsl,ssi-dma-channel";
reg = <0x80 0x80>;
cell-index = <1>;
interrupts = <77 2>;
--
1.7.3.4
^ permalink raw reply related
* RE: [PATCH v2 2/3] powerpc: document the Open PIC device tree binding
From: Yoder Stuart-B08248 @ 2011-02-03 17:02 UTC (permalink / raw)
To: Grant Likely, Meador Inge
Cc: linuxppc-dev@lists.ozlabs.org,
devicetree-discuss@lists.ozlabs.org, Hollis Blanchard
In-Reply-To: <AANLkTinVak_ukf0wP8=zbGTq8ENV4d88hNuzW_tqvCFf@mail.gmail.com>
> > + =A0- no-reset
> > + =A0 =A0 =A0Usage: optional
> > + =A0 =A0 =A0Value type: <empty>
> > + =A0 =A0 =A0Definition: The presence of this property indicates that t=
he
> > + PIC
> > + =A0 =A0 =A0 =A0 =A0should not be reset during runtime initialization.=
=A0The
> > + presence of
> > + =A0 =A0 =A0 =A0 =A0this property also mandates that any initializatio=
n related
> > + to
> > + =A0 =A0 =A0 =A0 =A0interrupt sources shall be limited to sources expl=
icitly
> > + referenced
> > + =A0 =A0 =A0 =A0 =A0in the device tree.
>=20
> Please follow the lead set by the other binding documentation which is mo=
re
> concise and tends to be of the form:
>=20
> Required properties:
> - reg : <description>
> - interrupt-controller : <description>
>=20
> Optional Properties:
> - no-reset : blah
>=20
> I'm considering formalizing the binding format so that fully specified an=
d
> cross-referenced documentation can be generated from the bindings
> directory.
Regarding the format-- The definition should also to specify the value
type. I don't see this being consistently done in existing bindings. =20
They are not completely unclear, but using consistent terms might help.
The ePAPR uses this convention:
<empty> # no value, a Boolean
<u32> # A 32-bit integer in big-endian format
<u64> # A 64-bit integer in big-endian format
<string> # null terminated
<prop-encoded-array> # format specific to the property
<phandle> # A <u32> value, referecnes another node
<stringlist> # A list of <string> values concatenated together.
The identifier prop-encoded-array came from precedence in other
of binding and ieee1275. prop-encoded-arrays should be
be specifically defined in terms of # of cells and the=20
meaning of each cell.
If you use the above types identifiers, there is no ambiguity.
Also, there are properties that don't necessarily fall in 'required'
and 'optional', but may be required depending on the context. Thus
the 'Usage' identifier which Meador derived from my mpic binding
posted. Usage could be:
Required
Optional
See Definition
Stuart
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc: document the Open PIC device tree binding
From: Grant Likely @ 2011-02-03 16:36 UTC (permalink / raw)
To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <4D4AD7EE.90207@mentor.com>
On Thu, Feb 3, 2011 at 9:29 AM, Meador Inge <meador_inge@mentor.com> wrote:
> On 02/03/2011 09:56 AM, Grant Likely wrote:
>>
>> On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge<meador_inge@mentor.com>
>> =A0wrote:
>>>
>>> This binding documents several properties that have been in use for qui=
te
>>> some time, and adds one new property 'no-reset', which controls whether
>>> the
>>> Open PIC should be reset during runtime initialization.
>>>
>>> The general formatting and interrupt specifier definition is based off =
of
>>> Stuart Yoder's FSL MPIC binding.
>>>
>>> Signed-off-by: Meador Inge<meador_inge@mentor.com>
>>> CC: Hollis Blanchard<hollis_blanchard@mentor.com>
>>> CC: Stuart Yoder<stuart.yoder@freescale.com>
>>> ---
>>> =A0Documentation/powerpc/dts-bindings/open-pic.txt | =A0115
>>> +++++++++++++++++++++++
>>> =A01 files changed, 115 insertions(+), 0 deletions(-)
>>> =A0create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>>>
>>> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt
>>> b/Documentation/powerpc/dts-bindings/open-pic.txt
>>> new file mode 100644
>>> index 0000000..447ef65
>>> --- /dev/null
>>> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
>>> @@ -0,0 +1,115 @@
>>> +* Open PIC Binding
>>> +
>>> +This binding specifies what properties must be available in the device
>>> tree
>>> +representation of an Open PIC compliant interrupt controller. =A0This
>>> binding is
>>> +based on the binding defined for Open PIC in [1] and is a superset of
>>> that
>>> +binding.
>>> +
>>> +PROPERTIES
>>> +
>>> + =A0NOTE: Many of these descriptions were paraphrased here from [1] to=
aid
>>> + =A0 =A0 =A0 =A0readability.
>>> +
>>> + =A0- compatible
>>> + =A0 =A0 =A0Usage: required
>>> + =A0 =A0 =A0Value type:<string>
>>> + =A0 =A0 =A0Definition: Specifies the compatibility list for the PIC. =
=A0The
>>> + =A0 =A0 =A0 =A0 =A0property value shall include "open-pic".
>>> +
>>> + =A0- reg
>>> + =A0 =A0 =A0Usage: required
>>> + =A0 =A0 =A0Value type:<prop-encoded-array>
>>> + =A0 =A0 =A0Definition: Specifies the base physical address(s) and siz=
e(s) of
>>> this
>>> + =A0 =A0 =A0 =A0 =A0PIC's addressable register space.
>>> +
>>> + =A0- interrupt-controller
>>> + =A0 =A0 =A0Usage: required
>>> + =A0 =A0 =A0Value type:<empty>
>>> + =A0 =A0 =A0Definition: The presence of this property identifies the n=
ode
>>> + =A0 =A0 =A0 =A0 =A0as an Open PIC. =A0No property value should be def=
ined.
>>> +
>>> + =A0- #interrupt-cells
>>> + =A0 =A0 =A0Usage: required
>>> + =A0 =A0 =A0Value type:<u32>
>>> + =A0 =A0 =A0Definition: Specifies the number of cells needed to encode=
an
>>> + =A0 =A0 =A0 =A0 =A0interrupt source. =A0Shall be 2.
>>> +
>>> + =A0- #address-cells
>>> + =A0 =A0 =A0Usage: required
>>> + =A0 =A0 =A0Value type:<u32>
>>> + =A0 =A0 =A0Definition: Specifies the number of cells needed to encode=
an
>>> + =A0 =A0 =A0 =A0 =A0address. =A0The value of this property shall alway=
s be 0.
>>> + =A0 =A0 =A0 =A0 =A0As such, 'interrupt-map' nodes do not have to spec=
ify a
>>> + =A0 =A0 =A0 =A0 =A0parent unit address.
>>> +
>>> + =A0- no-reset
>>> + =A0 =A0 =A0Usage: optional
>>> + =A0 =A0 =A0Value type:<empty>
>>> + =A0 =A0 =A0Definition: The presence of this property indicates that t=
he PIC
>>> + =A0 =A0 =A0 =A0 =A0should not be reset during runtime initialization.=
=A0The
>>> presence of
>>> + =A0 =A0 =A0 =A0 =A0this property also mandates that any initializatio=
n related to
>>> + =A0 =A0 =A0 =A0 =A0interrupt sources shall be limited to sources expl=
icitly
>>> referenced
>>> + =A0 =A0 =A0 =A0 =A0in the device tree.
>>
>> Please follow the lead set by the other binding documentation which is
>> more concise and tends to be of the form:
>>
>> =A0 =A0 Required properties:
>> =A0 =A0 =A0 =A0 - reg :<description>
>> =A0 =A0 =A0 =A0 - interrupt-controller :<description>
>>
>> =A0 =A0 Optional Properties:
>> =A0 =A0 =A0 =A0 - no-reset : blah
>
> OK, will do. =A0The one thing that I like about the other format, though,=
is
> that it specifies the value type. =A0That is a useful addition.
>
>> I'm considering formalizing the binding format so that fully specified
>> and cross-referenced documentation can be generated from the bindings
>> directory.
>
> Formalizing the binding format would be great. =A0Perhaps we should add a
> HOWTO write a new binding document to the "Documentation" directory? The
> would be a great place to capture some of the common pitfalls that have b=
een
> coming up on the list lately (versioned compatibility tags, for example).
>
>> Also, to avoid the potential of a future namespace collision, it would
>> not be a bad idea to name this openpic-no-reset or something that
>> makes it clear that this is a binding specific property. =A0"no-reset"
>> sounds generic enough to give me pause.
>
> Isn't that a little redundant, though (e.g. "/soc/pic/openpic-no-reset")?
> =A0It is already scoped to the PIC node:
>
> =A0 mpic: pic@40000 {
> =A0 =A0 =A0compatible =3D "open-pic";
> =A0 =A0 =A0no-reset;
> =A0 };
>
> Or are you worried that someone will find the wrong "no-reset" property w=
hen
> searching from a location higher in the tree than the PIC node?
>
> I don't have a serious objection to the idea, but it seems slightly odd t=
o
> partially flatten the hierarchy back into the property names. =A0On the o=
ther
> hand, I do see the practical consideration of having a more unique proper=
ty
> which might prevent programming confusion/errors.
It's the sort of thing where properties with really generic names,
like no-reset, I could potentially see as gaining a meaning across the
whole tree. For instance, in the not so distant past the 'status'
property was defined for all nodes to indicate whether or not the
device is usable. If any binding defined status for its own purposes,
then it would now be broken. It is worth a little bit of
consideration to avoid collisions with names that might gain a meaning
in the global domain. I don't care much about what the specific name
is, and openpic-no-reset may indeed be a little long, so feel free to
suggest something that you like better.
g.
>
> --
> Meador Inge =A0 =A0 | meador_inge AT mentor.com
> Mentor Embedded | http://www.mentor.com/embedded-software
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc: document the Open PIC device tree binding
From: Meador Inge @ 2011-02-03 16:29 UTC (permalink / raw)
To: Grant Likely; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <AANLkTinVak_ukf0wP8=zbGTq8ENV4d88hNuzW_tqvCFf@mail.gmail.com>
On 02/03/2011 09:56 AM, Grant Likely wrote:
> On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge<meador_inge@mentor.com> wrote:
>> This binding documents several properties that have been in use for quite
>> some time, and adds one new property 'no-reset', which controls whether the
>> Open PIC should be reset during runtime initialization.
>>
>> The general formatting and interrupt specifier definition is based off of
>> Stuart Yoder's FSL MPIC binding.
>>
>> Signed-off-by: Meador Inge<meador_inge@mentor.com>
>> CC: Hollis Blanchard<hollis_blanchard@mentor.com>
>> CC: Stuart Yoder<stuart.yoder@freescale.com>
>> ---
>> Documentation/powerpc/dts-bindings/open-pic.txt | 115 +++++++++++++++++++++++
>> 1 files changed, 115 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>>
>> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentation/powerpc/dts-bindings/open-pic.txt
>> new file mode 100644
>> index 0000000..447ef65
>> --- /dev/null
>> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
>> @@ -0,0 +1,115 @@
>> +* Open PIC Binding
>> +
>> +This binding specifies what properties must be available in the device tree
>> +representation of an Open PIC compliant interrupt controller. This binding is
>> +based on the binding defined for Open PIC in [1] and is a superset of that
>> +binding.
>> +
>> +PROPERTIES
>> +
>> + NOTE: Many of these descriptions were paraphrased here from [1] to aid
>> + readability.
>> +
>> + - compatible
>> + Usage: required
>> + Value type:<string>
>> + Definition: Specifies the compatibility list for the PIC. The
>> + property value shall include "open-pic".
>> +
>> + - reg
>> + Usage: required
>> + Value type:<prop-encoded-array>
>> + Definition: Specifies the base physical address(s) and size(s) of this
>> + PIC's addressable register space.
>> +
>> + - interrupt-controller
>> + Usage: required
>> + Value type:<empty>
>> + Definition: The presence of this property identifies the node
>> + as an Open PIC. No property value should be defined.
>> +
>> + - #interrupt-cells
>> + Usage: required
>> + Value type:<u32>
>> + Definition: Specifies the number of cells needed to encode an
>> + interrupt source. Shall be 2.
>> +
>> + - #address-cells
>> + Usage: required
>> + Value type:<u32>
>> + Definition: Specifies the number of cells needed to encode an
>> + address. The value of this property shall always be 0.
>> + As such, 'interrupt-map' nodes do not have to specify a
>> + parent unit address.
>> +
>> + - no-reset
>> + Usage: optional
>> + Value type:<empty>
>> + Definition: The presence of this property indicates that the PIC
>> + should not be reset during runtime initialization. The presence of
>> + this property also mandates that any initialization related to
>> + interrupt sources shall be limited to sources explicitly referenced
>> + in the device tree.
>
> Please follow the lead set by the other binding documentation which is
> more concise and tends to be of the form:
>
> Required properties:
> - reg :<description>
> - interrupt-controller :<description>
>
> Optional Properties:
> - no-reset : blah
OK, will do. The one thing that I like about the other format, though,
is that it specifies the value type. That is a useful addition.
> I'm considering formalizing the binding format so that fully specified
> and cross-referenced documentation can be generated from the bindings
> directory.
Formalizing the binding format would be great. Perhaps we should add a
HOWTO write a new binding document to the "Documentation" directory?
The would be a great place to capture some of the common pitfalls that
have been coming up on the list lately (versioned compatibility tags,
for example).
> Also, to avoid the potential of a future namespace collision, it would
> not be a bad idea to name this openpic-no-reset or something that
> makes it clear that this is a binding specific property. "no-reset"
> sounds generic enough to give me pause.
Isn't that a little redundant, though (e.g.
"/soc/pic/openpic-no-reset")? It is already scoped to the PIC node:
mpic: pic@40000 {
compatible = "open-pic";
no-reset;
};
Or are you worried that someone will find the wrong "no-reset" property
when searching from a location higher in the tree than the PIC node?
I don't have a serious objection to the idea, but it seems slightly odd
to partially flatten the hierarchy back into the property names. On the
other hand, I do see the practical consideration of having a more unique
property which might prevent programming confusion/errors.
--
Meador Inge | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc: document the Open PIC device tree binding
From: Grant Likely @ 2011-02-03 15:56 UTC (permalink / raw)
To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1296697900-14004-3-git-send-email-meador_inge@mentor.com>
On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge <meador_inge@mentor.com> wrote:
> This binding documents several properties that have been in use for quite
> some time, and adds one new property 'no-reset', which controls whether t=
he
> Open PIC should be reset during runtime initialization.
>
> The general formatting and interrupt specifier definition is based off of
> Stuart Yoder's FSL MPIC binding.
>
> Signed-off-by: Meador Inge <meador_inge@mentor.com>
> CC: Hollis Blanchard <hollis_blanchard@mentor.com>
> CC: Stuart Yoder <stuart.yoder@freescale.com>
> ---
> =A0Documentation/powerpc/dts-bindings/open-pic.txt | =A0115 +++++++++++++=
++++++++++
> =A01 files changed, 115 insertions(+), 0 deletions(-)
> =A0create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>
> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentat=
ion/powerpc/dts-bindings/open-pic.txt
> new file mode 100644
> index 0000000..447ef65
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
> @@ -0,0 +1,115 @@
> +* Open PIC Binding
> +
> +This binding specifies what properties must be available in the device t=
ree
> +representation of an Open PIC compliant interrupt controller. =A0This bi=
nding is
> +based on the binding defined for Open PIC in [1] and is a superset of th=
at
> +binding.
> +
> +PROPERTIES
> +
> + =A0NOTE: Many of these descriptions were paraphrased here from [1] to a=
id
> + =A0 =A0 =A0 =A0readability.
> +
> + =A0- compatible
> + =A0 =A0 =A0Usage: required
> + =A0 =A0 =A0Value type: <string>
> + =A0 =A0 =A0Definition: Specifies the compatibility list for the PIC. =
=A0The
> + =A0 =A0 =A0 =A0 =A0property value shall include "open-pic".
> +
> + =A0- reg
> + =A0 =A0 =A0Usage: required
> + =A0 =A0 =A0Value type: <prop-encoded-array>
> + =A0 =A0 =A0Definition: Specifies the base physical address(s) and size(=
s) of this
> + =A0 =A0 =A0 =A0 =A0PIC's addressable register space.
> +
> + =A0- interrupt-controller
> + =A0 =A0 =A0Usage: required
> + =A0 =A0 =A0Value type: <empty>
> + =A0 =A0 =A0Definition: The presence of this property identifies the nod=
e
> + =A0 =A0 =A0 =A0 =A0as an Open PIC. =A0No property value should be defin=
ed.
> +
> + =A0- #interrupt-cells
> + =A0 =A0 =A0Usage: required
> + =A0 =A0 =A0Value type: <u32>
> + =A0 =A0 =A0Definition: Specifies the number of cells needed to encode a=
n
> + =A0 =A0 =A0 =A0 =A0interrupt source. =A0Shall be 2.
> +
> + =A0- #address-cells
> + =A0 =A0 =A0Usage: required
> + =A0 =A0 =A0Value type: <u32>
> + =A0 =A0 =A0Definition: Specifies the number of cells needed to encode a=
n
> + =A0 =A0 =A0 =A0 =A0address. =A0The value of this property shall always =
be 0.
> + =A0 =A0 =A0 =A0 =A0As such, 'interrupt-map' nodes do not have to specif=
y a
> + =A0 =A0 =A0 =A0 =A0parent unit address.
> +
> + =A0- no-reset
> + =A0 =A0 =A0Usage: optional
> + =A0 =A0 =A0Value type: <empty>
> + =A0 =A0 =A0Definition: The presence of this property indicates that the=
PIC
> + =A0 =A0 =A0 =A0 =A0should not be reset during runtime initialization. =
=A0The presence of
> + =A0 =A0 =A0 =A0 =A0this property also mandates that any initialization =
related to
> + =A0 =A0 =A0 =A0 =A0interrupt sources shall be limited to sources explic=
itly referenced
> + =A0 =A0 =A0 =A0 =A0in the device tree.
Please follow the lead set by the other binding documentation which is
more concise and tends to be of the form:
Required properties:
- reg : <description>
- interrupt-controller : <description>
Optional Properties:
- no-reset : blah
I'm considering formalizing the binding format so that fully specified
and cross-referenced documentation can be generated from the bindings
directory.
Also, to avoid the potential of a future namespace collision, it would
not be a bad idea to name this openpic-no-reset or something that
makes it clear that this is a binding specific property. "no-reset"
sounds generic enough to give me pause.
> +
> +INTERRUPT SPECIFIER DEFINITION
> +
> + =A0Interrupt specifiers consists of 2 cells encoded as
> + =A0follows:
> +
> + =A0 <1st-cell> =A0 interrupt-number
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Identifies the interrupt source.
> +
> + =A0 <2nd-cell> =A0 level-sense information, encoded as follows:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00 =3D low-to-high edge triggered
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A01 =3D active low level-sensitive
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A02 =3D active high level-sensitiv=
e
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A03 =3D high-to-low edge triggered
> +
> +EXAMPLE 1
> +
> + =A0 =A0/*
> + =A0 =A0 * An Open PIC interrupt controller
> + =A0 =A0 */
> + =A0 =A0 =A0 mpic: pic@40000 {
> + =A0 =A0 =A0 =A0// This is an interrupt controller node.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupt-controller;
> +
> + =A0 =A0 =A0 =A0// No address cells so that 'interrupt-map' nodes which =
reference
> + =A0 =A0 =A0 =A0// this Open PIC node do not need a parent address speci=
fier.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 #address-cells =3D <0>;
> +
> + =A0 =A0 =A0 =A0// Two cells to encode interrupt sources.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 #interrupt-cells =3D <2>;
> +
> + =A0 =A0 =A0 =A0// Offset address of 0x40000 and size of 0x40000.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x40000 0x40000>;
> +
> + =A0 =A0 =A0 =A0// Compatible with Open PIC.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "open-pic";
> +
> + =A0 =A0 =A0 =A0// The PIC should not be reset.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 no-reset;
> + =A0 =A0 =A0 };
> +
> +EXAMPLE 2
> +
> + =A0 =A0/*
> + =A0 =A0 * An interrupt generating device that is wired to an Open PIC.
> + =A0 =A0 */
> + =A0 =A0serial0: serial@4500 {
> + =A0 =A0 =A0 =A0// Interrupt source '42' that is active high level-sensi=
tive.
> + =A0 =A0 =A0 =A0// Note that there are only two cells as specified in th=
e interrupt
> + =A0 =A0 =A0 =A0// parent's '#interrupt-cells' property.
> + =A0 =A0 =A0 =A0interrupts =3D <42 2>;
> +
> + =A0 =A0 =A0 =A0// The interrupt controller that this device is wired to=
.
> + =A0 =A0 =A0 =A0interrupt-parent =3D <&mpic>;
> + =A0 =A0};
> +
> +REFERENCES
> +
> +[1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platfor=
m
> + =A0 =A0Requirements (ePAPR), Version 1.0, July 2008.
> + =A0 =A0(http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v=
1.0.pdf)
> +
> --
> 1.6.3.3
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc: Removing support for 'protected-sources'
From: Arnd Bergmann @ 2011-02-03 15:56 UTC (permalink / raw)
To: devicetree-discuss; +Cc: Meador Inge, Hollis Blanchard, linuxppc-dev
In-Reply-To: <1296697900-14004-2-git-send-email-meador_inge@mentor.com>
On Thursday 03 February 2011, Meador Inge wrote:
> In a recent discussion [1, 2] concerning device trees for AMP systems, the
> question of whether we really need 'protected-sources' arose. The general
> consensus was that if you don't want a source to be used, then it should *not*
> be mentioned in an 'interrupts' property. If a source really needs to be
> mentioned, then it should be put in a property other than 'interrupts' with
> a specific binding for that use case.
>
> [1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
> [2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html
That doesn't work in the case that this code was written for:
http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg01394.html
The problem is that you don't want the mpic to initialize the interrupt
line to the default, but instead leave it at whatever the boot firmware
has set up. Note that interrupt is not listed in any "interrupts"
property of any of the devices on the CPU interpreting the device
tree, but it may be mentioned in the device tree that another CPU
uses to access the same MPIC.
Arnd
^ permalink raw reply
* Re: [PATCH v2 0/3] powerpc: Open PIC binding and 'no-reset' implementation
From: Kumar Gala @ 2011-02-03 15:22 UTC (permalink / raw)
To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1296697900-14004-1-git-send-email-meador_inge@mentor.com>
On Feb 2, 2011, at 7:51 PM, Meador Inge wrote:
> This patch set provides a binding for Open PIC and implements support =
for
> a new property, specified by that binding, called 'no-reset'. With =
'no-reset'
> in place the 'protected-sources' property is no longer needed and was =
removed.
>=20
> Signed-off-by: Meador Inge <meador_inge@mentor.com>
> CC: Hollis Blanchard <hollis_blanchard@mentor.com>
>=20
> Meador Inge (3):
> powerpc: Removing support for 'protected-sources'
> powerpc: document the Open PIC device tree binding
> powerpc: make MPIC honor the 'no-reset' device tree property
>=20
> Documentation/powerpc/dts-bindings/open-pic.txt | 115 =
+++++++++++++++++++++++
> arch/powerpc/include/asm/mpic.h | 7 +-
> arch/powerpc/sysdev/mpic.c | 92 =
++++++++----------
> 3 files changed, 160 insertions(+), 54 deletions(-)
> create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>=20
Please cleanup all the .dts that are impacted by this.
- k
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: David Gibson @ 2011-02-03 5:03 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296690839.12290.108.camel@shaggy-w500>
On Wed, Feb 02, 2011 at 05:53:59PM -0600, Dave Kleikamp wrote:
> On Thu, 2011-02-03 at 10:08 +1100, David Gibson wrote:
> > On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> > > Since other OS's may be running on the other cores don't use tlbivax
> >
> > [snip]
> > > +#ifdef CONFIG_44x
> > > +void __init early_init_mmu_44x(void)
> > > +{
> > > + unsigned long root = of_get_flat_dt_root();
> > > + if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> > > + amp = 1;
> > > +}
> > > +#endif /* CONFIG_44x */
> >
> > A test against a hardcoded compatible string seems a nasty way to do
> > this. Maybe we should define a new boolean property for the root
> > node.
>
> I'm not crazy about this string, but I needed something in the device
> tree to key off of. Freescale has something similar (i.e.
> MPC8572DS-CAMP), so I chose to follow their example. I'd be happy to
> replace it with a boolean property. Any objection to just using
> "amp"?
Bit too short, I think. I'd suggest either spelling out
'asymmetric-multiprocessor' or 'cooperative-partition' (a more
accurate term, IMO).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [PATCH 6/6] ftrace syscalls: Early terminate search for sys_ni_syscall
From: Ian Munsie @ 2011-02-03 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, Namhyung Kim,
Jiri Kosina, Jason Baron, Alexander Graf, Steven Rostedt,
David Gibson, Ingo Molnar, Paul Mackerras, Ian Munsie,
KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Nathan Lynch,
Andrew Morton, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
Many system calls are unimplemented and mapped to sys_ni_syscall, but at
boot ftrace would still search through every syscall metadata entry for
a match which wouldn't be there.
This patch adds causes the search to terminate early if the system call
is not mapped.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
kernel/trace/trace_syscalls.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 7b76a65..f498154 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -84,6 +84,9 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
stop = (struct syscall_metadata *)__stop_syscalls_metadata;
kallsyms_lookup(syscall, NULL, NULL, NULL, str);
+ if (arch_syscall_match_sym_name(str, "sys_ni_syscall"))
+ return NULL;
+
for ( ; start < stop; start++) {
if (start->name && arch_syscall_match_sym_name(str, start->name))
return start;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 5/6] ftrace, powerpc: Implement raw syscall tracepoints on PowerPC
From: Ian Munsie @ 2011-02-03 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, Namhyung Kim,
Jiri Kosina, Jason Baron, Alexander Graf, Steven Rostedt,
David Gibson, Ingo Molnar, Paul Mackerras, Ian Munsie,
KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Nathan Lynch,
Andrew Morton, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>
From: Ian Munsie <imunsie@au.ibm.com>
This patch implements the raw syscall tracepoints on PowerPC and exports
them for ftrace syscalls to use.
To minimise reworking existing code, I slightly re-ordered the thread
info flags such that the new TIF_SYSCALL_TRACEPOINT bit would still fit
within the 16 bits of the andi. instruction's UI field. The instructions
in question are in /arch/powerpc/kernel/entry_{32,64}.S to and the
_TIF_SYSCALL_T_OR_A with the thread flags to see if system call tracing
is enabled.
In the case of 64bit PowerPC, arch_syscall_addr and
arch_syscall_match_sym_name are overridden to allow ftrace syscalls to
work given the unusual system call table structure and symbol names that
start with a period.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/ftrace.h | 14 ++++++++++++++
arch/powerpc/include/asm/syscall.h | 5 +++++
arch/powerpc/include/asm/thread_info.h | 7 +++++--
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/ftrace.c | 8 ++++++++
arch/powerpc/kernel/ptrace.c | 10 ++++++++++
7 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 7d69e9b..a0e8e02 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -134,6 +134,7 @@ config PPC
select HAVE_GENERIC_HARDIRQS
select HAVE_SPARSE_IRQ
select IRQ_PER_CPU
+ select HAVE_SYSCALL_TRACEPOINTS
config EARLY_PRINTK
bool
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index dde1296..169d039 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -60,4 +60,18 @@ struct dyn_arch_ftrace {
#endif
+#if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PPC64) && !defined(__ASSEMBLY__)
+#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
+static inline bool arch_syscall_match_sym_name(const char *sym, const char *name)
+{
+ /*
+ * Compare the symbol name with the system call name. Skip the .sys or .SyS
+ * prefix from the symbol name and the sys prefix from the system call name and
+ * just match the rest. This is only needed on ppc64 since symbol names on
+ * 32bit do not start with a period so the generic function will work.
+ */
+ return !strcmp(sym + 4, name + 3);
+}
+#endif /* CONFIG_FTRACE_SYSCALLS && CONFIG_PPC64 && !__ASSEMBLY__ */
+
#endif /* _ASM_POWERPC_FTRACE */
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 23913e9..b54b2ad 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -15,6 +15,11 @@
#include <linux/sched.h>
+/* ftrace syscalls requires exporting the sys_call_table */
+#ifdef CONFIG_FTRACE_SYSCALLS
+extern const unsigned long *sys_call_table;
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
{
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 65eb859..4403f09 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -110,7 +110,8 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NOERROR 12 /* Force successful syscall return */
#define TIF_NOTIFY_RESUME 13 /* callback before returning to user */
#define TIF_FREEZE 14 /* Freezing for suspend */
-#define TIF_RUNLATCH 15 /* Is the runlatch enabled? */
+#define TIF_SYSCALL_TRACEPOINT 15 /* syscall tracepoint instrumentation */
+#define TIF_RUNLATCH 16 /* Is the runlatch enabled? */
/* as above, but as bit values */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
@@ -127,8 +128,10 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_NOERROR (1<<TIF_NOERROR)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
#define _TIF_FREEZE (1<<TIF_FREEZE)
+#define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)
#define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
-#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)
+#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+ _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
#define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_NOTIFY_RESUME)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 3bb2a3e..fe1ac47 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -105,6 +105,7 @@ obj64-$(CONFIG_AUDIT) += compat_audit.o
obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
+obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o
obj-$(CONFIG_PPC_PERF_CTRS) += perf_event.o
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index ce1f3e4..bf99cfa 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -22,6 +22,7 @@
#include <asm/cacheflush.h>
#include <asm/code-patching.h>
#include <asm/ftrace.h>
+#include <asm/syscall.h>
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -600,3 +601,10 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+
+#if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PPC64)
+unsigned long __init arch_syscall_addr(int nr)
+{
+ return sys_call_table[nr*2];
+}
+#endif /* CONFIG_FTRACE_SYSCALLS && CONFIG_PPC64 */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 9065369..b6ff0cb 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -29,6 +29,7 @@
#include <linux/signal.h>
#include <linux/seccomp.h>
#include <linux/audit.h>
+#include <trace/syscall.h>
#ifdef CONFIG_PPC32
#include <linux/module.h>
#endif
@@ -40,6 +41,9 @@
#include <asm/pgtable.h>
#include <asm/system.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/syscalls.h>
+
/*
* The parameter save area on the stack is used to store arguments being passed
* to callee function and is located at fixed offset from stack pointer.
@@ -1691,6 +1695,9 @@ long do_syscall_trace_enter(struct pt_regs *regs)
*/
ret = -1L;
+ if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
+ trace_sys_enter(regs, regs->gpr[0]);
+
if (unlikely(current->audit_context)) {
#ifdef CONFIG_PPC64
if (!is_32bit_task())
@@ -1719,6 +1726,9 @@ void do_syscall_trace_leave(struct pt_regs *regs)
audit_syscall_exit((regs->ccr&0x10000000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
regs->result);
+ if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
+ trace_sys_exit(regs, regs->result);
+
step = test_thread_flag(TIF_SINGLESTEP);
if (step || test_thread_flag(TIF_SYSCALL_TRACE))
tracehook_report_syscall_exit(regs, step);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching
From: Ian Munsie @ 2011-02-03 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp,
open list:DOCUMENTATION, Namhyung Kim, Jiri Kosina, Jason Baron,
Alexander Graf, Steven Rostedt, David Gibson, Ingo Molnar,
Paul Mackerras, Ian Munsie, KOSAKI Motohiro, Frederic Weisbecker,
Scott Wood, Nathan Lynch, Andrew Morton, linuxppc-dev,
Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
Some architectures have unusual symbol names and the generic code to
match the symbol name with the function name for the syscall metadata
will fail. For example, symbols on PPC64 start with a period and the
generic code will fail to match them.
This patch moves the match logic out into a separate function which an
arch can override by defining ARCH_HAS_SYSCALL_MATCH_SYM_NAME in
asm/ftrace.h and implementing arch_syscall_match_sym_name.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
Documentation/trace/ftrace-design.txt | 4 ++++
kernel/trace/trace_syscalls.c | 21 ++++++++++++++-------
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 6fca17b..79fcafc 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -250,6 +250,10 @@ You need very few things to get the syscalls tracing in an arch.
- If the system call table on this arch is more complicated than a simple array
of addresses of the system calls, implement an arch_syscall_addr to return
the address of a given system call.
+- If the symbol names of the system calls do not match the function names on
+ this arch, define ARCH_HAS_SYSCALL_MATCH_SYM_NAME in asm/ftrace.h and
+ implement arch_syscall_match_sym_name with the appropriate logic to return
+ true if the function name corresponds with the symbol name.
- Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 33360b9..7b76a65 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -60,6 +60,19 @@ extern unsigned long __stop_syscalls_metadata[];
static struct syscall_metadata **syscalls_metadata;
+#ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME
+static inline bool arch_syscall_match_sym_name(const char *sym, const char *name)
+{
+ /*
+ * Only compare after the "sys" prefix. Archs that use
+ * syscall wrappers may have syscalls symbols aliases prefixed
+ * with "SyS" instead of "sys", leading to an unwanted
+ * mismatch.
+ */
+ return !strcmp(sym + 3, name + 3);
+}
+#endif
+
static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
{
struct syscall_metadata *start;
@@ -72,13 +85,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
kallsyms_lookup(syscall, NULL, NULL, NULL, str);
for ( ; start < stop; start++) {
- /*
- * Only compare after the "sys" prefix. Archs that use
- * syscall wrappers may have syscalls symbols aliases prefixed
- * with "SyS" instead of "sys", leading to an unwanted
- * mismatch.
- */
- if (start->name && !strcmp(start->name + 3, str + 3))
+ if (start->name && arch_syscall_match_sym_name(str, start->name))
return start;
}
return NULL;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/6] ftrace syscalls: Make arch_syscall_addr weak
From: Ian Munsie @ 2011-02-03 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp,
open list:DOCUMENTATION, Namhyung Kim, Jiri Kosina, Jason Baron,
Alexander Graf, Steven Rostedt, David Gibson, Ingo Molnar,
Paul Mackerras, Ian Munsie, KOSAKI Motohiro, Frederic Weisbecker,
Scott Wood, Nathan Lynch, Andrew Morton, linuxppc-dev,
Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
Some architectures use non-trivial system call tables and will not work
with the generic arch_syscall_addr code. For example, PowerPC64 uses a
table of twin long longs.
This patch makes the generic arch_syscall_addr weak to allow
architectures with non-trivial system call tables to override it.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
Documentation/trace/ftrace-design.txt | 3 +++
kernel/trace/trace_syscalls.c | 2 +-
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index dc52bd4..6fca17b 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -247,6 +247,9 @@ You need very few things to get the syscalls tracing in an arch.
- Support the TIF_SYSCALL_TRACEPOINT thread flags.
- Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
in the ptrace syscalls tracing path.
+- If the system call table on this arch is more complicated than a simple array
+ of addresses of the system calls, implement an arch_syscall_addr to return
+ the address of a given system call.
- Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 1a6e8dd..33360b9 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -445,7 +445,7 @@ int init_syscall_trace(struct ftrace_event_call *call)
return id;
}
-unsigned long __init arch_syscall_addr(int nr)
+unsigned long __init __weak arch_syscall_addr(int nr)
{
return (unsigned long)sys_call_table[nr];
}
--
1.7.2.3
^ 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