* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Tabi Timur-B04825 @ 2011-02-05 2:04 UTC (permalink / raw)
To: Dan Malek; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <2A5A3DDA-94B3-48E3-9E49-6A5260BE1484@digitaldans.com>
Dan Malek wrote:
>
>
>> On a side note, do I really need to pass GFP_DMA ....
>
> The GFP_DMA is architecture dependent. Are you writing
> a driver to be used across multiple architectures?
It's conceivable that the driver could work on PowerPC and ARM.
> If it's
> necessary, I'd document why you are using it (an ISA device
> on x86 for example) and then let other architectures
> determine if it's necessary for them.
I guess I'm not clear. I was wondering why an API called "dma_alloc_cohere=
nt" (that has the word "dma" in it) needs to be told to allocate DMA-safe m=
emory. When would it make sense to call dma_alloc_cohernet without GFP_DMA=
? If you don't want DMA-able memory, then you shouldn't be calling dma_all=
oc_anything.
--=20
Timur Tabi
Linux kernel developer=
^ permalink raw reply
* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Dan Malek @ 2011-02-05 1:09 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <AANLkTi=1VkJM6hJH44jeUmYwoEtJj22d6WyKVDitg=KY@mail.gmail.com>
On Feb 4, 2011, at 4:14 PM, Timur Tabi wrote:
> Is there any official statement on what the minimum alignment is for
> memory returned by dma_alloc_coherent?
This is dependent upon the particular implementation.
There have been several over the history of this API,
and some would work out of a DMA pool that would
only provide word alignment.
> Would anyone have a complaint if I did this:
>
> vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA | __GFP_ZERO);
> BUG_ON(paddr & (alignment - 1));
Well, the usual design for anything that requires alignment
is that you never assume you get it unless it can be specified,
so over allocate as you are doing currently, and then adjust
the base address. Or, you could allocate exactly what you
want, if it aligns properly you are good to go, and if not release,
over allocate, and adjust.
The problem with the suggested implementation is the results
aren't predictable. Even if the API doesn't guarantee your
alignment, you may get it most of the time, but once in a
while this may fail. You could test and all looks good,
ship something and it fails.
Today, this may work fine, but it would be nice to ensure
it continues to work in the future. :-)
> On a side note, do I really need to pass GFP_DMA ....
The GFP_DMA is architecture dependent. Are you writing
a driver to be used across multiple architectures? If it's
necessary, I'd document why you are using it (an ISA device
on x86 for example) and then let other architectures
determine if it's necessary for them.
Thanks.
-- Dan
^ permalink raw reply
* minimum guaranteed alignment of dma_alloc_coherent?
From: Timur Tabi @ 2011-02-05 0:14 UTC (permalink / raw)
To: linuxppc-dev
Is there any official statement on what the minimum alignment is for
memory returned by dma_alloc_coherent? I know that since it uses a
page allocator to do the actual allocation, that the memory is page
aligned (at least on PowerPC). Is this something I can rely on?
Would anyone have a complaint if I did this:
vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA | __GFP_ZERO);
BUG_ON(paddr & (alignment - 1));
'alignment' will always be 32 or less.
The reason I ask is that I currently have code that ensures an
alignment, but it does so by allocating extra memory, and that seems
wasteful.
On a side note, do I really need to pass GFP_DMA when calling
dma_alloc_coherent? That seems redundant, but I don't see the code
force it to be set. Should it be forced on?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Problem with Busybox shell
From: Gorelik, Jacob (335F) @ 2011-02-04 23:14 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org; +Cc: mohanreddykv@gmail.com
Hi Mohan,
Were you able to fix your problem? I am having the same problem with my boa=
rd. It is PPC750FX with Marvell MV64460.
Jacob=
^ permalink raw reply
* [PATCH v3 4/4] powerpc: Replacing "protected-sources" with "pic-no-reset" in DTS files
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>
The "protected-sources" property was being used in the AMP configured
MPC8572DS and P2020RDB DTS files. This changeset modifies those files
to use "pic-no-reset" instead.
Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
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 +------
4 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
index 3375c2a..4f7ee6d 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
@@ -252,11 +252,7 @@
reg = <0x40000 0x40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
- protected-sources = <
- 31 32 33 37 38 39 /* enet2 enet3 */
- 76 77 78 79 26 42 /* dma2 pci2 serial*/
- 0xe4 0xe5 0xe6 0xe7 /* msi */
- >;
+ pic-no-reset;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
index e7b477f..62fd64f 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
@@ -178,16 +178,7 @@
reg = <0x40000 0x40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
- protected-sources = <
- 18 16 10 42 45 58 /* MEM L2 mdio serial crypto */
- 29 30 34 35 36 40 /* enet0 enet1 */
- 24 25 20 21 22 23 /* pci0 pci1 dma1 */
- 43 /* i2c */
- 0x1 0x2 0x3 0x4 /* pci slot */
- 0x9 0xa 0xb 0xc /* usb */
- 0x6 0x7 0xe 0x5 /* Audio elgacy SATA */
- 0xe0 0xe1 0xe2 0xe3 /* msi */
- >;
+ pic-no-reset;
};
};
diff --git a/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts b/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
index 0fe93d0..f749cbb 100644
--- a/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
+++ b/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
@@ -318,12 +318,7 @@
reg = <0x40000 0x40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
- protected-sources = <
- 42 76 77 78 79 /* serial1 , dma2 */
- 29 30 34 26 /* enet0, pci1 */
- 0xe0 0xe1 0xe2 0xe3 /* msi */
- 0xe4 0xe5 0xe6 0xe7
- >;
+ pic-no-reset;
};
global-utilities@e0000 {
diff --git a/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts b/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
index e95a512..6ef2e64 100644
--- a/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
+++ b/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
@@ -129,12 +129,7 @@
reg = <0x40000 0x40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
- protected-sources = <
- 17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
- 16 20 21 22 23 28 /* L2, dma1, USB */
- 03 35 36 40 31 32 33 /* mdio, enet1, enet2 */
- 72 45 58 25 /* sdhci, crypto , pci */
- >;
+ pic-no-reset;
};
msi@41600 {
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 3/4] powerpc: make MPIC honor the "pic-no-reset" device tree property
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>
This property, defined in the Open PIC binding, tells the kernel not to use the
reset bit in the global configuration register. Additionally, its presence
mandates that only sources which are actually used (i.e. appear in the device
tree) should have their VECPRI bits initialized.
The presence of "protected-sources" is checked for the backwards
compatibility of systems which are already using "protected-sources"
in firmware and the firmware can not be updated. Any sources encoded in the
"protected-sources" cells are not processed. "protected-sources" is
effectively a synonym for "pic-no-reset".
Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/include/asm/mpic.h | 4 ++-
arch/powerpc/sysdev/mpic.c | 64 +++++++++++++++++++++++++++++++-------
2 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 9b94f18..688e3e0 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -322,6 +322,8 @@ struct mpic
#ifdef CONFIG_PM
struct mpic_irq_save *save_data;
#endif
+
+ int cpu;
};
/*
@@ -333,7 +335,7 @@ struct mpic
*/
/* This is the primary controller, only that one has IPIs and
- * has afinity control. A non-primary MPIC always uses CPU0
+ * has affinity control. A non-primary MPIC always uses CPU0
* registers only
*/
#define MPIC_PRIMARY 0x00000001
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index a98f41d..8de47ca 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -308,6 +308,15 @@ static inline void mpic_map(struct mpic *mpic, struct device_node *node,
#define mpic_map(m,n,p,b,o,s) _mpic_map_mmio(m,p,b,o,s)
#endif /* !CONFIG_PPC_DCR */
+static inline void mpic_init_vector(struct mpic *mpic, int source)
+{
+ /* start with vector = source number, and masked */
+ u32 vecpri = MPIC_VECPRI_MASK | source | (8 << MPIC_VECPRI_PRIORITY_SHIFT);
+
+ /* init hw */
+ mpic_irq_write(source, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
+ mpic_irq_write(source, MPIC_INFO(IRQ_DESTINATION), 1 << mpic->cpu);
+}
/* Check if we have one of those nice broken MPICs with a flipped endian on
@@ -622,6 +631,14 @@ static unsigned int mpic_is_ipi(struct mpic *mpic, unsigned int irq)
return (src >= mpic->ipi_vecs[0] && src <= mpic->ipi_vecs[3]);
}
+/* Determine if the linux irq is a timer interrupt */
+static unsigned int mpic_is_timer_interrupt(struct mpic *mpic, unsigned int irq)
+{
+ unsigned int src = mpic_irq_to_hw(irq);
+
+ return (src >= mpic->timer_vecs[0] && src <= mpic->timer_vecs[3]);
+}
+
/* Convert a cpu mask from logical to physical cpu numbers. */
static inline u32 mpic_physmask(u32 cpumask)
@@ -963,6 +980,15 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
if (hw >= mpic->irq_count)
return -EINVAL;
+ /* If the MPIC was reset, then all vectors have already been
+ * initialized. Otherwise, the appropriate vector needs to be
+ * initialized here to ensure that only used sources are setup with
+ * a vector.
+ */
+ if (!(mpic->flags & MPIC_WANTS_RESET))
+ if (!(mpic_is_ipi(mpic, hw) || mpic_is_timer_interrupt(mpic, hw)))
+ mpic_init_vector(mpic, hw);
+
mpic_msi_reserve_hwirq(mpic, hw);
/* Default chip */
@@ -1029,6 +1055,16 @@ static struct irq_host_ops mpic_host_ops = {
.xlate = mpic_host_xlate,
};
+static int mpic_reset_prohibited(struct device_node *node)
+{
+ /* The presence of "protected-sources" is checked for the backwards
+ * compatibility of systems which are already using "protected-sources"
+ * in firmware and the firmware can not be updated.
+ */
+ return node && (of_get_property(node, "pic-no-reset", NULL)
+ || of_get_property(node, "protected-sources", NULL));
+}
+
/*
* Exported functions
*/
@@ -1129,7 +1165,16 @@ struct mpic * __init mpic_alloc(struct device_node *node,
mpic_map(mpic, node, paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
/* Reset */
- if (flags & MPIC_WANTS_RESET) {
+
+ /* When using a device-node, reset requests are only honored if the MPIC
+ * is allowed to reset.
+ */
+ if (mpic_reset_prohibited(node)) {
+ mpic->flags &= ~MPIC_WANTS_RESET;
+ }
+
+ if (mpic->flags & MPIC_WANTS_RESET) {
+ printk(KERN_DEBUG "mpic: Resetting\n");
mpic_write(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0),
mpic_read(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0))
| MPIC_GREG_GCONF_RESET);
@@ -1246,7 +1291,6 @@ void __init mpic_set_default_senses(struct mpic *mpic, u8 *senses, int count)
void __init mpic_init(struct mpic *mpic)
{
int i;
- int cpu;
BUG_ON(mpic->num_sources == 0);
@@ -1290,18 +1334,14 @@ void __init mpic_init(struct mpic *mpic)
mpic_pasemi_msi_init(mpic);
if (mpic->flags & MPIC_PRIMARY)
- cpu = hard_smp_processor_id();
+ mpic->cpu = hard_smp_processor_id();
else
- cpu = 0;
+ mpic->cpu = 0;
- 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);
-
- /* init hw */
- mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
- mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
+ if (mpic->flags & MPIC_WANTS_RESET) {
+ for (i = 0; i < mpic->num_sources; i++) {
+ mpic_init_vector(mpic, i);
+ }
}
/* Init spurious vector */
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 2/4] powerpc: document the Open PIC device tree binding
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard, Stuart Yoder
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>
This binding documents several properties that have been in use for quite
some time, and adds one new property 'pic-no-reset', which controls the runtime
initialization behavior of the PIC. More specifically, the presence of
'pic-no-reset' mandates that the PIC shall not be reset during runtime
initialization and that any initialization related to interrupt sources
shall be limited to sources explicitly referenced in the device tree. This
functionality is useful in AMP systems where multiple OSes are sharing the
PIC and the reinitialization of the PIC can interfere with OSes that are
already up and running.
The 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 | 98 +++++++++++++++++++++++
1 files changed, 98 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..909a902
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/open-pic.txt
@@ -0,0 +1,98 @@
+* 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.
+
+Required properties:
+
+ NOTE: Many of these descriptions were paraphrased here from [1] to aid
+ readability.
+
+ - compatible: Specifies the compatibility list for the PIC. The type
+ shall be <string> and the value shall include "open-pic".
+
+ - reg: Specifies the base physical address(s) and size(s) of this
+ PIC's addressable register space. The type shall be <prop-encoded-array>.
+
+ - interrupt-controller: The presence of this property identifies the node
+ as an Open PIC. No property value shall be defined.
+
+ - #interrupt-cells: Specifies the number of cells needed to encode an
+ interrupt source. The type shall be a <u32> and the value shall be 2.
+
+ - #address-cells: Specifies the number of cells needed to encode an
+ address. The type shall be <u32> and the value shall be 0. As such,
+ 'interrupt-map' nodes do not have to specify a parent unit address.
+
+Optional properties:
+
+ - pic-no-reset: The presence of this property indicates that the PIC
+ shall not be reset during runtime initialization. No property value shall
+ be defined. 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.
+
+* Interrupt Specifier Definition
+
+ Interrupt specifiers consists of 2 cells encoded as
+ follows:
+
+ - <1st-cell>: The interrupt-number that identifies the interrupt source.
+
+ - <2nd-cell>: The level-sense information, encoded as follows:
+ 0 = low-to-high edge triggered
+ 1 = active low level-sensitive
+ 2 = active high level-sensitive
+ 3 = high-to-low edge triggered
+
+* Examples
+
+Example 1:
+
+ /*
+ * An Open PIC interrupt controller
+ */
+ mpic: pic@40000 {
+ // This is an interrupt controller node.
+ interrupt-controller;
+
+ // No address cells so that 'interrupt-map' nodes which reference
+ // this Open PIC node do not need a parent address specifier.
+ #address-cells = <0>;
+
+ // Two cells to encode interrupt sources.
+ #interrupt-cells = <2>;
+
+ // Offset address of 0x40000 and size of 0x40000.
+ reg = <0x40000 0x40000>;
+
+ // Compatible with Open PIC.
+ compatible = "open-pic";
+
+ // The PIC shall not be reset.
+ pic-no-reset;
+ };
+
+Example 2:
+
+ /*
+ * An interrupt generating device that is wired to an Open PIC.
+ */
+ serial0: serial@4500 {
+ // Interrupt source '42' that is active high level-sensitive.
+ // Note that there are only two cells as specified in the interrupt
+ // parent's '#interrupt-cells' property.
+ interrupts = <42 2>;
+
+ // The interrupt controller that this device is wired to.
+ interrupt-parent = <&mpic>;
+ };
+
+* References
+
+[1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platform
+ Requirements (ePAPR), Version 1.0, July 2008.
+ (http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
+
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>
In a recent thread [1,2,3] concerning device trees for AMP systems, the
question of whether we really need 'protected-sources' arose. The general
consensus was that a new boolean property 'pic-no-reset' (described in more
detail in a following patch) could be expanded to cover the use cases that
'protected-sources' was covering.
One concern that was raised was for legacy systems which already use the
'protected-sources' property [4]. For legacy use cases, 'protected-sources'
will be treated as an alias of 'pic-no-reset'. The sources
encoded in the 'protected-sources' cells, however, will not be processed. This
legacy check is added in a later patch in the series.
[1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
[2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html
[3] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004043.html
[4] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-February/004254.html
Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/include/asm/mpic.h | 3 ---
arch/powerpc/sysdev/mpic.c | 38 --------------------------------------
2 files changed, 0 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index e000cce..9b94f18 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -301,9 +301,6 @@ struct mpic
struct mpic_reg_bank cpuregs[MPIC_MAX_CPUS];
struct mpic_reg_bank isus[MPIC_MAX_ISU];
- /* Protected sources */
- unsigned long *protected;
-
#ifdef CONFIG_MPIC_WEIRD
/* Pointer to HW info array */
u32 *hw_set;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 7c13426..a98f41d 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -947,8 +947,6 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
if (hw == mpic->spurious_vec)
return -EINVAL;
- if (mpic->protected && test_bit(hw, mpic->protected))
- return -EINVAL;
#ifdef CONFIG_SMP
else if (hw >= mpic->ipi_vecs[0]) {
@@ -1095,26 +1093,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
if (node && of_get_property(node, "big-endian", NULL) != NULL)
mpic->flags |= MPIC_BIG_ENDIAN;
- /* Look for protected sources */
- if (node) {
- int psize;
- unsigned int bits, mapsize;
- const u32 *psrc =
- of_get_property(node, "protected-sources", &psize);
- if (psrc) {
- psize /= 4;
- bits = intvec_top + 1;
- mapsize = BITS_TO_LONGS(bits) * sizeof(unsigned long);
- mpic->protected = kzalloc(mapsize, GFP_KERNEL);
- BUG_ON(mpic->protected == NULL);
- for (i = 0; i < psize; i++) {
- if (psrc[i] > intvec_top)
- continue;
- __set_bit(psrc[i], mpic->protected);
- }
- }
- }
-
#ifdef CONFIG_MPIC_WEIRD
mpic->hw_set = mpic_infos[MPIC_GET_REGSET(flags)];
#endif
@@ -1321,9 +1299,6 @@ void __init mpic_init(struct mpic *mpic)
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);
@@ -1492,13 +1467,6 @@ static unsigned int _mpic_get_one_irq(struct mpic *mpic, int reg)
mpic_eoi(mpic);
return NO_IRQ;
}
- if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
- if (printk_ratelimit())
- printk(KERN_WARNING "%s: Got protected source %d !\n",
- mpic->name, (int)src);
- mpic_eoi(mpic);
- return NO_IRQ;
- }
return irq_linear_revmap(mpic->irqhost, src);
}
@@ -1532,12 +1500,6 @@ unsigned int mpic_get_coreint_irq(void)
mpic_eoi(mpic);
return NO_IRQ;
}
- if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
- if (printk_ratelimit())
- printk(KERN_WARNING "%s: Got protected source %d !\n",
- mpic->name, (int)src);
- return NO_IRQ;
- }
return irq_linear_revmap(mpic->irqhost, src);
#else
--
1.6.3.3
^ permalink raw reply related
* [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
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